dmaengine: idxd: fix sequence for pci driver remove() and shutdown()

->shutdown() call should only be responsible for quiescing the device.
Currently it is doing PCI device tear down. This causes issue when things
like MMIO mapping is removed while idxd_unregister_devices() will trigger
removal of idxd device sub-driver and still initiates MMIO writes to the
device. Another issue is with the unregistering of idxd 'struct device',
the memory context gets freed. So the teardown calls are accessing freed
memory and can cause kernel oops. Move all the teardown bits that doesn't
belong in shutdown to ->remove() call. Move unregistering of the idxd
conf_dev 'struct device' to after doing all the teardown to free all
the memory that's no longer needed.

Fixes: 47c16ac27d4c ("dmaengine: idxd: fix idxd conf_dev 'struct device' lifetime")
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Link: https://lore.kernel.org/r/162629983901.395844.17964803190905549615.stgit@djiang5-desk3.ch.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 7eac0d167bde6963540a575aac1f9b54fce611f7..75ac6a4bc9d1afc099848b9bb5a79b748a99e0e7 100644 (file)
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -754,32 +754,40 @@ static void idxd_shutdown(struct pci_dev *pdev)
        for (i = 0; i < msixcnt; i++) {
                irq_entry = &idxd->irq_entries[i];
                synchronize_irq(irq_entry->vector);
-               free_irq(irq_entry->vector, irq_entry);
                if (i == 0)
                        continue;
                idxd_flush_pending_llist(irq_entry);
                idxd_flush_work_list(irq_entry);
        }
-
-       idxd_msix_perm_clear(idxd);
-       idxd_release_int_handles(idxd);
-       pci_free_irq_vectors(pdev);
-       pci_iounmap(pdev, idxd->reg_base);
-       pci_disable_device(pdev);
-       destroy_workqueue(idxd->wq);
+       flush_workqueue(idxd->wq);
 }
 
 static void idxd_remove(struct pci_dev *pdev)
 {
        struct idxd_device *idxd = pci_get_drvdata(pdev);
+       struct idxd_irq_entry *irq_entry;
+       int msixcnt = pci_msix_vec_count(pdev);
+       int i;
 
        dev_dbg(&pdev->dev, "%s called\n", __func__);
        idxd_shutdown(pdev);
        if (device_pasid_enabled(idxd))
                idxd_disable_system_pasid(idxd);
        idxd_unregister_devices(idxd);
-       perfmon_pmu_remove(idxd);
+
+       for (i = 0; i < msixcnt; i++) {
+               irq_entry = &idxd->irq_entries[i];
+               free_irq(irq_entry->vector, irq_entry);
+       }
+       idxd_msix_perm_clear(idxd);
+       idxd_release_int_handles(idxd);
+       pci_free_irq_vectors(pdev);
+       pci_iounmap(pdev, idxd->reg_base);
        iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
+       pci_disable_device(pdev);
+       destroy_workqueue(idxd->wq);
+       perfmon_pmu_remove(idxd);
+       device_unregister(&idxd->conf_dev);
 }
 
 static struct pci_driver idxd_pci_driver = {

diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
index a193de32536d176d8c6b5ce802a6a9d6f225d55b..33c27df40f1e8c7966d02f1ca38d943403b2475f 100644 (file)
--- a/drivers/dma/idxd/sysfs.c
+++ b/drivers/dma/idxd/sysfs.c
@@ -1751,8 +1751,6 @@ void idxd_unregister_devices(struct idxd_device *idxd)
 
                device_unregister(&group->conf_dev);
        }
-
-       device_unregister(&idxd->conf_dev);
 }
 
 int idxd_register_bus_type(void)