[PATCH] device-assignment: be more selective in interrupt disabling

* [PATCH] device-assignment: be more selective in interrupt disabling
@ 2010-06-17 17:51 Alex Williamson
  2010-06-17 19:07 ` Chris Wright
  2010-06-23 13:05 ` Avi Kivity
  0 siblings, 2 replies; 3+ messages in thread
From: Alex Williamson @ 2010-06-17 17:51 UTC (permalink / raw)
  To: kvm; +Cc: chrisw, alex.williamson

An 82576 physical function assigned to a Windows 7 guest currently
doesn't work because the driver seems to gratuitiously disable
MSI and MSIX interrupts.  When it does this, we blindly deassign
the current interrupt setup, leaving the device with no interrupts.
Instead let's only deassign the irq if we were previously using
MSI/MSIX or if we're going to start using MSI/MSIX.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

Note, this doesn't fix 82576 virtual functions on win7.  I can't find a
driver that even tries to claim the vf.  Please send a pointer if one exists.

 hw/device-assignment.c |   34 +++++++++++++++++++++++++---------
 1 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index ba02157..85cd414 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -1031,13 +1031,20 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev, unsigned int ctrl_pos)
         calc_assigned_dev_id(assigned_dev->h_segnr, assigned_dev->h_busnr,
                 (uint8_t)assigned_dev->h_devfn);
 
-    if (assigned_dev->irq_requested_type) {
-	    assigned_irq_data.flags = assigned_dev->irq_requested_type;
-	    free_dev_irq_entries(assigned_dev);
-	    r = kvm_deassign_irq(kvm_context, &assigned_irq_data);
-	    /* -ENXIO means no assigned irq */
-	    if (r && r != -ENXIO)
-		    perror("assigned_dev_update_msi: deassign irq");
+    /* Some guests gratuitously disable MSI even if they're not using it,
+     * try to catch this by only deassigning irqs if the guest is using
+     * MSI or intends to start. */
+    if ((assigned_dev->irq_requested_type & KVM_DEV_IRQ_GUEST_MSI) ||
+        (ctrl_byte & PCI_MSI_FLAGS_ENABLE)) {
+
+        assigned_irq_data.flags = assigned_dev->irq_requested_type;
+        free_dev_irq_entries(assigned_dev);
+        r = kvm_deassign_irq(kvm_context, &assigned_irq_data);
+        /* -ENXIO means no assigned irq */
+        if (r && r != -ENXIO)
+            perror("assigned_dev_update_msi: deassign irq");
+
+        assigned_irq_data.flags = 0;
     }
 
     if (ctrl_byte & PCI_MSI_FLAGS_ENABLE) {
@@ -1188,17 +1195,26 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev, unsigned int ctrl_pos)
             calc_assigned_dev_id(assigned_dev->h_segnr, assigned_dev->h_busnr,
                     (uint8_t)assigned_dev->h_devfn);
 
-    if (assigned_dev->irq_requested_type) {
+    /* Some guests gratuitously disable MSIX even if they're not using it,
+     * try to catch this by only deassigning irqs if the guest is using
+     * MSIX or intends to start. */
+    if ((assigned_dev->irq_requested_type & KVM_DEV_IRQ_GUEST_MSIX) ||
+        (*ctrl_word & PCI_MSIX_ENABLE)) {
+
         assigned_irq_data.flags = assigned_dev->irq_requested_type;
         free_dev_irq_entries(assigned_dev);
         r = kvm_deassign_irq(kvm_context, &assigned_irq_data);
         /* -ENXIO means no assigned irq */
         if (r && r != -ENXIO)
             perror("assigned_dev_update_msix: deassign irq");
+
+        assigned_irq_data.flags = 0;
     }
-    assigned_irq_data.flags = KVM_DEV_IRQ_HOST_MSIX | KVM_DEV_IRQ_GUEST_MSIX;
 
     if (*ctrl_word & PCI_MSIX_ENABLE) {
+        assigned_irq_data.flags = KVM_DEV_IRQ_HOST_MSIX |
+                                  KVM_DEV_IRQ_GUEST_MSIX;
+
         if (assigned_dev_update_msix_mmio(pci_dev) < 0) {
             perror("assigned_dev_update_msix_mmio");
             return;


^ permalink raw reply related	[flat|nested] 3+ messages in thread