* [PATCH v2] iommu/exynos: Fix set_platform_dma_ops() callback
[not found] <CGME20230315232523eucas1p2565ef5954e3b5b451803c6200c8bce32@eucas1p2.samsung.com>
@ 2023-03-15 23:25 ` Marek Szyprowski
2023-03-16 19:20 ` Sam Protsenko
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Marek Szyprowski @ 2023-03-15 23:25 UTC (permalink / raw)
To: iommu, linux-samsung-soc
Cc: Marek Szyprowski, Joerg Roedel, Will Deacon, Robin Murphy,
Krzysztof Kozlowski, Alim Akhtar, Lu Baolu, Jason Gunthorpe
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(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 483aaaeb6dae..1abd187c6075 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1415,23 +1415,26 @@ static struct iommu_device *exynos_iommu_probe_device(struct device *dev)
return &data->iommu;
}
-static void exynos_iommu_release_device(struct device *dev)
+static void exynos_iommu_set_platform_dma(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);
}
}
+}
+
+static void exynos_iommu_release_device(struct device *dev)
+{
+ struct exynos_iommu_owner *owner = dev_iommu_priv_get(dev);
+ struct sysmmu_drvdata *data;
+
+ exynos_iommu_set_platform_dma(dev);
list_for_each_entry(data, &owner->controllers, owner_node)
device_link_del(data->link);
@@ -1479,7 +1482,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,
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] iommu/exynos: Fix set_platform_dma_ops() callback
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-28 13:35 ` Joerg Roedel
2 siblings, 0 replies; 10+ messages in thread
From: Sam Protsenko @ 2023-03-16 19:20 UTC (permalink / raw)
To: Marek Szyprowski
Cc: iommu, linux-samsung-soc, Joerg Roedel, Will Deacon,
Robin Murphy, Krzysztof Kozlowski, Alim Akhtar, Lu Baolu,
Jason Gunthorpe
On Wed, 15 Mar 2023 at 18:25, Marek Szyprowski <m.szyprowski@samsung.com> 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>
> ---
Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> 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(-)
>
> diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> index 483aaaeb6dae..1abd187c6075 100644
> --- a/drivers/iommu/exynos-iommu.c
> +++ b/drivers/iommu/exynos-iommu.c
> @@ -1415,23 +1415,26 @@ static struct iommu_device *exynos_iommu_probe_device(struct device *dev)
> return &data->iommu;
> }
>
> -static void exynos_iommu_release_device(struct device *dev)
> +static void exynos_iommu_set_platform_dma(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);
> }
> }
> +}
> +
> +static void exynos_iommu_release_device(struct device *dev)
> +{
> + struct exynos_iommu_owner *owner = dev_iommu_priv_get(dev);
> + struct sysmmu_drvdata *data;
> +
> + exynos_iommu_set_platform_dma(dev);
>
> list_for_each_entry(data, &owner->controllers, owner_node)
> device_link_del(data->link);
> @@ -1479,7 +1482,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,
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] iommu/exynos: Fix set_platform_dma_ops() callback
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-28 13:35 ` Joerg Roedel
2 siblings, 1 reply; 10+ messages in thread
From: Jason Gunthorpe @ 2023-03-21 14:41 UTC (permalink / raw)
To: Marek Szyprowski
Cc: iommu, linux-samsung-soc, Joerg Roedel, Will Deacon,
Robin Murphy, Krzysztof Kozlowski, Alim Akhtar, Lu Baolu
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?
Jason
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] iommu/exynos: Fix set_platform_dma_ops() callback
2023-03-21 14:41 ` Jason Gunthorpe
@ 2023-03-21 15:43 ` Marek Szyprowski
2023-03-21 16:42 ` Jason Gunthorpe
0 siblings, 1 reply; 10+ messages in thread
From: Marek Szyprowski @ 2023-03-21 15:43 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: iommu, linux-samsung-soc, Joerg Roedel, Will Deacon,
Robin Murphy, Krzysztof Kozlowski, Alim Akhtar, Lu Baolu
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
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] iommu/exynos: Fix set_platform_dma_ops() callback
2023-03-21 15:43 ` Marek Szyprowski
@ 2023-03-21 16:42 ` Jason Gunthorpe
2023-03-24 20:59 ` Marek Szyprowski
0 siblings, 1 reply; 10+ messages in thread
From: Jason Gunthorpe @ 2023-03-21 16:42 UTC (permalink / raw)
To: Marek Szyprowski
Cc: iommu, linux-samsung-soc, Joerg Roedel, Will Deacon,
Robin Murphy, Krzysztof Kozlowski, Alim Akhtar, Lu Baolu
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,
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] iommu/exynos: Fix set_platform_dma_ops() callback
2023-03-21 16:42 ` Jason Gunthorpe
@ 2023-03-24 20:59 ` Marek Szyprowski
2023-03-24 21:06 ` Jason Gunthorpe
2023-03-28 13:11 ` Joerg Roedel
0 siblings, 2 replies; 10+ messages in thread
From: Marek Szyprowski @ 2023-03-24 20:59 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: iommu, linux-samsung-soc, Joerg Roedel, Will Deacon,
Robin Murphy, Krzysztof Kozlowski, Alim Akhtar, Lu Baolu
Hi Jason,
On 21.03.2023 17:42, Jason Gunthorpe wrote:
> 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?
Frankly speaking I would like to have a minimal fix (like my $subject
patch) applied to v6.3-rcX ASAP and then apply this identity domain
support on top of than for -next (6.4). I've checked and your proposed
patch works fine in my case, so You can send it as formal patch.
Joerg: is that okay for you?
> 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,
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] iommu/exynos: Fix set_platform_dma_ops() callback
2023-03-24 20:59 ` Marek Szyprowski
@ 2023-03-24 21:06 ` Jason Gunthorpe
2023-03-28 13:11 ` Joerg Roedel
1 sibling, 0 replies; 10+ messages in thread
From: Jason Gunthorpe @ 2023-03-24 21:06 UTC (permalink / raw)
To: Marek Szyprowski
Cc: iommu, linux-samsung-soc, Joerg Roedel, Will Deacon,
Robin Murphy, Krzysztof Kozlowski, Alim Akhtar, Lu Baolu
On Fri, Mar 24, 2023 at 09:59:27PM +0100, Marek Szyprowski wrote:
> Hi Jason,
>
> On 21.03.2023 17:42, Jason Gunthorpe wrote:
> > 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?
>
> Frankly speaking I would like to have a minimal fix (like my $subject
> patch) applied to v6.3-rcX ASAP and then apply this identity domain
> support on top of than for -next (6.4). I've checked and your proposed
> patch works fine in my case, so You can send it as formal patch.
I thought this was a cosmetic fix, do you have an actual bug here? If
so we should split it as you say, but you should describe the actual
bug in the commit message.
Jason
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] iommu/exynos: Fix set_platform_dma_ops() callback
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
1 sibling, 1 reply; 10+ messages in thread
From: Joerg Roedel @ 2023-03-28 13:11 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Jason Gunthorpe, iommu, linux-samsung-soc, Will Deacon,
Robin Murphy, Krzysztof Kozlowski, Alim Akhtar, Lu Baolu
On Fri, Mar 24, 2023 at 09:59:27PM +0100, Marek Szyprowski wrote:
> Frankly speaking I would like to have a minimal fix (like my $subject
> patch) applied to v6.3-rcX ASAP and then apply this identity domain
> support on top of than for -next (6.4). I've checked and your proposed
> patch works fine in my case, so You can send it as formal patch.
>
> Joerg: is that okay for you?
Agreed, a minimal fix for 6.3 would be great.
Regards,
Joerg
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] iommu/exynos: Fix set_platform_dma_ops() callback
2023-03-28 13:11 ` Joerg Roedel
@ 2023-03-28 13:29 ` Marek Szyprowski
0 siblings, 0 replies; 10+ messages in thread
From: Marek Szyprowski @ 2023-03-28 13:29 UTC (permalink / raw)
To: Joerg Roedel
Cc: Jason Gunthorpe, iommu, linux-samsung-soc, Will Deacon,
Robin Murphy, Krzysztof Kozlowski, Alim Akhtar, Lu Baolu
On 28.03.2023 15:11, Joerg Roedel wrote:
> On Fri, Mar 24, 2023 at 09:59:27PM +0100, Marek Szyprowski wrote:
>> Frankly speaking I would like to have a minimal fix (like my $subject
>> patch) applied to v6.3-rcX ASAP and then apply this identity domain
>> support on top of than for -next (6.4). I've checked and your proposed
>> patch works fine in my case, so You can send it as formal patch.
>>
>> Joerg: is that okay for you?
> Agreed, a minimal fix for 6.3 would be great.
Then this v2 is the minimal fix.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] iommu/exynos: Fix set_platform_dma_ops() callback
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-28 13:35 ` Joerg Roedel
2 siblings, 0 replies; 10+ messages in thread
From: Joerg Roedel @ 2023-03-28 13:35 UTC (permalink / raw)
To: Marek Szyprowski
Cc: iommu, linux-samsung-soc, Will Deacon, Robin Murphy,
Krzysztof Kozlowski, Alim Akhtar, Lu Baolu, Jason Gunthorpe
On Thu, Mar 16, 2023 at 12:25:14AM +0100, Marek Szyprowski wrote:
> drivers/iommu/exynos-iommu.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
Applied to iommu/fixes.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-03-28 13:35 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[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
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
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.