* [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 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).