Re: [Qemu-devel] [PATCH 2/2] pci-assign: Fix memory out of bound when MSI-X table not fit in a single page

[Michael S. Tsirkin]

On Wed, Apr 09, 2014 at 10:56:57AM +0000, Gonglei (Arei) wrote:
> Hi,
> 
> > > QEMU only mmap MSIX_PAGE_SIZE memory for all pci devices in
> > > assigned_dev_register_msix_mmio(), meanwhile the set the one
> > > page memmory to zero, so the rest memory will be random value
> > > (maybe etnry.data is not 0). In the assigned_dev_update_msix_mmio()
> > > maybe occur the issue of entry_nr > 256, and the kmod reports
> > > the EINVAL error.
> > >
> > > Signed-off-by: Gonglei <address@hidden>
> > 
> > Okay so this kind of works because guest does not try
> > to use so many vectors.
> > 
> Yes. It's true.
> 
> > But I think it's better not to give guest more entries
> > than we can actually support.
> > 
> > How about tweaking MSIX capability exposed to guest to limit table size?
> > 
> IMHO, the MSIX table entry size got form the physic NIC hardware. The software
> layer shouldn't tweak the capability of real hardware, neither QEMU nor KVM. 
> 
> 
> Best regards,
> -Gonglei

Should be easy to fix though. Does the following help?

-->

pci-assign: limit # of msix vectors


Signed-off-by: Michael S. Tsirkin <address@hidden>


diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index a825871..76aa86e 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1258,6 +1258,7 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
     if (pos != 0 && kvm_device_msix_supported(kvm_state)) {
         int bar_nr;
         uint32_t msix_table_entry;
+        uint16_t msix_max;
 
         if (!check_irqchip_in_kernel()) {
             return -ENOTSUP;
@@ -1269,9 +1270,10 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
         }
         pci_dev->msix_cap = pos;
 
-        pci_set_word(pci_dev->config + pos + PCI_MSIX_FLAGS,
-                     pci_get_word(pci_dev->config + pos + PCI_MSIX_FLAGS) &
-                     PCI_MSIX_FLAGS_QSIZE);
+        msix_max = (pci_get_word(pci_dev->config + pos + PCI_MSIX_FLAGS) &
+                    PCI_MSIX_FLAGS_QSIZE) + 1;
+        msix_max = MIN(msix_max, KVM_MAX_MSIX_PER_DEV);
+        pci_set_word(pci_dev->config + pos + PCI_MSIX_FLAGS, msix_max - 1);
 
         /* Only enable and function mask bits are writable */
         pci_set_word(pci_dev->wmask + pos + PCI_MSIX_FLAGS,
@@ -1281,9 +1283,7 @@ static int assigned_device_pci_cap_init(PCIDevice 
*pci_dev)
         bar_nr = msix_table_entry & PCI_MSIX_FLAGS_BIRMASK;
         msix_table_entry &= ~PCI_MSIX_FLAGS_BIRMASK;
         dev->msix_table_addr = pci_region[bar_nr].base_addr + msix_table_entry;
-        dev->msix_max = pci_get_word(pci_dev->config + pos + PCI_MSIX_FLAGS);
-        dev->msix_max &= PCI_MSIX_FLAGS_QSIZE;
-        dev->msix_max += 1;
+        dev->msix_max = msix_max;
     }
 
     /* Minimal PM support, nothing writable, device appears to NAK changes */