[Spice-commits] 7 commits - hw/vfio_pci.c main-loop.c tests/m48t59-test.c tests/rtc-test.c

Gerd Hoffmann kraxel at kemper.freedesktop.org
Thu Jan 10 23:33:46 PST 2013


 hw/vfio_pci.c       |   34 ++++++++++++++++++++++++++++------
 main-loop.c         |   27 +++++++++++++--------------
 tests/m48t59-test.c |    5 +++++
 tests/rtc-test.c    |    4 ++++
 4 files changed, 50 insertions(+), 20 deletions(-)

New commits:
commit a6308bc2224db238e72c570482717b68246a7ce0
Merge: 8757c09 067f069
Author: Anthony Liguori <aliguori at us.ibm.com>
Date:   Thu Jan 10 13:26:31 2013 -0600

    Merge remote-tracking branch 'kraxel/build.1' into staging
    
    * kraxel/build.1:
      m48t59-test: don't touch watchdog
      rtc-test: skip year-2038 overflow check in case time_t is 32bit only
    
    Signed-off-by: Anthony Liguori <aliguori at us.ibm.com>

commit 8757c09f15dcd455f81b4faed73da0d35d7e6b53
Merge: 5e3bc73 8fc94e5
Author: Anthony Liguori <aliguori at us.ibm.com>
Date:   Thu Jan 10 13:26:12 2013 -0600

    Merge remote-tracking branch 'awilliam/tags/qemu-1.4-vfio-20130109.0' into staging
    
    vfio-pci: Fixes for qemu 1.4 & stable
    
    * awilliam/tags/qemu-1.4-vfio-20130109.0:
      vfio-pci: Loosen sanity checks to allow future features
      vfio-pci: Make host MSI-X enable track guest
    
    Signed-off-by: Anthony Liguori <aliguori at us.ibm.com>

commit 067f0691277325dcce8401534d2ffc6164305021
Author: Gerd Hoffmann <kraxel at redhat.com>
Date:   Fri Jan 4 17:12:18 2013 +0100

    m48t59-test: don't touch watchdog
    
    Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>

diff --git a/tests/m48t59-test.c b/tests/m48t59-test.c
index 5179681..d79f554 100644
--- a/tests/m48t59-test.c
+++ b/tests/m48t59-test.c
@@ -233,6 +233,11 @@ static void fuzz_registers(void)
         reg = (uint8_t)g_test_rand_int_range(0, 16);
         val = (uint8_t)g_test_rand_int_range(0, 256);
 
+        if (reg == 7) {
+            /* watchdog setup register, may trigger system reset, skip */
+            continue;
+        }
+
         cmos_write(reg, val);
         cmos_read(reg);
     }
commit 4e45deedf57c6cc7113b588282d0c16f89298aff
Author: Gerd Hoffmann <kraxel at redhat.com>
Date:   Fri Jan 4 10:37:50 2013 +0100

    rtc-test: skip year-2038 overflow check in case time_t is 32bit only
    
    Signed-off-by: Gerd Hoffmann <kraxel at redhat.com>

diff --git a/tests/rtc-test.c b/tests/rtc-test.c
index 02edbf5..e7123ca 100644
--- a/tests/rtc-test.c
+++ b/tests/rtc-test.c
@@ -201,6 +201,10 @@ static void set_year_20xx(void)
     g_assert_cmpint(cmos_read(RTC_YEAR), ==, 0x11);
     g_assert_cmpint(cmos_read(RTC_CENTURY), ==, 0x20);
 
+    if (sizeof(time_t) == 4) {
+        return;
+    }
+
     /* Set a date in 2080 to ensure there is no year-2038 overflow.  */
     cmos_write(RTC_REG_A, 0x76);
     cmos_write(RTC_YEAR, 0x80);
