diff mbox series

[v2,2/6] iommu/sun50i: Track masters attached to the domain

Message ID 20230103010903.11181-3-samuel@sholland.org (mailing list archive)
State New, archived
Headers show
Series iommu/sun50i: Allwinner D1 support | expand

Commit Message

Samuel Holland Jan. 3, 2023, 1:08 a.m. UTC 
The refcount logic here is broken. The refcount is initialized to 1 with
no devices attached, so it will be decremented back to 1 when the last
device is detached. However, refcount_dec_and_test() returns true only
when the refcount gets decremented to zero, so the domain will never be
cleaned up, and the hardware will never be disabled.

Replace the refcount with a mask of masters attached to the domain. The
domain can be cleaned up when the last master detaches. This mask is
also necessary to correctly set the bypass register.

Fixes: 4100b8c229b3 ("iommu: Add Allwinner H6 IOMMU driver")
Signed-off-by: Samuel Holland <samuel@sholland.org>
---

Changes in v2:
 - New patch for v2

 drivers/iommu/sun50i-iommu.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

Comments

Jernej Škrabec Jan. 4, 2023, 10:04 p.m. UTC | #1 
Dne torek, 03. januar 2023 ob 02:08:59 CET je Samuel Holland napisal(a):
> The refcount logic here is broken. The refcount is initialized to 1 with
> no devices attached, so it will be decremented back to 1 when the last
> device is detached. However, refcount_dec_and_test() returns true only
> when the refcount gets decremented to zero, so the domain will never be
> cleaned up, and the hardware will never be disabled.
> 
> Replace the refcount with a mask of masters attached to the domain. The
> domain can be cleaned up when the last master detaches. This mask is
> also necessary to correctly set the bypass register.
> 
> Fixes: 4100b8c229b3 ("iommu: Add Allwinner H6 IOMMU driver")
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej
diff mbox series

Patch

diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index 5b585eace3d4..3757d5a18318 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -114,8 +114,8 @@  struct sun50i_iommu {
 struct sun50i_iommu_domain {
 	struct iommu_domain domain;
 
-	/* Number of devices attached to the domain */
-	refcount_t refcnt;
+	/* Mask of masters attached to this domain */
+	atomic_t masters;
 
 	/* L1 Page Table */
 	u32 *dt;
@@ -684,8 +684,6 @@  static struct iommu_domain *sun50i_iommu_domain_alloc(unsigned type)
 	if (!sun50i_domain->dt)
 		goto err_free_domain;
 
-	refcount_set(&sun50i_domain->refcnt, 1);
-
 	sun50i_domain->domain.geometry.aperture_start = 0;
 	sun50i_domain->domain.geometry.aperture_end = DMA_BIT_MASK(32);
 	sun50i_domain->domain.geometry.force_aperture = true;
@@ -760,23 +758,27 @@  static void sun50i_iommu_detach_domain(struct sun50i_iommu *iommu,
 static void sun50i_iommu_detach_device(struct iommu_domain *domain,
 				       struct device *dev)
 {
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
 	struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain);
 	struct sun50i_iommu *iommu = dev_iommu_priv_get(dev);
+	int masters = 0;
 
 	dev_dbg(dev, "Detaching from IOMMU domain\n");
 
-	if (iommu->domain != domain)
-		return;
+	for (unsigned int i = 0; i < fwspec->num_ids; i++)
+		masters |= BIT(fwspec->ids[i]);
 
-	if (refcount_dec_and_test(&sun50i_domain->refcnt))
+	if (atomic_fetch_andnot(masters, &sun50i_domain->masters) == masters)
 		sun50i_iommu_detach_domain(iommu, sun50i_domain);
 }
 
 static int sun50i_iommu_attach_device(struct iommu_domain *domain,
 				      struct device *dev)
 {
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
 	struct sun50i_iommu_domain *sun50i_domain = to_sun50i_domain(domain);
 	struct sun50i_iommu *iommu;
+	int masters = 0;
 
 	iommu = sun50i_iommu_from_dev(dev);
 	if (!iommu)
@@ -784,15 +786,11 @@  static int sun50i_iommu_attach_device(struct iommu_domain *domain,
 
 	dev_dbg(dev, "Attaching to IOMMU domain\n");
 
-	refcount_inc(&sun50i_domain->refcnt);
-
-	if (iommu->domain == domain)
-		return 0;
-
-	if (iommu->domain)
-		sun50i_iommu_detach_device(iommu->domain, dev);
+	for (unsigned int i = 0; i < fwspec->num_ids; i++)
+		masters |= BIT(fwspec->ids[i]);
 
-	sun50i_iommu_attach_domain(iommu, sun50i_domain);
+	if (atomic_fetch_or(masters, &sun50i_domain->masters) == 0)
+		sun50i_iommu_attach_domain(iommu, sun50i_domain);
 
 	return 0;
 }