Skip to content

Commit 860cd61

Browse files
author
Alex Williamson
committed
vfio/type1: Use vfio_batch for vaddr_get_pfns()
JIRA: https://issues.redhat.com/browse/RHEL-85587 commit eb996ee Author: Alex Williamson <alex.williamson@redhat.com> Date: Tue Feb 18 15:22:03 2025 -0700 vfio/type1: Use vfio_batch for vaddr_get_pfns() Passing the vfio_batch to vaddr_get_pfns() allows for greater distinction between page backed pfns and pfnmaps. In the case of page backed pfns, vfio_batch.size is set to a positive value matching the number of pages filled in vfio_batch.pages. For a pfnmap, vfio_batch.size remains zero as vfio_batch.pages are not used. In both cases the return value continues to indicate the number of pfns and the provided pfn arg is set to the initial pfn value. This allows us to shortcut the pfnmap case, which is detected by the zero vfio_batch.size. pfnmaps do not contribute to locked memory accounting, therefore we can update counters and continue directly, which also enables a future where vaddr_get_pfns() can return a value greater than one for consecutive pfnmaps. NB. Now that we're not guessing whether the initial pfn is page backed or pfnmap, we no longer need to special case the put_pfn() and batch size reset. It's safe for vfio_batch_unpin() to handle this case. Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: Mitchell Augustin <mitchell.augustin@canonical.com> Tested-by: Mitchell Augustin <mitchell.augustin@canonical.com> Link: https://lore.kernel.org/r/20250218222209.1382449-4-alex.williamson@redhat.com Signed-off-by: Alex Williamson <alex.williamson@redhat.com> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
1 parent e8ccfb8 commit 860cd61

File tree

1 file changed

+35
-28
lines changed

1 file changed

+35
-28
lines changed

drivers/vfio/vfio_iommu_type1.c

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -556,12 +556,16 @@ static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
556556

557557
/*
558558
* Returns the positive number of pfns successfully obtained or a negative
559-
* error code.
559+
* error code. The initial pfn is stored in the pfn arg. For page-backed
560+
* pfns, the provided batch is also updated to indicate the filled pages and
561+
* initial offset. For VM_PFNMAP pfns, only the returned number of pfns and
562+
* returned initial pfn are provided; subsequent pfns are contiguous.
560563
*/
561564
static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
562565
long npages, int prot, unsigned long *pfn,
563-
struct page **pages)
566+
struct vfio_batch *batch)
564567
{
568+
long pin_pages = min_t(long, npages, batch->capacity);
565569
struct vm_area_struct *vma;
566570
unsigned int flags = 0;
567571
int ret;
@@ -570,10 +574,12 @@ static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
570574
flags |= FOLL_WRITE;
571575

572576
mmap_read_lock(mm);
573-
ret = pin_user_pages_remote(mm, vaddr, npages, flags | FOLL_LONGTERM,
574-
pages, NULL);
577+
ret = pin_user_pages_remote(mm, vaddr, pin_pages, flags | FOLL_LONGTERM,
578+
batch->pages, NULL);
575579
if (ret > 0) {
576-
*pfn = page_to_pfn(pages[0]);
580+
*pfn = page_to_pfn(batch->pages[0]);
581+
batch->size = ret;
582+
batch->offset = 0;
577583
goto done;
578584
} else if (!ret) {
579585
ret = -EFAULT;
@@ -629,32 +635,42 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
629635
*pfn_base = 0;
630636
}
631637

638+
if (unlikely(disable_hugepages))
639+
npage = 1;
640+
632641
while (npage) {
633642
if (!batch->size) {
634643
/* Empty batch, so refill it. */
635-
long req_pages = min_t(long, npage, batch->capacity);
636-
637-
ret = vaddr_get_pfns(mm, vaddr, req_pages, dma->prot,
638-
&pfn, batch->pages);
644+
ret = vaddr_get_pfns(mm, vaddr, npage, dma->prot,
645+
&pfn, batch);
639646
if (ret < 0)
640647
goto unpin_out;
641648

642-
batch->size = ret;
643-
batch->offset = 0;
644-
645649
if (!*pfn_base) {
646650
*pfn_base = pfn;
647651
rsvd = is_invalid_reserved_pfn(*pfn_base);
648652
}
653+
654+
/* Handle pfnmap */
655+
if (!batch->size) {
656+
if (pfn != *pfn_base + pinned || !rsvd)
657+
goto out;
658+
659+
pinned += ret;
660+
npage -= ret;
661+
vaddr += (PAGE_SIZE * ret);
662+
iova += (PAGE_SIZE * ret);
663+
continue;
664+
}
649665
}
650666

651667
/*
652-
* pfn is preset for the first iteration of this inner loop and
653-
* updated at the end to handle a VM_PFNMAP pfn. In that case,
654-
* batch->pages isn't valid (there's no struct page), so allow
655-
* batch->pages to be touched only when there's more than one
656-
* pfn to check, which guarantees the pfns are from a
657-
* !VM_PFNMAP vma.
668+
* pfn is preset for the first iteration of this inner loop
669+
* due to the fact that vaddr_get_pfns() needs to provide the
670+
* initial pfn for pfnmaps. Therefore to reduce redundancy,
671+
* the next pfn is fetched at the end of the loop.
672+
* A PageReserved() page could still qualify as page backed
673+
* and rsvd here, and therefore continues to use the batch.
658674
*/
659675
while (true) {
660676
if (pfn != *pfn_base + pinned ||
@@ -689,21 +705,12 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
689705

690706
pfn = page_to_pfn(batch->pages[batch->offset]);
691707
}
692-
693-
if (unlikely(disable_hugepages))
694-
break;
695708
}
696709

697710
out:
698711
ret = vfio_lock_acct(dma, lock_acct, false);
699712

700713
unpin_out:
701-
if (batch->size == 1 && !batch->offset) {
702-
/* May be a VM_PFNMAP pfn, which the batch can't remember. */
703-
put_pfn(pfn, dma->prot);
704-
batch->size = 0;
705-
}
706-
707714
if (ret < 0) {
708715
if (pinned && !rsvd) {
709716
for (pfn = *pfn_base ; pinned ; pfn++, pinned--)
@@ -751,7 +758,7 @@ static int vfio_pin_page_external(struct vfio_dma *dma, unsigned long vaddr,
751758

752759
vfio_batch_init_single(&batch);
753760

754-
ret = vaddr_get_pfns(mm, vaddr, 1, dma->prot, pfn_base, batch.pages);
761+
ret = vaddr_get_pfns(mm, vaddr, 1, dma->prot, pfn_base, &batch);
755762
if (ret != 1)
756763
goto out;
757764

0 commit comments

Comments
 (0)