commit 5e3bc735d93dd23f074b5116fd11e1ad8cd4962f
Author: Fabien Chouteau <chouteau at adacore.com>
Date:   Tue Jan 8 16:30:56 2013 +0100

    Check return values from g_poll and select
    
    The current implementation of os_host_main_loop_wait() on Windows,
    returns 1 only when a g_poll() event occurs because the return value of
    select() is overridden. This is wrong as we may skip a socket event, as
    shown in this example:
    
    1. select() returns 0
    2. g_poll() returns 1  (socket event occurs)
    3. os_host_main_loop_wait() returns 1
    4. qemu_iohandler_poll() sees no socket event because select() has
       return before the event occurs
    5. select() returns 1
    6. g_poll() returns 0 (g_poll overrides select's return value)
    7. os_host_main_loop_wait() returns 0
    8. qemu_iohandler_poll() doesn't check for socket events because the
       return value of os_host_main_loop_wait() is zero.
    9. goto 5
    
    This patch use one variable for each of these return values, so we don't
    miss a select() event anymore.
    
    Also move the call to select() after g_poll(), this will improve latency
    as we don't have to go through two os_host_main_loop_wait() calls to
    detect a socket event.
    
    Reviewed-by: Paolo Bonzini <pbonzini at redhat.com>
    Signed-off-by: Fabien Chouteau <chouteau at adacore.com>
    Signed-off-by: Anthony Liguori <aliguori at us.ibm.com>

diff --git a/main-loop.c b/main-loop.c
index 54f38ae..6f52ac3 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -330,7 +330,7 @@ void qemu_fd_register(int fd)
 static int os_host_main_loop_wait(uint32_t timeout)
 {
     GMainContext *context = g_main_context_default();
-    int ret, i;
+    int select_ret, g_poll_ret, ret, i;
     PollingEntry *pe;
     WaitObjects *w = &wait_objects;
     gint poll_timeout;
@@ -345,13 +345,6 @@ static int os_host_main_loop_wait(uint32_t timeout)
         return ret;
     }
 
-    if (nfds >= 0) {
-        ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv0);
-        if (ret != 0) {
-            timeout = 0;
-        }
-    }
-
     g_main_context_prepare(context, &max_priority);
     n_poll_fds = g_main_context_query(context, max_priority, &poll_timeout,
                                       poll_fds, ARRAY_SIZE(poll_fds));
@@ -367,9 +360,9 @@ static int os_host_main_loop_wait(uint32_t timeout)
     }
 
     qemu_mutex_unlock_iothread();
-    ret = g_poll(poll_fds, n_poll_fds + w->num, poll_timeout);
+    g_poll_ret = g_poll(poll_fds, n_poll_fds + w->num, poll_timeout);
     qemu_mutex_lock_iothread();
-    if (ret > 0) {
+    if (g_poll_ret > 0) {
         for (i = 0; i < w->num; i++) {
             w->revents[i] = poll_fds[n_poll_fds + i].revents;
         }
@@ -384,12 +377,18 @@ static int os_host_main_loop_wait(uint32_t timeout)
         g_main_context_dispatch(context);
     }
 
-    /* If an edge-triggered socket event occurred, select will return a
-     * positive result on the next iteration.  We do not need to do anything
-     * here.
+    /* Call select after g_poll to avoid a useless iteration and therefore
+     * improve socket latency.
      */
 
-    return ret;
+    if (nfds >= 0) {
+        select_ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv0);
+        if (select_ret != 0) {
+            timeout = 0;
+        }
+    }
+
+    return select_ret || g_poll_ret;
 }
 #endif
 
commit 8fc94e5a8046e349e07976f9bcaffbcd5833f3a2
Author: Alex Williamson <alex.williamson at redhat.com>
Date:   Tue Jan 8 14:10:03 2013 -0700

    vfio-pci: Loosen sanity checks to allow future features
    
    VFIO_PCI_NUM_REGIONS and VFIO_PCI_NUM_IRQS should never have been
    used in this manner as it locks a specific kernel implementation.
    Future features may introduce new regions or interrupt entries
    (VGA may add legacy ranges, AER might add an IRQ for error
    signalling).  Fix this before it gets us into trouble.
    
    Signed-off-by: Alex Williamson <alex.williamson at redhat.com>
    Cc: qemu-stable at nongnu.org

diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c
index 8ec1faf..c51ae67 100644
--- a/hw/vfio_pci.c
+++ b/hw/vfio_pci.c
@@ -1837,13 +1837,13 @@ static int vfio_get_device(VFIOGroup *group, const char *name, VFIODevice *vdev)
         error_report("Warning, device %s does not support reset\n", name);
     }
 
