PCI/MSI: Decouple MSI[-X] disable from pcim_release()

The MSI core will introduce runtime allocation of MSI related data. This
data will be devres managed and has to be set up before enabling
PCI/MSI[-X]. This would introduce an ordering issue vs. pcim_release().

The setup order is:

   pcim_enable_device()
	devres_alloc(pcim_release...);
	...
	pci_irq_alloc()
	  msi_setup_device_data()
	     devres_alloc(msi_device_data_release, ...)

and once the device is released these release functions are invoked in the
opposite order:

    msi_device_data_release()
    ...
    pcim_release()
       pci_disable_msi[x]()

which is obviously wrong, because pci_disable_msi[x]() requires the MSI
data to be available to tear down the MSI[-X] interrupts.

Remove the MSI[-X] teardown from pcim_release() and add an explicit action
to be installed on the attempt of enabling PCI/MSI[-X].

This allows the MSI core data allocation to be ordered correctly in a
subsequent step.

Reported-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Michael Kelley <mikelley@microsoft.com>
Tested-by: Nishanth Menon <nm@ti.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Link: https://lore.kernel.org/r/87tuf9rdoj.ffs@tglx

diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c
index 5af8d9b6d4242f7de1ade56b48eb03c0784187d6..358b63e507e9602b813eefe23c201d7e9689b32a 100644 (file)
--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -341,6 +341,31 @@ void pci_restore_msi_state(struct pci_dev *dev)
 }
 EXPORT_SYMBOL_GPL(pci_restore_msi_state);
 
+static void pcim_msi_release(void *pcidev)
+{
+       struct pci_dev *dev = pcidev;
+
+       dev->is_msi_managed = false;
+       pci_free_irq_vectors(dev);
+}
+
+/*
+ * Needs to be separate from pcim_release to prevent an ordering problem
+ * vs. msi_device_data_release() in the MSI core code.
+ */
+static int pcim_setup_msi_release(struct pci_dev *dev)
+{
+       int ret;
+
+       if (!pci_is_managed(dev) || dev->is_msi_managed)
+               return 0;
+
+       ret = devm_add_action(&dev->dev, pcim_msi_release, dev);
+       if (!ret)
+               dev->is_msi_managed = true;
+       return ret;
+}
+
 static struct msi_desc *
 msi_setup_entry(struct pci_dev *dev, int nvec, struct irq_affinity *affd)
 {
@@ -884,6 +909,10 @@ static int __pci_enable_msi_range(struct pci_dev *dev, int minvec, int maxvec,
        if (nvec > maxvec)
                nvec = maxvec;
 
+       rc = pcim_setup_msi_release(dev);
+       if (rc)
+               return rc;
+
        for (;;) {
                if (affd) {
                        nvec = irq_calc_affinity_vectors(minvec, nvec, affd);
@@ -927,6 +956,10 @@ static int __pci_enable_msix_range(struct pci_dev *dev,
        if (WARN_ON_ONCE(dev->msix_enabled))
                return -EINVAL;
 
+       rc = pcim_setup_msi_release(dev);
+       if (rc)
+               return rc;
+
        for (;;) {
                if (affd) {
                        nvec = irq_calc_affinity_vectors(minvec, nvec, affd);

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 3d2fb394986a4eba685fce6ae2726798f5acea19..f3f606c232a8ab6c966cabde64a0d901a5426bfb 100644 (file)
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2024,11 +2024,6 @@ static void pcim_release(struct device *gendev, void *res)
        struct pci_devres *this = res;
        int i;
 
-       if (dev->msi_enabled)
-               pci_disable_msi(dev);
-       if (dev->msix_enabled)
-               pci_disable_msix(dev);
-
        for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
                if (this->region_mask & (1 << i))
                        pci_release_region(dev, i);

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 5cc46baef5190df8edf8fb83a82a3ddb911eaaa6..a09736d3e05e8bebcaf3c4e748f3ea10a45ce3f5 100644 (file)
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -425,7 +425,8 @@ struct pci_dev {
        unsigned int    ats_enabled:1;          /* Address Translation Svc */
        unsigned int    pasid_enabled:1;        /* Process Address Space ID */
        unsigned int    pri_enabled:1;          /* Page Request Interface */
-       unsigned int    is_managed:1;
+       unsigned int    is_managed:1;           /* Managed via devres */
+       unsigned int    is_msi_managed:1;       /* MSI release via devres installed */
        unsigned int    needs_freset:1;         /* Requires fundamental reset */
        unsigned int    state_saved:1;
        unsigned int    is_physfn:1;