diff mbox series

[v2] iommu/exynos: Fix set_platform_dma_ops() callback

Message ID 20230315232514.1046589-1-m.szyprowski@samsung.com (mailing list archive)
State Accepted
Commit f91bf3272a18356e8585f6bbba896d794632f2af
Headers show
Series [v2] iommu/exynos: Fix set_platform_dma_ops() callback | expand

Commit Message

Marek Szyprowski March 15, 2023, 11:25 p.m. UTC
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(-)

Comments

Sam Protsenko March 16, 2023, 7:20 p.m. UTC | #1
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
>
Jason Gunthorpe March 21, 2023, 2:41 p.m. UTC | #2
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
Marek Szyprowski March 21, 2023, 3:43 p.m. UTC | #3
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
Jason Gunthorpe March 21, 2023, 4:42 p.m. UTC | #4
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,
Marek Szyprowski March 24, 2023, 8:59 p.m. UTC | #5
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
Jason Gunthorpe March 24, 2023, 9:06 p.m. UTC | #6
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
Joerg Roedel March 28, 2023, 1:11 p.m. UTC | #7
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
Marek Szyprowski March 28, 2023, 1:29 p.m. UTC | #8
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
Joerg Roedel March 28, 2023, 1:35 p.m. UTC | #9
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.
diff mbox series

Patch

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,