Linux IOMMU Development
 help / color / mirror / Atom feed
From: Vasant Hegde <vasant.hegde@amd.com>
To: iommu@lists.linux.dev, joro@8bytes.org
Cc: robin.murphy@arm.com, will@kernel.org, suravee.suthikulpanit@amd.com
Subject: Re: [PATCH 1/3] iommu: Attach device group to old domain in error path
Date: Wed, 1 Feb 2023 10:49:54 +0530	[thread overview]
Message-ID: <4750852f-53ea-719a-cc03-e2da34e5782d@amd.com> (raw)
In-Reply-To: <20230113135956.5788-1-vasant.hegde@amd.com>

Hi Joerg,

Did you get chance to look into this series?

-Vasant


On 1/13/2023 7:29 PM, Vasant Hegde wrote:
> iommu_attach_group() attaches all devices in a group to domain and then
> sets group domain (group->domain). Current code (__iommu_attach_group())
> does not handle error path. This creates problem as devices to domain
> attachment is in inconsistent state.
> 
> To recover from this situation, we need force attaching all devices back
> to the old domain. This patch introduces `force` parameter to
> __iommu_group_set_domain() so that we can attach devices back to old
> domain.
> 
> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
> ---
>  drivers/iommu/iommu.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index de91dd88705b..e58683cd3bf7 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -95,7 +95,8 @@ static int __iommu_attach_device(struct iommu_domain *domain,
>  static int __iommu_attach_group(struct iommu_domain *domain,
>  				struct iommu_group *group);
>  static int __iommu_group_set_domain(struct iommu_group *group,
> -				    struct iommu_domain *new_domain);
> +				    struct iommu_domain *new_domain,
> +				    bool force);
>  static int iommu_create_device_direct_mappings(struct iommu_group *group,
>  					       struct device *dev);
>  static struct iommu_group *iommu_group_get_for_dev(struct device *dev);
> @@ -1974,7 +1975,7 @@ static void __iommu_group_set_core_domain(struct iommu_group *group)
>  	else
>  		new_domain = group->default_domain;
>  
> -	ret = __iommu_group_set_domain(group, new_domain);
> +	ret = __iommu_group_set_domain(group, new_domain, false);
>  	WARN(ret, "iommu driver failed to attach the default/blocking domain");
>  }
>  
> @@ -2124,8 +2125,16 @@ static int __iommu_attach_group(struct iommu_domain *domain,
>  
>  	ret = __iommu_group_for_each_dev(group, domain,
>  					 iommu_group_do_attach_device);
> -	if (ret == 0)
> +	if (ret == 0) {
>  		group->domain = domain;
> +	} else {
> +		/*
> +		 * To recover from the case when certain device within the
> +		 * group fails to attach to the new domain, we need force
> +		 * attaching all devices back to the old domain.
> +		 */
> +		__iommu_group_set_domain(group, group->domain, true);
> +	}
>  
>  	return ret;
>  }
> @@ -2164,11 +2173,12 @@ static int iommu_group_do_detach_device(struct device *dev, void *data)
>  }
>  
>  static int __iommu_group_set_domain(struct iommu_group *group,
> -				    struct iommu_domain *new_domain)
> +				    struct iommu_domain *new_domain,
> +				    bool force)
>  {
>  	int ret;
>  
> -	if (group->domain == new_domain)
> +	if (!force && group->domain == new_domain)
>  		return 0;
>  
>  	/*
> @@ -3135,7 +3145,7 @@ static int __iommu_take_dma_ownership(struct iommu_group *group, void *owner)
>  	ret = __iommu_group_alloc_blocking_domain(group);
>  	if (ret)
>  		return ret;
> -	ret = __iommu_group_set_domain(group, group->blocking_domain);
> +	ret = __iommu_group_set_domain(group, group->blocking_domain, false);
>  	if (ret)
>  		return ret;
>  
> @@ -3222,7 +3232,7 @@ static void __iommu_release_dma_ownership(struct iommu_group *group)
>  
>  	group->owner_cnt = 0;
>  	group->owner = NULL;
> -	ret = __iommu_group_set_domain(group, group->default_domain);
> +	ret = __iommu_group_set_domain(group, group->default_domain, false);
>  	WARN(ret, "iommu driver failed to attach the default domain");
>  }
>  


      parent reply	other threads:[~2023-02-01  5:20 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-13 13:59 [PATCH 1/3] iommu: Attach device group to old domain in error path Vasant Hegde
2023-01-13 13:59 ` [PATCH 2/3] iommu/amd: Skip attach device domain is same as new domain Vasant Hegde
2023-01-13 13:59 ` [PATCH 3/3] iommu/amd: Call BUG_ON in page fault hanlder path if domain is NULL Vasant Hegde
2023-01-13 16:15   ` Robin Murphy
2023-01-17  4:56     ` Vasant Hegde
2023-02-03  9:36   ` Joerg Roedel
2023-02-03 10:40     ` Vasant Hegde
2023-02-03 11:12       ` Joerg Roedel
2023-02-03 16:52         ` Vasant Hegde
2023-02-01  5:19 ` Vasant Hegde [this message]

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=4750852f-53ea-719a-cc03-e2da34e5782d@amd.com \
    --to=vasant.hegde@amd.com \
    --cc=iommu@lists.linux.dev \
    --cc=joro@8bytes.org \
    --cc=robin.murphy@arm.com \
    --cc=suravee.suthikulpanit@amd.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