From: Thomas Gleixner tglx@linutronix.de
[ Upstream commit 3f35d2cf9fbc656db82579d849cc69c373b1ad0d ]
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 Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/pci/msi.c | 33 +++++++++++++++++++++++++++++++++ drivers/pci/pci.c | 5 ----- include/linux/pci.h | 3 ++- 3 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index e11530cb05699..696ce4a2c0d15 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -461,6 +461,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) { @@ -1028,6 +1053,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); @@ -1071,6 +1100,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 a101faf3e88a9..61fce23a467b9 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -2005,11 +2005,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 152a4d74f87f0..5f1f3de8c6b79 100644 --- 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;