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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Wed, Jun 01, 2011 at 10:51:03AM -0400, Konrad Rzeszutek Wilk wrote:
> 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)

Didn't even compile :-( How about this one instead (hadn't tested it yet):

diff --git a/drivers/pci/xen-pciback/pciback.h b/drivers/pci/xen-pciback/pciback.h
index 788c3ee..c665a4c 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_PCI_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..5c789df 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->msi_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_enabled = 0;
+	}
 	return 0;
 }
 #endif