From: Jason Gunthorpe <jgg@ziepe.ca>
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: iommu@lists.linux.dev, linux-samsung-soc@vger.kernel.org,
Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
Robin Murphy <robin.murphy@arm.com>,
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
Alim Akhtar <alim.akhtar@samsung.com>,
Lu Baolu <baolu.lu@linux.intel.com>
Subject: Re: [PATCH v2] iommu/exynos: Fix set_platform_dma_ops() callback
Date: Tue, 21 Mar 2023 13:42:39 -0300 [thread overview]
Message-ID: <ZBnef7g7GCxogPNz@ziepe.ca> (raw)
In-Reply-To: <da174ce4-b922-21d0-62da-b69798a51371@samsung.com>
On Tue, Mar 21, 2023 at 04:43:42PM +0100, Marek Szyprowski wrote:
>
> On 21.03.2023 15:41, Jason Gunthorpe wrote:
> > On Thu, Mar 16, 2023 at 12:25:14AM +0100, Marek Szyprowski wrote:
> >> There are some subtle differences between release_device() and
> >> set_platform_dma_ops() callbacks, so separate those two callbacks. Device
> >> links should be removed only in release_device(), because they were
> >> created in probe_device() on purpose and they are needed for proper
> >> Exynos IOMMU driver operation. While fixing this, remove the conditional
> >> code as it is not really needed.
> >>
> >> Reported-by: Jason Gunthorpe <jgg@ziepe.ca>
> >> Fixes: 189d496b48b1 ("iommu/exynos: Add missing set_platform_dma_ops callback")
> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >> ---
> >> v2:
> >> - keep set_platform_dma_ops only on ARM 32bit
> >>
> >> Some more background why set_platform_dma_ops is needed on ARM 32bit is
> >> available here:
> >> https://lore.kernel.org/all/9a12fcac-c347-5d81-acef-1124c50d0c37@arm.com/
> >> ---
> >> drivers/iommu/exynos-iommu.c | 17 ++++++++++-------
> >> 1 file changed, 10 insertions(+), 7 deletions(-)
> > It seems OK, but do you know what state the device is left in after
> > exynos_iommu_detach_device ? Ie is it blocking or identity?
>
> identity
Can you do this cleanup like this instead?
Jason
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 483aaaeb6daeac..2c2b5cba191459 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -24,6 +24,7 @@
typedef u32 sysmmu_iova_t;
typedef u32 sysmmu_pte_t;
+static struct iommu_domain exynos_identity_domain;
/* We do not consider super section mapping (16MB) */
#define SECT_ORDER 20
@@ -900,6 +901,9 @@ static struct iommu_domain *exynos_iommu_domain_alloc(unsigned type)
dma_addr_t handle;
int i;
+ if (type == IOMMU_DOMAIN_IDENTITY)
+ return &exynos_identity_domain;
+
/* Check if correct PTE offsets are initialized */
BUG_ON(PG_ENT_SHIFT < 0 || !dma_dev);
@@ -988,17 +992,22 @@ static void exynos_iommu_domain_free(struct iommu_domain *iommu_domain)
kfree(domain);
}
-static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain,
- struct device *dev)
+static int exynos_iommu_identity_attach(struct iommu_domain *identity_domain,
+ struct device *dev)
{
- struct exynos_iommu_domain *domain = to_exynos_domain(iommu_domain);
struct exynos_iommu_owner *owner = dev_iommu_priv_get(dev);
- phys_addr_t pagetable = virt_to_phys(domain->pgtable);
+ struct exynos_iommu_domain *domain;
+ phys_addr_t pagetable;
struct sysmmu_drvdata *data, *next;
unsigned long flags;
- if (!has_sysmmu(dev) || owner->domain != iommu_domain)
- return;
+ if (!owner)
+ return -ENODEV;
+ if (owner->domain == identity_domain)
+ return 0;
+
+ domain = to_exynos_domain(owner->domain);
+ pagetable = virt_to_phys(domain->pgtable);
mutex_lock(&owner->rpm_lock);
@@ -1017,15 +1026,32 @@ static void exynos_iommu_detach_device(struct iommu_domain *iommu_domain,
list_del_init(&data->domain_node);
spin_unlock(&data->lock);
}
- owner->domain = NULL;
+ owner->domain = identity_domain;
spin_unlock_irqrestore(&domain->lock, flags);
mutex_unlock(&owner->rpm_lock);
dev_dbg(dev, "%s: Detached IOMMU with pgtable %pa\n", __func__,
&pagetable);
+ return 0;
}
+static struct iommu_domain_ops exynos_identity_ops = {
+ .attach_dev = exynos_iommu_identity_attach,
+};
+
+static struct iommu_domain exynos_identity_domain = {
+ .type = IOMMU_DOMAIN_IDENTITY,
+ .ops = &exynos_identity_ops,
+};
+
+#ifdef CONFIG_ARM
+static void exynos_iommu_set_platform_dma(struct device *dev)
+{
+ WARN_ON(exynos_iommu_identity_attach(&exynos_identity_domain, dev));
+}
+#endif
+
static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
struct device *dev)
{
@@ -1034,12 +1060,11 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
struct sysmmu_drvdata *data;
phys_addr_t pagetable = virt_to_phys(domain->pgtable);
unsigned long flags;
+ int err;
- if (!has_sysmmu(dev))
- return -ENODEV;
-
- if (owner->domain)
- exynos_iommu_detach_device(owner->domain, dev);
+ err = exynos_iommu_identity_attach(&exynos_identity_domain, dev);
+ if (err)
+ return err;
mutex_lock(&owner->rpm_lock);
@@ -1420,18 +1445,7 @@ static void exynos_iommu_release_device(struct device *dev)
struct exynos_iommu_owner *owner = dev_iommu_priv_get(dev);
struct sysmmu_drvdata *data;
- if (owner->domain) {
- struct iommu_group *group = iommu_group_get(dev);
-
- if (group) {
-#ifndef CONFIG_ARM
- WARN_ON(owner->domain !=
- iommu_group_default_domain(group));
-#endif
- exynos_iommu_detach_device(owner->domain, dev);
- iommu_group_put(group);
- }
- }
+ WARN_ON(exynos_iommu_identity_attach(&exynos_identity_domain, dev));
list_for_each_entry(data, &owner->controllers, owner_node)
device_link_del(data->link);
@@ -1462,6 +1476,7 @@ static int exynos_iommu_of_xlate(struct device *dev,
INIT_LIST_HEAD(&owner->controllers);
mutex_init(&owner->rpm_lock);
+ owner->domain = &exynos_identity_domain;
dev_iommu_priv_set(dev, owner);
}
@@ -1479,7 +1494,7 @@ static const struct iommu_ops exynos_iommu_ops = {
.domain_alloc = exynos_iommu_domain_alloc,
.device_group = generic_device_group,
#ifdef CONFIG_ARM
- .set_platform_dma_ops = exynos_iommu_release_device,
+ .set_platform_dma_ops = exynos_iommu_set_platform_dma,
#endif
.probe_device = exynos_iommu_probe_device,
.release_device = exynos_iommu_release_device,
next prev parent reply other threads:[~2023-03-21 16:43 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20230315232523eucas1p2565ef5954e3b5b451803c6200c8bce32@eucas1p2.samsung.com>
2023-03-15 23:25 ` [PATCH v2] iommu/exynos: Fix set_platform_dma_ops() callback Marek Szyprowski
2023-03-16 19:20 ` Sam Protsenko
2023-03-21 14:41 ` Jason Gunthorpe
2023-03-21 15:43 ` Marek Szyprowski
2023-03-21 16:42 ` Jason Gunthorpe [this message]
2023-03-24 20:59 ` Marek Szyprowski
2023-03-24 21:06 ` Jason Gunthorpe
2023-03-28 13:11 ` Joerg Roedel
2023-03-28 13:29 ` Marek Szyprowski
2023-03-28 13:35 ` Joerg Roedel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZBnef7g7GCxogPNz@ziepe.ca \
--to=jgg@ziepe.ca \
--cc=alim.akhtar@samsung.com \
--cc=baolu.lu@linux.intel.com \
--cc=iommu@lists.linux.dev \
--cc=joro@8bytes.org \
--cc=krzysztof.kozlowski@linaro.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=robin.murphy@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).