dmaengine.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dmaengine: idxd: fix sequence for pci driver remove() and shutdown()
@ 2021-07-14 21:57 Dave Jiang
  2021-07-20 13:48 ` Vinod Koul
  0 siblings, 1 reply; 2+ messages in thread
From: Dave Jiang @ 2021-07-14 21:57 UTC (permalink / raw)
  To: vkoul; +Cc: dmaengine, dan.j.williams

->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>
---
 drivers/dma/idxd/init.c  |   26 +++++++++++++++++---------
 drivers/dma/idxd/sysfs.c |    2 --
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 4e32a4dcc3ab..c0f4c0422f32 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -760,32 +760,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 0460d58e3941..bb4df63906a7 100644
--- a/drivers/dma/idxd/sysfs.c
+++ b/drivers/dma/idxd/sysfs.c
@@ -1744,8 +1744,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)



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] dmaengine: idxd: fix sequence for pci driver remove() and shutdown()
  2021-07-14 21:57 [PATCH] dmaengine: idxd: fix sequence for pci driver remove() and shutdown() Dave Jiang
@ 2021-07-20 13:48 ` Vinod Koul
  0 siblings, 0 replies; 2+ messages in thread
From: Vinod Koul @ 2021-07-20 13:48 UTC (permalink / raw)
  To: Dave Jiang; +Cc: dmaengine, dan.j.williams

On 14-07-21, 14:57, Dave Jiang wrote:
> ->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.

Applied to fixes, thanks

-- 
~Vinod

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-07-20 13:53 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-14 21:57 [PATCH] dmaengine: idxd: fix sequence for pci driver remove() and shutdown() Dave Jiang
2021-07-20 13:48 ` Vinod Koul

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox