[PATCH 0/5] vfio: Improve DMA mapping performance for huge pfnmaps

[PATCH 0/5] vfio: Improve DMA mapping performance for huge pfnmaps

[Alex Williamson]

As GPU BAR sizes increase, the overhead of DMA mapping pfnmap ranges has
become a significant overhead for VMs making use of device assignment.
Not only does each mapping require upwards of a few seconds, but BARs
are mapped in and out of the VM address space multiple times during
guest boot.  Also factor in that multi-GPU configurations are
increasingly commonplace and BAR sizes are continuing to increase.
Configurations today can already be delayed minutes during guest boot.

We've taken steps to make Linux a better guest by batching PCI BAR
sizing operations[1], but it only provides and incremental improvement.

This series attempts to fully address the issue by leveraging the huge
pfnmap support added in v6.12.  When we insert pfnmaps using pud and pmd
mappings, we can later take advantage of the knowledge of the mapping
level page mask to iterate on the relevant mapping stride.  In the
commonly achieved optimal case, this results in a reduction of pfn
lookups by a factor of 256k.  For a local test system, an overhead of
~1s for DMA mapping a 32GB PCI BAR is reduced to sub-millisecond (8M
page sized operations reduced to 32 pud sized operations).

Please review, test, and provide feedback.  I hope that mm folks can
ack the trivial follow_pfnmap_args update to provide the mapping level
page mask.  Naming is hard, so any preference other than pgmask is
welcome.  Thanks,

Alex

[1]https://lore.kernel.org/all/20250120182202.1878581-1-alex.williamson@redhat.com/


Alex Williamson (5):
  vfio/type1: Catch zero from pin_user_pages_remote()
  vfio/type1: Convert all vaddr_get_pfns() callers to use vfio_batch
  vfio/type1: Use vfio_batch for vaddr_get_pfns()
  mm: Provide page mask in struct follow_pfnmap_args
  vfio/type1: Use mapping page mask for pfnmaps

 drivers/vfio/vfio_iommu_type1.c | 107 ++++++++++++++++++++------------
 include/linux/mm.h              |   2 +
 mm/memory.c                     |   1 +
 3 files changed, 72 insertions(+), 38 deletions(-)

-- 
2.47.1

[PATCH 1/5] vfio/type1: Catch zero from pin_user_pages_remote()

[Alex Williamson]

pin_user_pages_remote() can currently return zero for invalid args
or zero nr_pages, neither of which should ever happen.  However
vaddr_get_pfns() indicates it should only ever return a positive
value or -errno and there's a theoretical case where this can slip
through and be unhandled by callers.  Therefore convert zero to
-EFAULT.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/vfio/vfio_iommu_type1.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 50ebc9593c9d..119cf886d8c0 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -564,6 +564,8 @@ static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
 	if (ret > 0) {
 		*pfn = page_to_pfn(pages[0]);
 		goto done;
+	} else if (!ret) {
+		ret = -EFAULT;
 	}
 
 	vaddr = untagged_addr_remote(mm, vaddr);
-- 
2.47.1

Re: [PATCH 1/5] vfio/type1: Catch zero from pin_user_pages_remote()

[Mitchell Augustin]

Reviewed-by: "Mitchell Augustin" <mitchell.augustin@canonical.com>
Tested-by: "Mitchell Augustin" <mitchell.augustin@canonical.com>



On Wed, Feb 5, 2025 at 5:18 PM Alex Williamson
<alex.williamson@redhat.com> wrote:
>
> pin_user_pages_remote() can currently return zero for invalid args
> or zero nr_pages, neither of which should ever happen.  However
> vaddr_get_pfns() indicates it should only ever return a positive
> value or -errno and there's a theoretical case where this can slip
> through and be unhandled by callers.  Therefore convert zero to
> -EFAULT.
>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  drivers/vfio/vfio_iommu_type1.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 50ebc9593c9d..119cf886d8c0 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -564,6 +564,8 @@ static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
>         if (ret > 0) {
>                 *pfn = page_to_pfn(pages[0]);
>                 goto done;
> +       } else if (!ret) {
> +               ret = -EFAULT;
>         }
>
>         vaddr = untagged_addr_remote(mm, vaddr);
> --
> 2.47.1
>


--
Mitchell Augustin
Software Engineer - Ubuntu Partner Engineering

Re: [PATCH 1/5] vfio/type1: Catch zero from pin_user_pages_remote()

[Jason Gunthorpe]

On Wed, Feb 05, 2025 at 04:17:17PM -0700, Alex Williamson wrote:
> pin_user_pages_remote() can currently return zero for invalid args

It is so weird that it still does that, I tried to get rid of most of
those but didn't dare touch that..

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason

[PATCH 2/5] vfio/type1: Convert all vaddr_get_pfns() callers to use vfio_batch

[Alex Williamson]

This is a step towards passing the structure to vaddr_get_pfns()
directly in order to provide greater distinction between page backed
pfns and pfnmaps.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/vfio/vfio_iommu_type1.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 119cf886d8c0..2e95f5f4d881 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -471,12 +471,12 @@ static int put_pfn(unsigned long pfn, int prot)
 
 #define VFIO_BATCH_MAX_CAPACITY (PAGE_SIZE / sizeof(struct page *))
 
-static void vfio_batch_init(struct vfio_batch *batch)
+static void __vfio_batch_init(struct vfio_batch *batch, bool single)
 {
 	batch->size = 0;
 	batch->offset = 0;
 
-	if (unlikely(disable_hugepages))
+	if (single || unlikely(disable_hugepages))
 		goto fallback;
 
 	batch->pages = (struct page **) __get_free_page(GFP_KERNEL);
@@ -491,6 +491,16 @@ static void vfio_batch_init(struct vfio_batch *batch)
 	batch->capacity = 1;
 }
 
