| Message ID | 161524017090.3480.6508004360325488879.stgit@gimli.home (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio: Device memory DMA mapping improvements | expand |
On Mon, Mar 08, 2021 at 02:49:31PM -0700, Alex Williamson wrote: > Populate the page array to the extent available to enable batching. > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > drivers/vfio/vfio_iommu_type1.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index e89f11141dee..d499bccfbe3f 100644 > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -628,6 +628,8 @@ static int vaddr_get_pfns(struct vfio_iommu *iommu, struct vfio_dma *dma, > vma = find_vma_intersection(mm, vaddr, vaddr + 1); > > if (vma && vma->vm_flags & VM_PFNMAP) { > + unsigned long count, i; > + > if ((dma->prot & IOMMU_WRITE && !(vma->vm_flags & VM_WRITE)) || > (dma->prot & IOMMU_READ && !(vma->vm_flags & VM_READ))) { > ret = -EFAULT; > @@ -678,7 +680,13 @@ static int vaddr_get_pfns(struct vfio_iommu *iommu, struct vfio_dma *dma, > > *pfn = ((vaddr - vma->vm_start) >> PAGE_SHIFT) + > dma->pfnmap->base_pfn; > - ret = 1; > + count = min_t(long, > + (vma->vm_end - vaddr) >> PAGE_SHIFT, npages); > + > + for (i = 0; i < count; i++) > + pages[i] = pfn_to_page(*pfn + i); This isn't safe, we can't pass a VM_PFNMAP pfn into pfn_to_page(). The whole api here with the batch should be using pfns not struct pages Also.. this is not nice at all: static int put_pfn(unsigned long pfn, int prot) { if (!is_invalid_reserved_pfn(pfn)) { struct page *page = pfn_to_page(pfn); unpin_user_pages_dirty_lock(&page, 1, prot & IOMMU_WRITE); The manner in which the PFN was obtained should be tracked internally to VFIO, not deduced externally by the pfn type. *only* pages returned by pin_user_pages() should be used with unpin_user_pages() - the other stuff must be kept distinct. This is actually another bug with the way things are today, as if the user gets a PFNMAP VMA that happens to point to a struct page (eg a MIXEDMAP, these things exist in the kernel), the unpin will explode when it gets here. Something like what hmm_range_fault() does where the high bits of the pfn encode information about it (there is always PAGE_SHIFT high bits available for use) is much cleaner/safer. Jason
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index e89f11141dee..d499bccfbe3f 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -628,6 +628,8 @@ static int vaddr_get_pfns(struct vfio_iommu *iommu, struct vfio_dma *dma, vma = find_vma_intersection(mm, vaddr, vaddr + 1); if (vma && vma->vm_flags & VM_PFNMAP) { + unsigned long count, i; + if ((dma->prot & IOMMU_WRITE && !(vma->vm_flags & VM_WRITE)) || (dma->prot & IOMMU_READ && !(vma->vm_flags & VM_READ))) { ret = -EFAULT; @@ -678,7 +680,13 @@ static int vaddr_get_pfns(struct vfio_iommu *iommu, struct vfio_dma *dma, *pfn = ((vaddr - vma->vm_start) >> PAGE_SHIFT) + dma->pfnmap->base_pfn; - ret = 1; + count = min_t(long, + (vma->vm_end - vaddr) >> PAGE_SHIFT, npages); + + for (i = 0; i < count; i++) + pages[i] = pfn_to_page(*pfn + i); + + ret = count; } done: mmap_read_unlock(mm);
Populate the page array to the extent available to enable batching. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- drivers/vfio/vfio_iommu_type1.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)