qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/2] pci-assign: Fix memory out of bound when MS


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH 2/2] pci-assign: Fix memory out of bound when MSI-X table not fit in a single page
Date: Mon, 28 Apr 2014 15:51:00 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0

Il 09/04/2014 15:52, Michael S. Tsirkin ha scritto:
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 */


Michael, can you send it as toplevel and with a nice commit message so that I can apply it to uq/master? In the meanwhile I've applied patch 1.

Paolo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]