diff mbox

[v10,07/19] vfio iommu type1: Update argument of vaddr_get_pfn()

Message ID 1477517366-27871-8-git-send-email-kwankhede@nvidia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kirti Wankhede Oct. 26, 2016, 9:29 p.m. UTC 
Update arguments of vaddr_get_pfn() to take struct mm_struct *mm as input
argument.

Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
Signed-off-by: Neo Jia <cjia@nvidia.com>
Change-Id: I885fd4cd4a9f66f4ee2c1caf58267464ec239f52
---
 drivers/vfio/vfio_iommu_type1.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

Comments

Jike Song Oct. 27, 2016, 12:11 p.m. UTC | #1 
On 10/27/2016 05:29 AM, Kirti Wankhede wrote:
> Update arguments of vaddr_get_pfn() to take struct mm_struct *mm as input
> argument.
> 
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Neo Jia <cjia@nvidia.com>
> Change-Id: I885fd4cd4a9f66f4ee2c1caf58267464ec239f52
> ---
>  drivers/vfio/vfio_iommu_type1.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 72fee415044a..3d916b965492 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -228,20 +228,31 @@ static int put_pfn(unsigned long pfn, int prot)
>  	return 0;
>  }
>  
> -static int vaddr_get_pfn(unsigned long vaddr, int prot, unsigned long *pfn)
> +static int vaddr_get_pfn(struct mm_struct *remote_mm, unsigned long vaddr,
> +			 int prot, unsigned long *pfn)
>  {
>  	struct page *page[1];
>  	struct vm_area_struct *vma;
> +	struct mm_struct *mm = (remote_mm ? remote_mm : current->mm);
>  	int ret = -EFAULT;
>  
> -	if (get_user_pages_fast(vaddr, 1, !!(prot & IOMMU_WRITE), page) == 1) {
> +	if (remote_mm) {
> +		down_read(&mm->mmap_sem);
> +		ret = get_user_pages_remote(NULL, mm, vaddr, 1,
> +					!!(prot & IOMMU_WRITE), 0, page, NULL);

Hi Kirti,

It seems that get_user_pages_remote has only 7 args but you somehow gives 8
here?

--
Thanks,
Jike

> +		up_read(&mm->mmap_sem);
> +	} else
> +		ret = get_user_pages_fast(vaddr, 1,
> +					  !!(prot & IOMMU_WRITE), page);
> +
> +	if (ret == 1) {
>  		*pfn = page_to_pfn(page[0]);
>  		return 0;
>  	}
>  
> -	down_read(&current->mm->mmap_sem);
> +	down_read(&mm->mmap_sem);
>  
> -	vma = find_vma_intersection(current->mm, vaddr, vaddr + 1);
> +	vma = find_vma_intersection(mm, vaddr, vaddr + 1);
>  
>  	if (vma && vma->vm_flags & VM_PFNMAP) {
>  		*pfn = ((vaddr - vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff;
> @@ -249,7 +260,7 @@ static int vaddr_get_pfn(unsigned long vaddr, int prot, unsigned long *pfn)
>  			ret = 0;
>  	}
>  
> -	up_read(&current->mm->mmap_sem);
> +	up_read(&mm->mmap_sem);
>  
>  	return ret;
>  }
> @@ -270,7 +281,7 @@ static long vfio_pin_pages(unsigned long vaddr, long npage,
>  	if (!current->mm)
>  		return -ENODEV;
>  
> -	ret = vaddr_get_pfn(vaddr, prot, pfn_base);
> +	ret = vaddr_get_pfn(NULL, vaddr, prot, pfn_base);
>  	if (ret)
>  		return ret;
>  
> @@ -293,7 +304,7 @@ static long vfio_pin_pages(unsigned long vaddr, long npage,
>  	for (i = 1, vaddr += PAGE_SIZE; i < npage; i++, vaddr += PAGE_SIZE) {
>  		unsigned long pfn = 0;
>  
> -		ret = vaddr_get_pfn(vaddr, prot, &pfn);
> +		ret = vaddr_get_pfn(NULL, vaddr, prot, &pfn);
>  		if (ret)
>  			break;
>  
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kirti Wankhede Oct. 27, 2016, 12:24 p.m. UTC | #2 
On 10/27/2016 5:41 PM, Jike Song wrote:
> On 10/27/2016 05:29 AM, Kirti Wankhede wrote:
>> Update arguments of vaddr_get_pfn() to take struct mm_struct *mm as input
>> argument.
>>
>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>> Signed-off-by: Neo Jia <cjia@nvidia.com>
>> Change-Id: I885fd4cd4a9f66f4ee2c1caf58267464ec239f52
>> ---
>>  drivers/vfio/vfio_iommu_type1.c | 25 ++++++++++++++++++-------
>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
>> index 72fee415044a..3d916b965492 100644
>> --- a/drivers/vfio/vfio_iommu_type1.c
>> +++ b/drivers/vfio/vfio_iommu_type1.c
>> @@ -228,20 +228,31 @@ static int put_pfn(unsigned long pfn, int prot)
>>  	return 0;
>>  }
>>  
>> -static int vaddr_get_pfn(unsigned long vaddr, int prot, unsigned long *pfn)
>> +static int vaddr_get_pfn(struct mm_struct *remote_mm, unsigned long vaddr,
>> +			 int prot, unsigned long *pfn)
>>  {
>>  	struct page *page[1];
>>  	struct vm_area_struct *vma;
>> +	struct mm_struct *mm = (remote_mm ? remote_mm : current->mm);
>>  	int ret = -EFAULT;
>>  
>> -	if (get_user_pages_fast(vaddr, 1, !!(prot & IOMMU_WRITE), page) == 1) {
>> +	if (remote_mm) {
>> +		down_read(&mm->mmap_sem);
>> +		ret = get_user_pages_remote(NULL, mm, vaddr, 1,
>> +					!!(prot & IOMMU_WRITE), 0, page, NULL);
> 
> Hi Kirti,
> 
> It seems that get_user_pages_remote has only 7 args but you somehow gives 8
> here?
> 

I had tested this patch with linux-next. It has 8 arguments, see mm/gup.c:

long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
                unsigned long start, unsigned long nr_pages,
                int write, int force, struct page **pages,
                struct vm_area_struct **vmas)
{
        return __get_user_pages_locked(tsk, mm, start, nr_pages, write,
force,
                                       pages, vmas, NULL, false,
                                       FOLL_TOUCH | FOLL_REMOTE);
}
EXPORT_SYMBOL(get_user_pages_remote);


Thanks,
Kirti

> --
> Thanks,
> Jike
> 
>> +		up_read(&mm->mmap_sem);
>> +	} else
>> +		ret = get_user_pages_fast(vaddr, 1,
>> +					  !!(prot & IOMMU_WRITE), page);
>> +
>> +	if (ret == 1) {
>>  		*pfn = page_to_pfn(page[0]);
>>  		return 0;
>>  	}
>>  
>> -	down_read(&current->mm->mmap_sem);
>> +	down_read(&mm->mmap_sem);
>>  
>> -	vma = find_vma_intersection(current->mm, vaddr, vaddr + 1);
>> +	vma = find_vma_intersection(mm, vaddr, vaddr + 1);
>>  
>>  	if (vma && vma->vm_flags & VM_PFNMAP) {
>>  		*pfn = ((vaddr - vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff;
>> @@ -249,7 +260,7 @@ static int vaddr_get_pfn(unsigned long vaddr, int prot, unsigned long *pfn)
>>  			ret = 0;
>>  	}
>>  
>> -	up_read(&current->mm->mmap_sem);
>> +	up_read(&mm->mmap_sem);
>>  
>>  	return ret;
>>  }
>> @@ -270,7 +281,7 @@ static long vfio_pin_pages(unsigned long vaddr, long npage,
>>  	if (!current->mm)
>>  		return -ENODEV;
>>  
>> -	ret = vaddr_get_pfn(vaddr, prot, pfn_base);
>> +	ret = vaddr_get_pfn(NULL, vaddr, prot, pfn_base);
>>  	if (ret)
>>  		return ret;
>>  
>> @@ -293,7 +304,7 @@ static long vfio_pin_pages(unsigned long vaddr, long npage,
>>  	for (i = 1, vaddr += PAGE_SIZE; i < npage; i++, vaddr += PAGE_SIZE) {
>>  		unsigned long pfn = 0;
>>  
>> -		ret = vaddr_get_pfn(vaddr, prot, &pfn);
>> +		ret = vaddr_get_pfn(NULL, vaddr, prot, &pfn);
>>  		if (ret)
>>  			break;
>>  
>>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jike Song Oct. 28, 2016, 6:01 a.m. UTC | #3 
On 10/27/2016 08:24 PM, Kirti Wankhede wrote:
> 
> 
> On 10/27/2016 5:41 PM, Jike Song wrote:
>> On 10/27/2016 05:29 AM, Kirti Wankhede wrote:
>>> Update arguments of vaddr_get_pfn() to take struct mm_struct *mm as input
>>> argument.
>>>
>>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>>> Signed-off-by: Neo Jia <cjia@nvidia.com>
>>> Change-Id: I885fd4cd4a9f66f4ee2c1caf58267464ec239f52
>>> ---
>>>  drivers/vfio/vfio_iommu_type1.c | 25 ++++++++++++++++++-------
>>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
>>> index 72fee415044a..3d916b965492 100644
>>> --- a/drivers/vfio/vfio_iommu_type1.c
>>> +++ b/drivers/vfio/vfio_iommu_type1.c
>>> @@ -228,20 +228,31 @@ static int put_pfn(unsigned long pfn, int prot)
>>>  	return 0;
>>>  }
>>>  
>>> -static int vaddr_get_pfn(unsigned long vaddr, int prot, unsigned long *pfn)
>>> +static int vaddr_get_pfn(struct mm_struct *remote_mm, unsigned long vaddr,
>>> +			 int prot, unsigned long *pfn)
>>>  {
>>>  	struct page *page[1];
>>>  	struct vm_area_struct *vma;
>>> +	struct mm_struct *mm = (remote_mm ? remote_mm : current->mm);
>>>  	int ret = -EFAULT;
>>>  
>>> -	if (get_user_pages_fast(vaddr, 1, !!(prot & IOMMU_WRITE), page) == 1) {
>>> +	if (remote_mm) {
>>> +		down_read(&mm->mmap_sem);
>>> +		ret = get_user_pages_remote(NULL, mm, vaddr, 1,
>>> +					!!(prot & IOMMU_WRITE), 0, page, NULL);
>>
>> Hi Kirti,
>>
>> It seems that get_user_pages_remote has only 7 args but you somehow gives 8
>> here?
>>
> 
> I had tested this patch with linux-next. It has 8 arguments, see mm/gup.c:
> 
> long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
>                 unsigned long start, unsigned long nr_pages,
>                 int write, int force, struct page **pages,
>                 struct vm_area_struct **vmas)
> {
>         return __get_user_pages_locked(tsk, mm, start, nr_pages, write,
> force,
>                                        pages, vmas, NULL, false,
>                                        FOLL_TOUCH | FOLL_REMOTE);
> }
> EXPORT_SYMBOL(get_user_pages_remote);

It's queer. I checked:

https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/mm/gup.c

It still has 7 args.

I believe you have seen this change somewhere, would you share the repo url
and branch name of the linux-next you based on?

--
Thanks,
Jike
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kirti Wankhede Nov. 2, 2016, 8:06 a.m. UTC | #4 
On 10/28/2016 11:31 AM, Jike Song wrote:
> On 10/27/2016 08:24 PM, Kirti Wankhede wrote:
>>
>>
>> On 10/27/2016 5:41 PM, Jike Song wrote:
>>> On 10/27/2016 05:29 AM, Kirti Wankhede wrote:
>>>> Update arguments of vaddr_get_pfn() to take struct mm_struct *mm as input
>>>> argument.
>>>>
>>>> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
>>>> Signed-off-by: Neo Jia <cjia@nvidia.com>
>>>> Change-Id: I885fd4cd4a9f66f4ee2c1caf58267464ec239f52
>>>> ---
>>>>  drivers/vfio/vfio_iommu_type1.c | 25 ++++++++++++++++++-------
>>>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
>>>> index 72fee415044a..3d916b965492 100644
>>>> --- a/drivers/vfio/vfio_iommu_type1.c
>>>> +++ b/drivers/vfio/vfio_iommu_type1.c
>>>> @@ -228,20 +228,31 @@ static int put_pfn(unsigned long pfn, int prot)
>>>>  	return 0;
>>>>  }
>>>>  
>>>> -static int vaddr_get_pfn(unsigned long vaddr, int prot, unsigned long *pfn)
>>>> +static int vaddr_get_pfn(struct mm_struct *remote_mm, unsigned long vaddr,
>>>> +			 int prot, unsigned long *pfn)
>>>>  {
>>>>  	struct page *page[1];
>>>>  	struct vm_area_struct *vma;
>>>> +	struct mm_struct *mm = (remote_mm ? remote_mm : current->mm);
>>>>  	int ret = -EFAULT;
>>>>  
>>>> -	if (get_user_pages_fast(vaddr, 1, !!(prot & IOMMU_WRITE), page) == 1) {
>>>> +	if (remote_mm) {
>>>> +		down_read(&mm->mmap_sem);
>>>> +		ret = get_user_pages_remote(NULL, mm, vaddr, 1,
>>>> +					!!(prot & IOMMU_WRITE), 0, page, NULL);
>>>
>>> Hi Kirti,
>>>
>>> It seems that get_user_pages_remote has only 7 args but you somehow gives 8
>>> here?
>>>
>>
>> I had tested this patch with linux-next. It has 8 arguments, see mm/gup.c:
>>
>> long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
>>                 unsigned long start, unsigned long nr_pages,
>>                 int write, int force, struct page **pages,
>>                 struct vm_area_struct **vmas)
>> {
>>         return __get_user_pages_locked(tsk, mm, start, nr_pages, write,
>> force,
>>                                        pages, vmas, NULL, false,
>>                                        FOLL_TOUCH | FOLL_REMOTE);
>> }
>> EXPORT_SYMBOL(get_user_pages_remote);
> 
> It's queer. I checked:
> 
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/mm/gup.c
> 
> It still has 7 args.
> 
> I believe you have seen this change somewhere, would you share the repo url
> and branch name of the linux-next you based on?
> 

Sorry, my git was synced just few changes behind this change. I'll make
sure that I test the patch set at top of tree.

Thanks,
Kirti

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 72fee415044a..3d916b965492 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -228,20 +228,31 @@  static int put_pfn(unsigned long pfn, int prot)
 	return 0;
 }
 
-static int vaddr_get_pfn(unsigned long vaddr, int prot, unsigned long *pfn)
+static int vaddr_get_pfn(struct mm_struct *remote_mm, unsigned long vaddr,
+			 int prot, unsigned long *pfn)
 {
 	struct page *page[1];
 	struct vm_area_struct *vma;
+	struct mm_struct *mm = (remote_mm ? remote_mm : current->mm);
 	int ret = -EFAULT;
 
-	if (get_user_pages_fast(vaddr, 1, !!(prot & IOMMU_WRITE), page) == 1) {
+	if (remote_mm) {
+		down_read(&mm->mmap_sem);
+		ret = get_user_pages_remote(NULL, mm, vaddr, 1,
+					!!(prot & IOMMU_WRITE), 0, page, NULL);
+		up_read(&mm->mmap_sem);
+	} else
+		ret = get_user_pages_fast(vaddr, 1,
+					  !!(prot & IOMMU_WRITE), page);
+
+	if (ret == 1) {
 		*pfn = page_to_pfn(page[0]);
 		return 0;
 	}
 
-	down_read(&current->mm->mmap_sem);
+	down_read(&mm->mmap_sem);
 
-	vma = find_vma_intersection(current->mm, vaddr, vaddr + 1);
+	vma = find_vma_intersection(mm, vaddr, vaddr + 1);
 
 	if (vma && vma->vm_flags & VM_PFNMAP) {
 		*pfn = ((vaddr - vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff;
@@ -249,7 +260,7 @@  static int vaddr_get_pfn(unsigned long vaddr, int prot, unsigned long *pfn)
 			ret = 0;
 	}
 
-	up_read(&current->mm->mmap_sem);
+	up_read(&mm->mmap_sem);
 
 	return ret;
 }
@@ -270,7 +281,7 @@  static long vfio_pin_pages(unsigned long vaddr, long npage,
 	if (!current->mm)
 		return -ENODEV;
 
-	ret = vaddr_get_pfn(vaddr, prot, pfn_base);
+	ret = vaddr_get_pfn(NULL, vaddr, prot, pfn_base);
 	if (ret)
 		return ret;
 
@@ -293,7 +304,7 @@  static long vfio_pin_pages(unsigned long vaddr, long npage,
 	for (i = 1, vaddr += PAGE_SIZE; i < npage; i++, vaddr += PAGE_SIZE) {
 		unsigned long pfn = 0;
 
-		ret = vaddr_get_pfn(vaddr, prot, &pfn);
+		ret = vaddr_get_pfn(NULL, vaddr, prot, &pfn);
 		if (ret)
 			break;