Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i0ad843c9:Fastmail
From:   Samuel Holland <samuel@sholland.org>
To:     Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
        Robin Murphy <robin.murphy@arm.com>,
        Chen-Yu Tsai <wens@csie.org>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Rob Herring <robh+dt@kernel.org>
Cc:     iommu@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
        linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org, Maxime Ripard <mripard@kernel.org>,
        Samuel Holland <samuel@sholland.org>
Subject: [PATCH v2 2/6] iommu/sun50i: Track masters attached to the domain
Date:   Mon,  2 Jan 2023 19:08:59 -0600
Message-Id: <20230103010903.11181-3-samuel@sholland.org>
In-Reply-To: <20230103010903.11181-1-samuel@sholland.org>
References: <20230103010903.11181-1-samuel@sholland.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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(-)

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;
 }
-- 
2.37.4