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

From: Kirti Wankhede
Date: Mon Nov 21 2016 - 06:56:13 EST



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@xxxxxxxxxxxxxxx/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@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
>