qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/1] vfio iommu type1: set size to PAGE_SIZE whi


From: Alex Williamson
Subject: Re: [Qemu-devel] [PATCH 1/1] vfio iommu type1: set size to PAGE_SIZE while looking for iova in dma list
Date: Mon, 5 Dec 2016 15:54:21 -0700

On Tue, 6 Dec 2016 02:38:01 +0530
Kirti Wankhede <address@hidden> wrote:

> In the functions of pin_pages/unpin_pages from mdev vendor driver,
> vfio_find_dma() should be called with size as PAGE_SIZE instead of 0.
> vfio_find_dma() searches for the range in dma_list.
> 
> In vfio_dma_do_unmap(), vfio_find_dma() when used to look for start
> address of unmap->iova, size should be 1, not 0. Otherwise vfio_find_dma()
> returns NULL.

Hi Kirti,

I'd prefer to fix the preexisting case of this issue as a separate
patch, we might want to backport it for stable trees separately from the
new cases introduced with mdev.  It would also be great if we could go
into some detail about why size=0 doesn't work in these cases, ie.
vfio_find_dma() accounting for the end address of the range makes it
fall through at the wrong branch point.  For the case of the
preexisting issue, it would also be useful to mention that the other
case of passing size=0 works correctly due to the -1 in the start
address calculation.  Finally, I think I agree with your choice to use
PAGE_SIZE in one case and 1 in the preexisting case, but perhaps we
could spell that out as one is related to a page size interface and the
other is trying to test boundary conditions.  Thanks,

Alex

> Signed-off-by: Kirti Wankhede <address@hidden>
> Signed-off-by: Neo Jia <address@hidden>
> Change-Id: Iee6fd45441c342b5e8626087046b2e0075d19a08
> ---
>  drivers/vfio/vfio_iommu_type1.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index a28fbddb505c..023fba7b8d5a 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -581,7 +581,7 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
>               struct vfio_pfn *vpfn;
>  
>               iova = user_pfn[i] << PAGE_SHIFT;
> -             dma = vfio_find_dma(iommu, iova, 0);
> +             dma = vfio_find_dma(iommu, iova, PAGE_SIZE);
>               if (!dma) {
>                       ret = -EINVAL;
>                       goto pin_unwind;
> @@ -622,7 +622,7 @@ pin_unwind:
>               dma_addr_t iova;
>  
>               iova = user_pfn[j] << PAGE_SHIFT;
> -             dma = vfio_find_dma(iommu, iova, 0);
> +             dma = vfio_find_dma(iommu, iova, PAGE_SIZE);
>               vfio_unpin_page_external(dma, iova, do_accounting);
>               phys_pfn[j] = 0;
>       }
> @@ -659,7 +659,7 @@ static int vfio_iommu_type1_unpin_pages(void *iommu_data,
>               dma_addr_t iova;
>  
>               iova = user_pfn[i] << PAGE_SHIFT;
> -             dma = vfio_find_dma(iommu, iova, 0);
> +             dma = vfio_find_dma(iommu, iova, PAGE_SIZE);
>               if (!dma)
>                       goto unpin_exit;
>               vfio_unpin_page_external(dma, iova, do_accounting);
> @@ -826,7 +826,7 @@ again:
>        * mappings within the range.
>        */
>       if (iommu->v2) {
> -             dma = vfio_find_dma(iommu, unmap->iova, 0);
> +             dma = vfio_find_dma(iommu, unmap->iova, 1);
>               if (dma && dma->iova != unmap->iova) {
>                       ret = -EINVAL;
>                       goto unlock;




reply via email to

[Prev in Thread] Current Thread [Next in Thread]