-    if (dev_info.num_regions != VFIO_PCI_NUM_REGIONS) {
+    if (dev_info.num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
         error_report("vfio: unexpected number of io regions %u\n",
                      dev_info.num_regions);
         goto error;
     }
 
-    if (dev_info.num_irqs != VFIO_PCI_NUM_IRQS) {
+    if (dev_info.num_irqs < VFIO_PCI_MSIX_IRQ_INDEX + 1) {
         error_report("vfio: unexpected number of irqs %u\n", dev_info.num_irqs);
         goto error;
     }
commit b0223e29afdc88cc262a764026296414396cd129
Author: Alex Williamson <alex.williamson at redhat.com>
Date:   Tue Jan 8 14:09:03 2013 -0700

    vfio-pci: Make host MSI-X enable track guest
    
    Guests typically enable MSI-X with all of the vectors in the MSI-X
    vector table masked.  Only when the vector is enabled does the vector
    get unmasked, resulting in a vector_use callback.  These two points,
    enable and unmask, correspond to pci_enable_msix() and request_irq()
    for Linux guests.  Some drivers rely on VF/PF or PF/fw communication
    channels that expect the physical state of the device to match the
    guest visible state of the device.  They don't appreciate lazily
    enabling MSI-X on the physical device.
    
    To solve this, enable MSI-X with a single vector when the MSI-X
    capability is enabled and immediate disable the vector.  This leaves
    the physical device in exactly the same state between host and guest.
    Furthermore, the brief gap where we enable vector 0, it fires into
    userspace, not KVM, so the guest doesn't get spurious interrupts.
    Ideally we could call VFIO_DEVICE_SET_IRQS with the right parameters
    to enable MSI-X with zero vectors, but this will currently return an
    error as the Linux MSI-X interfaces do not allow it.
    
    Signed-off-by: Alex Williamson <alex.williamson at redhat.com>
    Cc: qemu-stable at nongnu.org

diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c
index 28c8303..8ec1faf 100644
--- a/hw/vfio_pci.c
+++ b/hw/vfio_pci.c
@@ -562,8 +562,8 @@ static int vfio_enable_vectors(VFIODevice *vdev, bool msix)
     return ret;
 }
 
-static int vfio_msix_vector_use(PCIDevice *pdev,
-                                unsigned int nr, MSIMessage msg)
+static int vfio_msix_vector_do_use(PCIDevice *pdev, unsigned int nr,
+                                   MSIMessage *msg, IOHandler *handler)
 {
     VFIODevice *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
     VFIOMSIVector *vector;
@@ -587,7 +587,7 @@ static int vfio_msix_vector_use(PCIDevice *pdev,
      * Attempt to enable route through KVM irqchip,
      * default to userspace handling if unavailable.
      */
-    vector->virq = kvm_irqchip_add_msi_route(kvm_state, msg);
+    vector->virq = msg ? kvm_irqchip_add_msi_route(kvm_state, *msg) : -1;
     if (vector->virq < 0 ||
         kvm_irqchip_add_irqfd_notifier(kvm_state, &vector->interrupt,
                                        vector->virq) < 0) {
@@ -596,7 +596,7 @@ static int vfio_msix_vector_use(PCIDevice *pdev,
             vector->virq = -1;
         }
         qemu_set_fd_handler(event_notifier_get_fd(&vector->interrupt),
-                            vfio_msi_interrupt, NULL, vector);
+                            handler, NULL, vector);
     }
 
     /*
@@ -639,6 +639,12 @@ static int vfio_msix_vector_use(PCIDevice *pdev,
     return 0;
 }
 
+static int vfio_msix_vector_use(PCIDevice *pdev,
+                                unsigned int nr, MSIMessage msg)
+{
+    return vfio_msix_vector_do_use(pdev, nr, &msg, vfio_msi_interrupt);
+}
+
 static void vfio_msix_vector_release(PCIDevice *pdev, unsigned int nr)
 {
     VFIODevice *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
@@ -697,6 +703,22 @@ static void vfio_enable_msix(VFIODevice *vdev)
 
     vdev->interrupt = VFIO_INT_MSIX;
 
+    /*
+     * Some communication channels between VF & PF or PF & fw rely on the
+     * physical state of the device and expect that enabling MSI-X from the
+     * guest enables the same on the host.  When our guest is Linux, the
+     * guest driver call to pci_enable_msix() sets the enabling bit in the
+     * MSI-X capability, but leaves the vector table masked.  We therefore
+     * can't rely on a vector_use callback (from request_irq() in the guest)
+     * to switch the physical device into MSI-X mode because that may come a
+     * long time after pci_enable_msix().  This code enables vector 0 with
+     * triggering to userspace, then immediately release the vector, leaving
+     * the physical device with no vectors enabled, but MSI-X enabled, just
+     * like the guest view.
+     */
+    vfio_msix_vector_do_use(&vdev->pdev, 0, NULL, NULL);
+    vfio_msix_vector_release(&vdev->pdev, 0);
+
     if (msix_set_vector_notifiers(&vdev->pdev, vfio_msix_vector_use,
                                   vfio_msix_vector_release, NULL)) {
         error_report("vfio: msix_set_vector_notifiers failed\n");


More information about the Spice-commits mailing list