+static void vfio_batch_init(struct vfio_batch *batch)
+{
+	__vfio_batch_init(batch, false);
+}
+
+static void vfio_batch_init_single(struct vfio_batch *batch)
+{
+	__vfio_batch_init(batch, true);
+}
+
 static void vfio_batch_unpin(struct vfio_batch *batch, struct vfio_dma *dma)
 {
 	while (batch->size) {
@@ -730,7 +740,7 @@ static long vfio_unpin_pages_remote(struct vfio_dma *dma, dma_addr_t iova,
 static int vfio_pin_page_external(struct vfio_dma *dma, unsigned long vaddr,
 				  unsigned long *pfn_base, bool do_accounting)
 {
-	struct page *pages[1];
+	struct vfio_batch batch;
 	struct mm_struct *mm;
 	int ret;
 
@@ -738,7 +748,9 @@ static int vfio_pin_page_external(struct vfio_dma *dma, unsigned long vaddr,
 	if (!mmget_not_zero(mm))
 		return -ENODEV;
 
-	ret = vaddr_get_pfns(mm, vaddr, 1, dma->prot, pfn_base, pages);
+	vfio_batch_init_single(&batch);
+
+	ret = vaddr_get_pfns(mm, vaddr, 1, dma->prot, pfn_base, batch.pages);
 	if (ret != 1)
 		goto out;
 
@@ -757,6 +769,7 @@ static int vfio_pin_page_external(struct vfio_dma *dma, unsigned long vaddr,
 	}
 
 out:
+	vfio_batch_fini(&batch);
 	mmput(mm);
 	return ret;
 }
-- 
2.47.1

Re: [PATCH 2/5] vfio/type1: Convert all vaddr_get_pfns() callers to use vfio_batch

[Mitchell Augustin]

Reviewed-by: "Mitchell Augustin" <mitchell.augustin@canonical.com>
Tested-by: "Mitchell Augustin" <mitchell.augustin@canonical.com>

[PATCH 3/5] vfio/type1: Use vfio_batch for vaddr_get_pfns()

[Alex Williamson]

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.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/vfio/vfio_iommu_type1.c | 62 ++++++++++++++++++---------------
 1 file changed, 34 insertions(+), 28 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 2e95f5f4d881..939920454da7 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -555,12 +555,16 @@ static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
 
 /*
  * Returns the positive number of pfns successfully obtained or a negative
- * error code.
+ * error code.  The initial pfn is stored in the pfn arg.  For page-backed
+ * pfns, the provided batch is also updated to indicate the filled pages and
+ * initial offset.  For VM_PFNMAP pfns, only the returned number of pfns and
+ * returned initial pfn are provided; subsequent pfns are contiguous.
  */
 static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
 			  long npages, int prot, unsigned long *pfn,
-			  struct page **pages)
+			  struct vfio_batch *batch)
 {
+	long pin_pages = min_t(long, npages, batch->capacity);
 	struct vm_area_struct *vma;
 	unsigned int flags = 0;
 	int ret;
@@ -569,10 +573,12 @@ static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
 		flags |= FOLL_WRITE;
 
 	mmap_read_lock(mm);
-	ret = pin_user_pages_remote(mm, vaddr, npages, flags | FOLL_LONGTERM,
-				    pages, NULL);
+	ret = pin_user_pages_remote(mm, vaddr, pin_pages, flags | FOLL_LONGTERM,
+				    batch->pages, NULL);
 	if (ret > 0) {
-		*pfn = page_to_pfn(pages[0]);
+		*pfn = page_to_pfn(batch->pages[0]);
+		batch->size = ret;
+		batch->offset = 0;
 		goto done;
 	} else if (!ret) {
 		ret = -EFAULT;
@@ -628,32 +634,41 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
 		*pfn_base = 0;
 	}
 
+	if (unlikely(disable_hugepages))
+		npage = 1;
+
 	while (npage) {
 		if (!batch->size) {
 			/* Empty batch, so refill it. */
-			long req_pages = min_t(long, npage, batch->capacity);
-
-			ret = vaddr_get_pfns(mm, vaddr, req_pages, dma->prot,
-					     &pfn, batch->pages);
+			ret = vaddr_get_pfns(mm, vaddr, npage, dma->prot,
+					     &pfn, batch);
 			if (ret < 0)
 				goto unpin_out;
 
-			batch->size = ret;
-			batch->offset = 0;
-
 			if (!*pfn_base) {
 				*pfn_base = pfn;
 				rsvd = is_invalid_reserved_pfn(*pfn_base);
 			}
+
+			/* Handle pfnmap */
+			if (!batch->size) {
+				if (pfn != *pfn_base + pinned || !rsvd)
+					goto out;
+
+				pinned += ret;
+				npage -= ret;
+				vaddr += (PAGE_SIZE * ret);
+				iova += (PAGE_SIZE * ret);
+				continue;
+			}
 		}
 
 		/*
-		 * pfn is preset for the first iteration of this inner loop and
-		 * updated at the end to handle a VM_PFNMAP pfn.  In that case,
-		 * batch->pages isn't valid (there's no struct page), so allow
-		 * batch->pages to be touched only when there's more than one
-		 * pfn to check, which guarantees the pfns are from a
-		 * !VM_PFNMAP vma.
+		 * pfn is preset for the first iteration of this inner loop due to the
+		 * fact that vaddr_get_pfns() needs to provide the initial pfn for pfnmaps.
+		 * Therefore to reduce redundancy, the next pfn is fetched at the end of
+		 * the loop.  A PageReserved() page could still qualify as page backed and
+		 * rsvd here, and therefore continues to use the batch.
 		 */
 		while (true) {
 			if (pfn != *pfn_base + pinned ||
@@ -688,21 +703,12 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
 
 			pfn = page_to_pfn(batch->pages[batch->offset]);
 		}
-
-		if (unlikely(disable_hugepages))
-			break;
 	}
 
 out:
 	ret = vfio_lock_acct(dma, lock_acct, false);
 
 unpin_out:
-	if (batch->size == 1 && !batch->offset) {
-		/* May be a VM_PFNMAP pfn, which the batch can't remember. */
-		put_pfn(pfn, dma->prot);
-		batch->size = 0;
-	}
-
 	if (ret < 0) {
 		if (pinned && !rsvd) {
 			for (pfn = *pfn_base ; pinned ; pfn++, pinned--)
@@ -750,7 +756,7 @@ static int vfio_pin_page_external(struct vfio_dma *dma, unsigned long vaddr,
 
 	vfio_batch_init_single(&batch);
 
-	ret = vaddr_get_pfns(mm, vaddr, 1, dma->prot, pfn_base, batch.pages);
+	ret = vaddr_get_pfns(mm, vaddr, 1, dma->prot, pfn_base, &batch);
 	if (ret != 1)
 		goto out;
 
-- 
2.47.1

Re: [PATCH 3/5] vfio/type1: Use vfio_batch for vaddr_get_pfns()

[Mitchell Augustin]

Reviewed-by: "Mitchell Augustin" <mitchell.augustin@canonical.com>
Tested-by: "Mitchell Augustin" <mitchell.augustin@canonical.com>



On Wed, Feb 5, 2025 at 5:18 PM Alex Williamson
<alex.williamson@redhat.com> wrote:
>
> 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.
>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  drivers/vfio/vfio_iommu_type1.c | 62 ++++++++++++++++++---------------
>  1 file changed, 34 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 2e95f5f4d881..939920454da7 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -555,12 +555,16 @@ static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
>
>  /*
>   * Returns the positive number of pfns successfully obtained or a negative
> - * error code.
> + * error code.  The initial pfn is stored in the pfn arg.  For page-backed
> + * pfns, the provided batch is also updated to indicate the filled pages and
> + * initial offset.  For VM_PFNMAP pfns, only the returned number of pfns and
> + * returned initial pfn are provided; subsequent pfns are contiguous.
>   */
>  static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
>                           long npages, int prot, unsigned long *pfn,
> -                         struct page **pages)
> +                         struct vfio_batch *batch)
>  {
> +       long pin_pages = min_t(long, npages, batch->capacity);
>         struct vm_area_struct *vma;
>         unsigned int flags = 0;
>         int ret;
> @@ -569,10 +573,12 @@ static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
>                 flags |= FOLL_WRITE;
>
>         mmap_read_lock(mm);
> -       ret = pin_user_pages_remote(mm, vaddr, npages, flags | FOLL_LONGTERM,
> -                                   pages, NULL);
> +       ret = pin_user_pages_remote(mm, vaddr, pin_pages, flags | FOLL_LONGTERM,
> +                                   batch->pages, NULL);
>         if (ret > 0) {
> -               *pfn = page_to_pfn(pages[0]);
> +               *pfn = page_to_pfn(batch->pages[0]);
> +               batch->size = ret;
> +               batch->offset = 0;
>                 goto done;
>         } else if (!ret) {
>                 ret = -EFAULT;
> @@ -628,32 +634,41 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
>                 *pfn_base = 0;
>         }
>
> +       if (unlikely(disable_hugepages))
> +               npage = 1;
> +
>         while (npage) {
>                 if (!batch->size) {
>                         /* Empty batch, so refill it. */
> -                       long req_pages = min_t(long, npage, batch->capacity);
> -
> -                       ret = vaddr_get_pfns(mm, vaddr, req_pages, dma->prot,
> -                                            &pfn, batch->pages);
> +                       ret = vaddr_get_pfns(mm, vaddr, npage, dma->prot,
> +                                            &pfn, batch);
>                         if (ret < 0)
>                                 goto unpin_out;
>
> -                       batch->size = ret;
> -                       batch->offset = 0;
> -
>                         if (!*pfn_base) {
>                                 *pfn_base = pfn;
>                                 rsvd = is_invalid_reserved_pfn(*pfn_base);
>                         }
> +
> +                       /* Handle pfnmap */
> +                       if (!batch->size) {
> +                               if (pfn != *pfn_base + pinned || !rsvd)
> +                                       goto out;
> +
> +                               pinned += ret;
> +                               npage -= ret;
> +                               vaddr += (PAGE_SIZE * ret);
> +                               iova += (PAGE_SIZE * ret);
> +                               continue;
> +                       }
>                 }
>
>                 /*
> -                * pfn is preset for the first iteration of this inner loop and
> -                * updated at the end to handle a VM_PFNMAP pfn.  In that case,
> -                * batch->pages isn't valid (there's no struct page), so allow
> -                * batch->pages to be touched only when there's more than one
> -                * pfn to check, which guarantees the pfns are from a
> -                * !VM_PFNMAP vma.
> +                * pfn is preset for the first iteration of this inner loop due to the
> +                * fact that vaddr_get_pfns() needs to provide the initial pfn for pfnmaps.
> +                * Therefore to reduce redundancy, the next pfn is fetched at the end of
> +                * the loop.  A PageReserved() page could still qualify as page backed and
> +                * rsvd here, and therefore continues to use the batch.
>                  */
>                 while (true) {
>                         if (pfn != *pfn_base + pinned ||
> @@ -688,21 +703,12 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
>
>                         pfn = page_to_pfn(batch->pages[batch->offset]);
>                 }
> -
> -               if (unlikely(disable_hugepages))
> -                       break;
>         }
>
>  out:
>         ret = vfio_lock_acct(dma, lock_acct, false);
>
>  unpin_out:
> -       if (batch->size == 1 && !batch->offset) {
> -               /* May be a VM_PFNMAP pfn, which the batch can't remember. */
> -               put_pfn(pfn, dma->prot);
> -               batch->size = 0;
> -       }
> -
>         if (ret < 0) {
>                 if (pinned && !rsvd) {
>                         for (pfn = *pfn_base ; pinned ; pfn++, pinned--)
> @@ -750,7 +756,7 @@ static int vfio_pin_page_external(struct vfio_dma *dma, unsigned long vaddr,
>
>         vfio_batch_init_single(&batch);
>
> -       ret = vaddr_get_pfns(mm, vaddr, 1, dma->prot, pfn_base, batch.pages);
> +       ret = vaddr_get_pfns(mm, vaddr, 1, dma->prot, pfn_base, &batch);
>         if (ret != 1)
>                 goto out;
>
> --
> 2.47.1
>


--
Mitchell Augustin
Software Engineer - Ubuntu Partner Engineering

[PATCH 4/5] mm: Provide page mask in struct follow_pfnmap_args

[Alex Williamson]

follow_pfnmap_start() walks the page table for a given address and
fills out the struct follow_pfnmap_args in pfnmap_args_setup().
The page mask of the page table level is already provided to this
latter function for calculating the pfn.  This page mask can also be
useful for the caller to determine the extent of the contiguous
mapping.

For example, vfio-pci now supports huge_fault for pfnmaps and is able
to insert pud and pmd mappings.  When we DMA map these pfnmaps, ex.
PCI MMIO BARs, we iterate follow_pfnmap_start() to get each pfn to test
for a contiguous pfn range.  Providing the mapping page mask allows us
to skip the extent of the mapping level.  Assuming a 1GB pud level and
4KB page size, iterations are reduced by a factor of 256K.  In wall
clock time, mapping a 32GB PCI BAR is reduced from ~1s to <1ms.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 include/linux/mm.h | 2 ++
 mm/memory.c        | 1 +
 2 files changed, 3 insertions(+)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index b1c3db9cf355..0ef7e7a0b4eb 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2416,11 +2416,13 @@ struct follow_pfnmap_args {
 	 * Outputs:
 	 *
 	 * @pfn: the PFN of the address
+	 * @pgmask: page mask covering pfn
 	 * @pgprot: the pgprot_t of the mapping
 	 * @writable: whether the mapping is writable
 	 * @special: whether the mapping is a special mapping (real PFN maps)
 	 */
 	unsigned long pfn;
+	unsigned long pgmask;
 	pgprot_t pgprot;
 	bool writable;
 	bool special;
diff --git a/mm/memory.c b/mm/memory.c
index 398c031be9ba..97ccd43761b2 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -6388,6 +6388,7 @@ static inline void pfnmap_args_setup(struct follow_pfnmap_args *args,
 	args->lock = lock;
 	args->ptep = ptep;
 	args->pfn = pfn_base + ((args->address & ~addr_mask) >> PAGE_SHIFT);
+	args->pgmask = addr_mask;
 	args->pgprot = pgprot;
 	args->writable = writable;
 	args->special = special;
-- 
2.47.1

Re: [PATCH 4/5] mm: Provide page mask in struct follow_pfnmap_args

[Mitchell Augustin]

Reviewed-by: "Mitchell Augustin" <mitchell.augustin@canonical.com>
Tested-by: "Mitchell Augustin" <mitchell.augustin@canonical.com>


On Wed, Feb 5, 2025 at 5:18 PM Alex Williamson
<alex.williamson@redhat.com> wrote:
>
> follow_pfnmap_start() walks the page table for a given address and
> fills out the struct follow_pfnmap_args in pfnmap_args_setup().
> The page mask of the page table level is already provided to this
> latter function for calculating the pfn.  This page mask can also be
> useful for the caller to determine the extent of the contiguous
> mapping.
>
> For example, vfio-pci now supports huge_fault for pfnmaps and is able
> to insert pud and pmd mappings.  When we DMA map these pfnmaps, ex.
> PCI MMIO BARs, we iterate follow_pfnmap_start() to get each pfn to test
> for a contiguous pfn range.  Providing the mapping page mask allows us
> to skip the extent of the mapping level.  Assuming a 1GB pud level and
> 4KB page size, iterations are reduced by a factor of 256K.  In wall
> clock time, mapping a 32GB PCI BAR is reduced from ~1s to <1ms.
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  include/linux/mm.h | 2 ++
>  mm/memory.c        | 1 +
>  2 files changed, 3 insertions(+)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index b1c3db9cf355..0ef7e7a0b4eb 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2416,11 +2416,13 @@ struct follow_pfnmap_args {
>          * Outputs:
>          *
>          * @pfn: the PFN of the address
> +        * @pgmask: page mask covering pfn
>          * @pgprot: the pgprot_t of the mapping
>          * @writable: whether the mapping is writable
>          * @special: whether the mapping is a special mapping (real PFN maps)
>          */
>         unsigned long pfn;
> +       unsigned long pgmask;
>         pgprot_t pgprot;
>         bool writable;
>         bool special;
> diff --git a/mm/memory.c b/mm/memory.c
> index 398c031be9ba..97ccd43761b2 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -6388,6 +6388,7 @@ static inline void pfnmap_args_setup(struct follow_pfnmap_args *args,
>         args->lock = lock;
>         args->ptep = ptep;
>         args->pfn = pfn_base + ((args->address & ~addr_mask) >> PAGE_SHIFT);
> +       args->pgmask = addr_mask;
>         args->pgprot = pgprot;
>         args->writable = writable;
>         args->special = special;
> --
> 2.47.1
>


--
Mitchell Augustin
Software Engineer - Ubuntu Partner Engineering

Re: [PATCH 4/5] mm: Provide page mask in struct follow_pfnmap_args

[Alex Williamson]

Nudge.  Peter Xu provided an R-b for the series.  Would any other mm
folks like to chime in here to provide objection or approval for this
change and merging it through the vfio tree?  Series[1].  Thanks!

Alex

[1]https://lore.kernel.org/all/20250205231728.2527186-1-alex.williamson@redhat.com/

On Wed,  5 Feb 2025 16:17:20 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:

> follow_pfnmap_start() walks the page table for a given address and
> fills out the struct follow_pfnmap_args in pfnmap_args_setup().
> The page mask of the page table level is already provided to this
> latter function for calculating the pfn.  This page mask can also be
> useful for the caller to determine the extent of the contiguous
> mapping.
> 
> For example, vfio-pci now supports huge_fault for pfnmaps and is able
> to insert pud and pmd mappings.  When we DMA map these pfnmaps, ex.
> PCI MMIO BARs, we iterate follow_pfnmap_start() to get each pfn to test
> for a contiguous pfn range.  Providing the mapping page mask allows us
> to skip the extent of the mapping level.  Assuming a 1GB pud level and
> 4KB page size, iterations are reduced by a factor of 256K.  In wall
> clock time, mapping a 32GB PCI BAR is reduced from ~1s to <1ms.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  include/linux/mm.h | 2 ++
>  mm/memory.c        | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index b1c3db9cf355..0ef7e7a0b4eb 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2416,11 +2416,13 @@ struct follow_pfnmap_args {
>  	 * Outputs:
>  	 *
>  	 * @pfn: the PFN of the address
> +	 * @pgmask: page mask covering pfn
>  	 * @pgprot: the pgprot_t of the mapping
>  	 * @writable: whether the mapping is writable
>  	 * @special: whether the mapping is a special mapping (real PFN maps)
>  	 */
>  	unsigned long pfn;
> +	unsigned long pgmask;
>  	pgprot_t pgprot;
>  	bool writable;
>  	bool special;
> diff --git a/mm/memory.c b/mm/memory.c
> index 398c031be9ba..97ccd43761b2 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -6388,6 +6388,7 @@ static inline void pfnmap_args_setup(struct follow_pfnmap_args *args,
>  	args->lock = lock;
>  	args->ptep = ptep;
>  	args->pfn = pfn_base + ((args->address & ~addr_mask) >> PAGE_SHIFT);
> +	args->pgmask = addr_mask;
>  	args->pgprot = pgprot;
>  	args->writable = writable;
>  	args->special = special;

Re: [PATCH 4/5] mm: Provide page mask in struct follow_pfnmap_args

[David Hildenbrand]

On 14.02.25 18:17, Alex Williamson wrote:
> 
> Nudge.  Peter Xu provided an R-b for the series.  Would any other mm
> folks like to chime in here to provide objection or approval for this
> change and merging it through the vfio tree?  Series[1].  Thanks!
> 

Only skimmed over it, nothing jumped at me except ...

Nitpicking:

I was wondering if "page mask" really the right term here. I know that 
we use it in some context (gup, hugetlb, zeropage) to express "mask this 
off and you get the start of the aligned huge page".

For something that walks PFNMAPs (page frames without any real "huge 
page" logical metadata etc. grouping) it was uintuitive for me at first.

addr_mask or pfn_mask (shifted addr_mask) would have been clearer for me.

No strong opinion, just what came to mind while reading this ...

-- 
Cheers,

David / dhildenb

Re: [PATCH 4/5] mm: Provide page mask in struct follow_pfnmap_args

[Alex Williamson]

On Fri, 14 Feb 2025 22:39:30 +0100
David Hildenbrand <david@redhat.com> wrote:

> On 14.02.25 18:17, Alex Williamson wrote:
> > 
> > Nudge.  Peter Xu provided an R-b for the series.  Would any other mm
> > folks like to chime in here to provide objection or approval for this
> > change and merging it through the vfio tree?  Series[1].  Thanks!
> >   
> 
> Only skimmed over it, nothing jumped at me except ...
> 
> Nitpicking:
> 
> I was wondering if "page mask" really the right term here. I know that 
> we use it in some context (gup, hugetlb, zeropage) to express "mask this 
> off and you get the start of the aligned huge page".
> 
> For something that walks PFNMAPs (page frames without any real "huge 
> page" logical metadata etc. grouping) it was uintuitive for me at first.
> 
> addr_mask or pfn_mask (shifted addr_mask) would have been clearer for me.
> 
> No strong opinion, just what came to mind while reading this ...

It's called addr_mask in pfnmap_args_setup() so I'm happy to keep that
naming if pgmask is less intuitive.  Thanks,

Alex

Re: [PATCH 4/5] mm: Provide page mask in struct follow_pfnmap_args

[Jason Gunthorpe]

On Wed, Feb 05, 2025 at 04:17:20PM -0700, Alex Williamson wrote:
> follow_pfnmap_start() walks the page table for a given address and
> fills out the struct follow_pfnmap_args in pfnmap_args_setup().
> The page mask of the page table level is already provided to this
> latter function for calculating the pfn.  This page mask can also be
> useful for the caller to determine the extent of the contiguous
> mapping.
> 
> For example, vfio-pci now supports huge_fault for pfnmaps and is able
> to insert pud and pmd mappings.  When we DMA map these pfnmaps, ex.
> PCI MMIO BARs, we iterate follow_pfnmap_start() to get each pfn to test
> for a contiguous pfn range.  Providing the mapping page mask allows us
> to skip the extent of the mapping level.  Assuming a 1GB pud level and
> 4KB page size, iterations are reduced by a factor of 256K.  In wall
> clock time, mapping a 32GB PCI BAR is reduced from ~1s to <1ms.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  include/linux/mm.h | 2 ++
>  mm/memory.c        | 1 +
>  2 files changed, 3 insertions(+)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason

[PATCH 5/5] vfio/type1: Use mapping page mask for pfnmaps

[Alex Williamson]

vfio-pci supports huge_fault for PCI MMIO BARs and will insert pud and
pmd mappings for well aligned mappings.  follow_pfnmap_start() walks the
page table and therefore knows the page mask of the level where the
address is found and returns this through follow_pfnmap_args.pgmask.
Subsequent pfns from this address until the end of the mapping page are
necessarily consecutive.  Use this information to retrieve a range of
pfnmap pfns in a single pass.

With optimal mappings and alignment on systems with 1GB pud and 4KB
page size, this reduces iterations for DMA mapping PCI BARs by a
factor of 256K.  In real world testing, the overhead of iterating
pfns for a VM DMA mapping a 32GB PCI BAR is reduced from ~1s to
sub-millisecond overhead.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/vfio/vfio_iommu_type1.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 939920454da7..6f3e8d981311 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -520,7 +520,7 @@ static void vfio_batch_fini(struct vfio_batch *batch)
 
 static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
 			    unsigned long vaddr, unsigned long *pfn,
-			    bool write_fault)
+			    unsigned long *pgmask, bool write_fault)
 {
 	struct follow_pfnmap_args args = { .vma = vma, .address = vaddr };
 	int ret;
@@ -544,10 +544,12 @@ static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
 			return ret;
 	}
 
-	if (write_fault && !args.writable)
+	if (write_fault && !args.writable) {
 		ret = -EFAULT;
-	else
+	} else {
 		*pfn = args.pfn;
+		*pgmask = args.pgmask;
+	}
 
 	follow_pfnmap_end(&args);
 	return ret;
@@ -590,15 +592,23 @@ static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
 	vma = vma_lookup(mm, vaddr);
 
 	if (vma && vma->vm_flags & VM_PFNMAP) {
-		ret = follow_fault_pfn(vma, mm, vaddr, pfn, prot & IOMMU_WRITE);
+		unsigned long pgmask;
+
+		ret = follow_fault_pfn(vma, mm, vaddr, pfn, &pgmask,
+				       prot & IOMMU_WRITE);
 		if (ret == -EAGAIN)
 			goto retry;
 
 		if (!ret) {
-			if (is_invalid_reserved_pfn(*pfn))
-				ret = 1;
-			else
+			if (is_invalid_reserved_pfn(*pfn)) {
+				unsigned long epfn;
+
+				epfn = (((*pfn << PAGE_SHIFT) + ~pgmask + 1)
+					& pgmask) >> PAGE_SHIFT;
+				ret = min_t(int, npages, epfn - *pfn);
+			} else {
 				ret = -EFAULT;
+			}
 		}
 	}
 done:
-- 
2.47.1

Re: [PATCH 5/5] vfio/type1: Use mapping page mask for pfnmaps

[Mitchell Augustin]

LGTM and completely eliminates guest VM PCI initialization slowdowns
on H100 and A100.
Also not seeing any obvious regressions on my side.

Reported-by: "Mitchell Augustin" <mitchell.augustin@canonical.com>
Reviewed-by: "Mitchell Augustin" <mitchell.augustin@canonical.com>
Tested-by: "Mitchell Augustin" <mitchell.augustin@canonical.com>


On Wed, Feb 5, 2025 at 5:18 PM Alex Williamson
<alex.williamson@redhat.com> wrote:
>
> vfio-pci supports huge_fault for PCI MMIO BARs and will insert pud and
> pmd mappings for well aligned mappings.  follow_pfnmap_start() walks the
> page table and therefore knows the page mask of the level where the
> address is found and returns this through follow_pfnmap_args.pgmask.
> Subsequent pfns from this address until the end of the mapping page are
> necessarily consecutive.  Use this information to retrieve a range of
> pfnmap pfns in a single pass.
>
> With optimal mappings and alignment on systems with 1GB pud and 4KB
> page size, this reduces iterations for DMA mapping PCI BARs by a
> factor of 256K.  In real world testing, the overhead of iterating
> pfns for a VM DMA mapping a 32GB PCI BAR is reduced from ~1s to
> sub-millisecond overhead.
>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  drivers/vfio/vfio_iommu_type1.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 939920454da7..6f3e8d981311 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -520,7 +520,7 @@ static void vfio_batch_fini(struct vfio_batch *batch)
>
>  static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
>                             unsigned long vaddr, unsigned long *pfn,
> -                           bool write_fault)
> +                           unsigned long *pgmask, bool write_fault)
>  {
>         struct follow_pfnmap_args args = { .vma = vma, .address = vaddr };
>         int ret;
> @@ -544,10 +544,12 @@ static int follow_fault_pfn(struct vm_area_struct *vma, struct mm_struct *mm,
>                         return ret;
>         }
>
> -       if (write_fault && !args.writable)
> +       if (write_fault && !args.writable) {
>                 ret = -EFAULT;
> -       else
> +       } else {
>                 *pfn = args.pfn;
> +               *pgmask = args.pgmask;
> +       }
>
>         follow_pfnmap_end(&args);
>         return ret;
> @@ -590,15 +592,23 @@ static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
>         vma = vma_lookup(mm, vaddr);
>
>         if (vma && vma->vm_flags & VM_PFNMAP) {
> -               ret = follow_fault_pfn(vma, mm, vaddr, pfn, prot & IOMMU_WRITE);
> +               unsigned long pgmask;
> +
> +               ret = follow_fault_pfn(vma, mm, vaddr, pfn, &pgmask,
> +                                      prot & IOMMU_WRITE);
>                 if (ret == -EAGAIN)
>                         goto retry;
>
>                 if (!ret) {
> -                       if (is_invalid_reserved_pfn(*pfn))
> -                               ret = 1;
> -                       else
> +                       if (is_invalid_reserved_pfn(*pfn)) {
> +                               unsigned long epfn;
> +
> +                               epfn = (((*pfn << PAGE_SHIFT) + ~pgmask + 1)
> +                                       & pgmask) >> PAGE_SHIFT;
> +                               ret = min_t(int, npages, epfn - *pfn);
> +                       } else {
>                                 ret = -EFAULT;
> +                       }
>                 }
>         }
>  done:
> --
> 2.47.1
>


--
Mitchell Augustin
Software Engineer - Ubuntu Partner Engineering

Re: [PATCH 5/5] vfio/type1: Use mapping page mask for pfnmaps

[Jason Gunthorpe]

On Wed, Feb 05, 2025 at 04:17:21PM -0700, Alex Williamson wrote:
> @@ -590,15 +592,23 @@ static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
>  	vma = vma_lookup(mm, vaddr);
>  
>  	if (vma && vma->vm_flags & VM_PFNMAP) {
> -		ret = follow_fault_pfn(vma, mm, vaddr, pfn, prot & IOMMU_WRITE);
> +		unsigned long pgmask;
> +
> +		ret = follow_fault_pfn(vma, mm, vaddr, pfn, &pgmask,
> +				       prot & IOMMU_WRITE);
>  		if (ret == -EAGAIN)
>  			goto retry;
>  
>  		if (!ret) {
> -			if (is_invalid_reserved_pfn(*pfn))
> -				ret = 1;
> -			else
> +			if (is_invalid_reserved_pfn(*pfn)) {
> +				unsigned long epfn;
> +
> +				epfn = (((*pfn << PAGE_SHIFT) + ~pgmask + 1)
> +					& pgmask) >> PAGE_SHIFT;

That seems a bit indirect

 epfn = ((*pfn) | (~pgmask >> PAGE_SHIFT)) + 1;

?

> +				ret = min_t(int, npages, epfn - *pfn);

It is nitty but the int's here should be long, and npages should be
unsigned long..

Jason

Re: [PATCH 5/5] vfio/type1: Use mapping page mask for pfnmaps

[Alex Williamson]

On Fri, 14 Feb 2025 15:27:04 -0400
Jason Gunthorpe <jgg@ziepe.ca> wrote:

> On Wed, Feb 05, 2025 at 04:17:21PM -0700, Alex Williamson wrote:
> > @@ -590,15 +592,23 @@ static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr,
> >  	vma = vma_lookup(mm, vaddr);
> >  
> >  	if (vma && vma->vm_flags & VM_PFNMAP) {
> > -		ret = follow_fault_pfn(vma, mm, vaddr, pfn, prot & IOMMU_WRITE);
> > +		unsigned long pgmask;
> > +
> > +		ret = follow_fault_pfn(vma, mm, vaddr, pfn, &pgmask,
> > +				       prot & IOMMU_WRITE);
> >  		if (ret == -EAGAIN)
> >  			goto retry;
> >  
> >  		if (!ret) {
> > -			if (is_invalid_reserved_pfn(*pfn))
> > -				ret = 1;
> > -			else
> > +			if (is_invalid_reserved_pfn(*pfn)) {
> > +				unsigned long epfn;
> > +
> > +				epfn = (((*pfn << PAGE_SHIFT) + ~pgmask + 1)
> > +					& pgmask) >> PAGE_SHIFT;  
> 
> That seems a bit indirect
> 
>  epfn = ((*pfn) | (~pgmask >> PAGE_SHIFT)) + 1;
> 
> ?

That is simpler, for sure.  Thanks!

> > +				ret = min_t(int, npages, epfn - *pfn);  
> 
> It is nitty but the int's here should be long, and npages should be
> unsigned long..

Added a new patch that uses unsigned long consistently for passed page
counts and long for returns.  Now we just need a system with a 16TiB
huge page size.  Thanks,

Alex

Re: [PATCH 5/5] vfio/type1: Use mapping page mask for pfnmaps

[Matthew Wilcox]

On Wed, Feb 05, 2025 at 04:17:21PM -0700, Alex Williamson wrote:
> +			if (is_invalid_reserved_pfn(*pfn)) {
> +				unsigned long epfn;
> +
> +				epfn = (((*pfn << PAGE_SHIFT) + ~pgmask + 1)
> +					& pgmask) >> PAGE_SHIFT;
> +				ret = min_t(int, npages, epfn - *pfn);

You've really made life hard for yourself by passing around a page mask
instead of an order (ie 0/PMD_ORDER/PUD_ORDER).  Why not:

				epfn = round_up(*pfn + 1, 1 << order);

Although if you insist on passing around a mask, this could be:

				unsigned long sz = (~pgmask >> PAGE_SHIFT) + 1;
				unsigned long epfn = round_up(*pfn + 1, sz)

Re: [PATCH 5/5] vfio/type1: Use mapping page mask for pfnmaps

[Alex Williamson]

On Fri, 14 Feb 2025 19:46:22 +0000
Matthew Wilcox <willy@infradead.org> wrote:

> On Wed, Feb 05, 2025 at 04:17:21PM -0700, Alex Williamson wrote:
> > +			if (is_invalid_reserved_pfn(*pfn)) {
> > +				unsigned long epfn;
> > +
> > +				epfn = (((*pfn << PAGE_SHIFT) + ~pgmask + 1)
> > +					& pgmask) >> PAGE_SHIFT;
> > +				ret = min_t(int, npages, epfn - *pfn);  
> 
> You've really made life hard for yourself by passing around a page mask
> instead of an order (ie 0/PMD_ORDER/PUD_ORDER).  Why not:
> 
> 				epfn = round_up(*pfn + 1, 1 << order);
> 
> Although if you insist on passing around a mask, this could be:
> 
> 				unsigned long sz = (~pgmask >> PAGE_SHIFT) + 1;
> 				unsigned long epfn = round_up(*pfn + 1, sz)
> 

Hey Willy!

I was wishing I had an order, but I didn't want to mangle
follow_pfnmap_start() and follow_pfnmap_setup() too much.  Currently
the latter is doing:

	args->pfn = pfn_base + ((args->address & ~addr_mask) >> PAGE_SHIFT);

If follow_pfnmap_start() passed an order, this would need to change to
something equivalent to:

	args->pfn = pfn_base + ((args->address >> PAGE_SHIFT) & ((1UL << order) - 1));

Looks pretty ugly as well, so maybe I'm just shifting the ugliness
around, or maybe someone can spot a more elegant representation.
Thanks,

Alex

Re: [PATCH 0/5] vfio: Improve DMA mapping performance for huge pfnmaps

[Peter Xu]

On Wed, Feb 05, 2025 at 04:17:16PM -0700, Alex Williamson wrote:
> As GPU BAR sizes increase, the overhead of DMA mapping pfnmap ranges has
> become a significant overhead for VMs making use of device assignment.
> Not only does each mapping require upwards of a few seconds, but BARs
> are mapped in and out of the VM address space multiple times during
> guest boot.  Also factor in that multi-GPU configurations are
> increasingly commonplace and BAR sizes are continuing to increase.
> Configurations today can already be delayed minutes during guest boot.
> 
> We've taken steps to make Linux a better guest by batching PCI BAR
> sizing operations[1], but it only provides and incremental improvement.
> 
> This series attempts to fully address the issue by leveraging the huge
> pfnmap support added in v6.12.  When we insert pfnmaps using pud and pmd
> mappings, we can later take advantage of the knowledge of the mapping
> level page mask to iterate on the relevant mapping stride.  In the
> commonly achieved optimal case, this results in a reduction of pfn
> lookups by a factor of 256k.  For a local test system, an overhead of
> ~1s for DMA mapping a 32GB PCI BAR is reduced to sub-millisecond (8M
> page sized operations reduced to 32 pud sized operations).
> 
> Please review, test, and provide feedback.  I hope that mm folks can
> ack the trivial follow_pfnmap_args update to provide the mapping level
> page mask.  Naming is hard, so any preference other than pgmask is
> welcome.  Thanks,
> 
> Alex
> 
> [1]https://lore.kernel.org/all/20250120182202.1878581-1-alex.williamson@redhat.com/
> 
> 
> Alex Williamson (5):
>   vfio/type1: Catch zero from pin_user_pages_remote()
>   vfio/type1: Convert all vaddr_get_pfns() callers to use vfio_batch
>   vfio/type1: Use vfio_batch for vaddr_get_pfns()
>   mm: Provide page mask in struct follow_pfnmap_args
>   vfio/type1: Use mapping page mask for pfnmaps

FWIW:

Reviewed-by: Peter Xu <peterx@redhat.com>

Thanks,

-- 
Peter Xu

Re: [PATCH 0/5] vfio: Improve DMA mapping performance for huge pfnmaps

[Mitchell Augustin]

Thanks Alex, this all looks great to me and completely eliminates the
boot time slowdown I was seeing in my tests on our DGX H100 and A100.
I also double-checked the memory mappings reported in /proc/iomem, and
everything looks consistent with how it was prior to this series on
both devices.

Reported-by: "Mitchell Augustin" <mitchell.augustin@canonical.com>
Reviewed-by: "Mitchell Augustin" <mitchell.augustin@canonical.com>
Tested-by: "Mitchell Augustin" <mitchell.augustin@canonical.com>


On Wed, Feb 5, 2025 at 5:18 PM Alex Williamson
<alex.williamson@redhat.com> wrote:
>
> As GPU BAR sizes increase, the overhead of DMA mapping pfnmap ranges has
> become a significant overhead for VMs making use of device assignment.
> Not only does each mapping require upwards of a few seconds, but BARs
> are mapped in and out of the VM address space multiple times during
> guest boot.  Also factor in that multi-GPU configurations are
> increasingly commonplace and BAR sizes are continuing to increase.
> Configurations today can already be delayed minutes during guest boot.
>
> We've taken steps to make Linux a better guest by batching PCI BAR
> sizing operations[1], but it only provides and incremental improvement.
>
> This series attempts to fully address the issue by leveraging the huge
> pfnmap support added in v6.12.  When we insert pfnmaps using pud and pmd
> mappings, we can later take advantage of the knowledge of the mapping
> level page mask to iterate on the relevant mapping stride.  In the
> commonly achieved optimal case, this results in a reduction of pfn
> lookups by a factor of 256k.  For a local test system, an overhead of
> ~1s for DMA mapping a 32GB PCI BAR is reduced to sub-millisecond (8M
> page sized operations reduced to 32 pud sized operations).
>
> Please review, test, and provide feedback.  I hope that mm folks can
> ack the trivial follow_pfnmap_args update to provide the mapping level
> page mask.  Naming is hard, so any preference other than pgmask is
> welcome.  Thanks,
>
> Alex
>
> [1]https://lore.kernel.org/all/20250120182202.1878581-1-alex.williamson@redhat.com/
>
>
> Alex Williamson (5):
>   vfio/type1: Catch zero from pin_user_pages_remote()
>   vfio/type1: Convert all vaddr_get_pfns() callers to use vfio_batch
>   vfio/type1: Use vfio_batch for vaddr_get_pfns()
>   mm: Provide page mask in struct follow_pfnmap_args
>   vfio/type1: Use mapping page mask for pfnmaps
>
>  drivers/vfio/vfio_iommu_type1.c | 107 ++++++++++++++++++++------------
>  include/linux/mm.h              |   2 +
>  mm/memory.c                     |   1 +
>  3 files changed, 72 insertions(+), 38 deletions(-)
>
> --
> 2.47.1
>


--
Mitchell Augustin
Software Engineer - Ubuntu Partner Engineering