diff mbox

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

Message ID 1479329194-10247-9-git-send-email-kwankhede@nvidia.com
State New
Headers show

Commit Message

Kirti Wankhede Nov. 16, 2016, 8:46 p.m. UTC
Add find_iommu_group()

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Reviewed-by: Jike Song <jike.song@intel.com>
Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>

Change-Id: I9d372f1ebe9eb01a5a21374b8a2b03f7df73601f
---
 drivers/vfio/vfio_iommu_type1.c | 57 ++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 24 deletions(-)

Comments

Eric Auger Nov. 21, 2016, 11:29 a.m. UTC | #1
Hi,
On 16/11/2016 21:46, Kirti Wankhede wrote:
> Add find_iommu_group()
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Neo Jia <cjia@nvidia.com>
> Reviewed-by: Jike Song <jike.song@intel.com>
> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> 
> 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@redhat.com>

Eric
Kirti Wankhede Nov. 21, 2016, 11:55 a.m. UTC | #2
Hi Eric,

Thanks for your reviews.
Alex had already pulled this series into his next branch for v4.10.

http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1274745.html

Thanks,
Kirti

On 11/21/2016 4:59 PM, Auger Eric wrote:
> Hi,
> On 16/11/2016 21:46, Kirti Wankhede wrote:
>> Add find_iommu_group()
>>
>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>> Signed-off-by: Neo Jia <cjia@nvidia.com>
>> Reviewed-by: Jike Song <jike.song@intel.com>
>> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
>>
>> 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@redhat.com>
> 
> Eric
>
Eric Auger Nov. 21, 2016, 12:42 p.m. UTC | #3
Hi Kirti,

On 21/11/2016 12:55, Kirti Wankhede wrote:
> 
> Hi Eric,
> 
> Thanks for your reviews.
> Alex had already pulled this series into his next branch for v4.10.
> 
> http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1274745.html

Yes I am showing up too late and noticed that later :-(

Anyway I needed (and still need) to look at it.

Thanks

Eric
> 
> Thanks,
> Kirti
> 
> On 11/21/2016 4:59 PM, Auger Eric wrote:
>> Hi,
>> On 16/11/2016 21:46, Kirti Wankhede wrote:
>>> Add find_iommu_group()
>>>
>>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>>> Signed-off-by: Neo Jia <cjia@nvidia.com>
>>> Reviewed-by: Jike Song <jike.song@intel.com>
>>> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
>>>
>>> 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@redhat.com>
>>
>> Eric
>>
diff mbox

Patch

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: