Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain

From: Jason Gunthorpe via iommu <iommu@lists.linux-foundation.org>
To: Baolu Lu <baolu.lu@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>,
	Robin Murphy <robin.murphy@arm.com>,
	Qian Cai <quic_qiancai@quicinc.com>,
	iommu@lists.linux-foundation.org, Will Deacon <will@kernel.org>
Subject: Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain
Date: Wed, 4 May 2022 11:38:33 -0300	[thread overview]
Message-ID: <20220504143833.GH49344@nvidia.com> (raw)
In-Reply-To: <8449a6fd-32f1-7cd3-6a4f-31f71df606a1@linux.intel.com>

On Wed, May 04, 2022 at 10:35:12PM +0800, Baolu Lu wrote:

> With below additional changes, this patch works on my Intel test
> machine.

Thanks!

> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 513da82f2ed1..7c415e9b6906 100644
> +++ b/drivers/iommu/iommu.c
> @@ -2063,7 +2063,8 @@ static int __iommu_attach_group(struct iommu_domain
> *domain,
>  {
>         int ret;
> 
> -       if (group->domain && group->domain != group->default_domain)
> +       if (group->domain && group->domain != group->default_domain &&
> +           group->domain != group->blocking_domain)
>                 return -EBUSY;
> 
>         ret = __iommu_group_for_each_dev(group, domain,
> @@ -2125,7 +2126,7 @@ static int __iommu_group_attach_domain(struct
> iommu_group *group,
>          * Note that this is called in error unwind paths, attaching to a
>          * domain that has already been attached cannot fail.
>          */
> -       ret = __iommu_group_for_each_dev(group, group->default_domain,
> +       ret = __iommu_group_for_each_dev(group, new_domain,
>                                          iommu_group_do_attach_device);
>         if (ret)
>                 return ret;

Done

> @@ -3180,7 +3181,9 @@ int iommu_group_claim_dma_owner(struct iommu_group
> *group, void *owner)
>                 ret = -EPERM;
>                 goto unlock_out;
>         } else {
> -               if (group->domain && group->domain != group->default_domain)
> {
> +               if (group->domain &&
> +                   group->domain != group->default_domain &&
> +                   group->domain != group->blocking_domain) {

Why do we need this hunk? This is just trying to check if there is
some conflict with some other domain attach, group->domain can never
be blocking_domain here.

Jason
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu