Re: [Xen-devel] [RFC PATCH linux-2.6.18-xen] pciback: clean up (MSI-X vec, entrynr) list when resetting PCI device

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Wed, Jun 01, 2011 at 12:05:44PM +0200, Laszlo Ersek wrote:
> Hi,
> 
> this is more of an RFC than a patch now for linux-2.6.18-xen. Describing the 
> situation captured in RHBZ#688673, in a nutshell:
> 
> - let's say we have an Intel 82576 card (igb),
> - the card exports virtual functions (igbvf),
> - one virtual function is passed through to a PV guest,
> - the igbvf driver, co-operating with pcifront, pciback, and the hypervisor, 
> sets up three MSI-X vectors for the virtual function PCI device,
> - when the domU is shut down, the MSI-X vectors are "leaked" in dom0, because 
> nobody ever reaches dom0's pci_disable_msix() / msi_remove_pci_irq_vectors() 
> during shutdown,

Could you keep a state in the dom0 of the fact that msi/msi-x were enabled, and 
use those
(instead of dev->msi_enabled) to call pci_disable_msi(). And you could also 
modify the
msi_enabled to be set to 1 before you make the pci_disable_msi() call?

And naturally if the frontend makes a PV call to disable your MSI device, then 
you
set internally your msi_enabled to zero.

Something like this patch (based off stable/2.6.39.x)

diff --git a/drivers/pci/xen-pciback/pciback.h 
b/drivers/pci/xen-pciback/pciback.h
index 788c3ee..69f0961 100644
--- a/drivers/pci/xen-pciback/pciback.h
+++ b/drivers/pci/xen-pciback/pciback.h
@@ -50,11 +50,39 @@ struct xen_pcibk_dev_data {
        unsigned int enable_intx:1;
        unsigned int isr_on:1; /* Whether the IRQ handler is installed. */
        unsigned int ack_intr:1; /* .. and ACK-ing */
+       unsigned int msi_enabled:1;
+       unsigned int msix_enabled:1;
        unsigned long handled;
        unsigned int irq; /* Saved in case device transitions to MSI/MSI-X */
        char irq_name[0]; /* xen-pcibk[000:04:00.0] */
 };
 
+#ifdef CONFIG_MSI
+static inline bool xen_pcibk_force_msi_disable(struct pci_dev *dev)
+{
+       struct xen_pcibk_dev_data *dev_data;
+
+       dev_data = pci_get_drvdata(dev);
+       if (!dev_data)
+               return false;
+
+       if (dev_data->msi_enabled && !dev->msi_enabled)
+               return true;
+       return false;
+}
+static inline bool xen_pcibk_force_msix_disable(struct pci_dev *dev)
+{
+       struct xen_pcibk_dev_data *dev_data;
+
+       dev_data = pci_get_drvdata(dev);
+       if (!dev_data)
+               return false;
+
+       if (dev_data->msix_enabled && !dev->msix_enabled)
+               return true;
+       return false;
+}
+#endif
 /* Used by XenBus and xen_pcibk_ops.c */
 extern wait_queue_head_t xen_pcibk_aer_wait_queue;
 extern struct workqueue_struct *xen_pcibk_wq;
diff --git a/drivers/pci/xen-pciback/pciback_ops.c 
b/drivers/pci/xen-pciback/pciback_ops.c
index 8c95c34..b37935b 100644
--- a/drivers/pci/xen-pciback/pciback_ops.c
+++ b/drivers/pci/xen-pciback/pciback_ops.c
@@ -109,6 +109,11 @@ void xen_pcibk_reset_device(struct pci_dev *dev)
 #ifdef CONFIG_PCI_MSI
                /* The guest could have been abruptly killed without
                 * disabling MSI/MSI-X interrupts.*/
+
+               if (xen_pcibk_force_msi_disable(dev))
+                       dev->msi_enabled = 1;
+               if (xen_pcibk_force_msix_disable(dev))
+                       dev->msix_enabled = 1;
                if (dev->msix_enabled)
                        pci_disable_msix(dev);
                if (dev->msi_enabled)
@@ -160,9 +165,10 @@ int xen_pcibk_enable_msi(struct xen_pcibk_device *pdev,
                        op->value);
 
        dev_data = pci_get_drvdata(dev);
-       if (dev_data)
+       if (dev_data) {
                dev_data->ack_intr = 0;
-
+               dev_data->msix_enabled = 1;
+       }
        return 0;
 }
 
@@ -182,8 +188,10 @@ int xen_pcibk_disable_msi(struct xen_pcibk_device *pdev,
                printk(KERN_DEBUG DRV_NAME ": %s: MSI: %d\n", pci_name(dev),
                        op->value);
        dev_data = pci_get_drvdata(dev);
-       if (dev_data)
+       if (dev_data) {
                dev_data->ack_intr = 1;
+               dev_data->msi_enabled = 0;
+       }
        return 0;
 }
 
@@ -232,9 +240,10 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev,
 
        op->value = result;
        dev_data = pci_get_drvdata(dev);
-       if (dev_data)
+       if (dev_data) {
                dev_data->ack_intr = 0;
-
+               dev_data->msix_enabled = 1;
+       }
        return result;
 }
 
@@ -257,8 +266,10 @@ int xen_pcibk_disable_msix(struct xen_pcibk_device *pdev,
                printk(KERN_DEBUG DRV_NAME ": %s: MSI-X: %d\n", pci_name(dev),
                        op->value);
        dev_data = pci_get_drvdata(dev);
-       if (dev_data)
+       if (dev_data) {
                dev_data->ack_intr = 1;
+               dev_data->msix_disabled = 0;
+       }
        return 0;
 }
 #endif

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel