Re: [PATCH v14 08/22] vfio iommu type1: Add find_iommu_group() function

[Auger Eric]

Hi,
On 16/11/2016 21:46, Kirti Wankhede wrote:
> Add find_iommu_group()
> 
> Signed-off-by: Kirti Wankhede <kwankhede@xxxxxxxxxx>
> Signed-off-by: Neo Jia <cjia@xxxxxxxxxx>
> Reviewed-by: Jike Song <jike.song@xxxxxxxxx>
> Reviewed-by: Dong Jia Shi <bjsdjshi@xxxxxxxxxxxxxxxxxx>
> 
> Change-Id: I9d372f1ebe9eb01a5a21374b8a2b03f7df73601f
> ---
>  drivers/vfio/vfio_iommu_type1.c | 57 ++++++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 52af5fc01d91..ffe2026f1341 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -752,11 +752,24 @@ static void vfio_test_domain_fgsp(struct vfio_domain *domain)
>  	__free_pages(pages, order);
>  }
>  
> +static struct vfio_group *find_iommu_group(struct vfio_domain *domain,
> +					   struct iommu_group *iommu_group)
> +{
> +	struct vfio_group *g;
> +
> +	list_for_each_entry(g, &domain->group_list, next) {
> +		if (g->iommu_group == iommu_group)
> +			return g;
> +	}
> +
> +	return NULL;
> +}
> +
>  static int vfio_iommu_type1_attach_group(void *iommu_data,
>  					 struct iommu_group *iommu_group)
>  {
>  	struct vfio_iommu *iommu = iommu_data;
> -	struct vfio_group *group, *g;
> +	struct vfio_group *group;
>  	struct vfio_domain *domain, *d;
>  	struct bus_type *bus = NULL;
>  	int ret;
> @@ -764,10 +777,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
>  	mutex_lock(&iommu->lock);
>  
>  	list_for_each_entry(d, &iommu->domain_list, next) {
> -		list_for_each_entry(g, &d->group_list, next) {
> -			if (g->iommu_group != iommu_group)
> -				continue;
> -
> +		if (find_iommu_group(d, iommu_group)) {
>  			mutex_unlock(&iommu->lock);
>  			return -EINVAL;
>  		}
> @@ -887,27 +897,26 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
>  	mutex_lock(&iommu->lock);
>  
>  	list_for_each_entry(domain, &iommu->domain_list, next) {
> -		list_for_each_entry(group, &domain->group_list, next) {
> -			if (group->iommu_group != iommu_group)
> -				continue;
> +		group = find_iommu_group(domain, iommu_group);
> +		if (!group)
> +			continue;
>  
> -			iommu_detach_group(domain->domain, iommu_group);
> -			list_del(&group->next);
> -			kfree(group);
> -			/*
> -			 * Group ownership provides privilege, if the group
> -			 * list is empty, the domain goes away.  If it's the
> -			 * last domain, then all the mappings go away too.
> -			 */
> -			if (list_empty(&domain->group_list)) {
> -				if (list_is_singular(&iommu->domain_list))
> -					vfio_iommu_unmap_unpin_all(iommu);
> -				iommu_domain_free(domain->domain);
> -				list_del(&domain->next);
> -				kfree(domain);
> -			}
> -			goto done;
> +		iommu_detach_group(domain->domain, iommu_group);
> +		list_del(&group->next);
> +		kfree(group);
> +		/*
> +		 * Group ownership provides privilege, if the group
> +		 * list is empty, the domain goes away.  If it's the
> +		 * last domain, then all the mappings go away too.
> +		 */
> +		if (list_empty(&domain->group_list)) {
> +			if (list_is_singular(&iommu->domain_list))
> +				vfio_iommu_unmap_unpin_all(iommu);
> +			iommu_domain_free(domain->domain);
> +			list_del(&domain->next);
> +			kfree(domain);
>  		}
> +		goto done;
>  	}
>  
>  done:
> 
Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx>

Eric