Re: [PATCH v2 06/16] mm: add mmap_action_simple_ioremap()

["Lorenzo Stoakes (Oracle)" <ljs@kernel.org>]

On Mon, Mar 16, 2026 at 09:14:28PM -0700, Suren Baghdasaryan wrote:
> On Mon, Mar 16, 2026 at 2:13 PM Lorenzo Stoakes (Oracle) <ljs@kernel.org> wrote:
> >
> > Currently drivers use vm_iomap_memory() as a simple helper function for
> > I/O remapping memory over a range starting at a specified physical address
> > over a specified length.
> >
> > In order to utilise this from mmap_prepare, separate out the core logic
> > into __simple_ioremap_prep(), update vm_iomap_memory() to use it, and add
> > simple_ioremap_prepare() to do the same with a VMA descriptor object.
> >
> > We also add MMAP_SIMPLE_IO_REMAP and relevant fields to the struct
> > mmap_action type to permit this operation also.
> >
> > We use mmap_action_ioremap() to set up the actual I/O remap operation once
> > we have checked and figured out the parameters, which makes
> > simple_ioremap_prepare() easy to implement.
> >
> > We then add mmap_action_simple_ioremap() to allow drivers to make use of
> > this mode.
> >
> > We update the mmap_prepare documentation to describe this mode.
> >
> > Finally, we update the VMA tests to reflect this change.
> >
> > Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
>
> A couple of nits, but otherwise LGTM.
>
> Reviewed-by: Suren Baghdasaryan <surenb@google.com>

Thanks!

>
> > ---
> >  Documentation/filesystems/mmap_prepare.rst |  3 +
> >  include/linux/mm.h                         | 24 +++++-
> >  include/linux/mm_types.h                   |  6 +-
> >  mm/internal.h                              |  2 +
> >  mm/memory.c                                | 87 +++++++++++++++-------
> >  mm/util.c                                  | 12 +++
> >  tools/testing/vma/include/dup.h            |  6 +-
> >  7 files changed, 112 insertions(+), 28 deletions(-)
> >
> > diff --git a/Documentation/filesystems/mmap_prepare.rst b/Documentation/filesystems/mmap_prepare.rst
> > index 20db474915da..be76ae475b9c 100644
> > --- a/Documentation/filesystems/mmap_prepare.rst
> > +++ b/Documentation/filesystems/mmap_prepare.rst
> > @@ -153,5 +153,8 @@ pointer. These are:
> >  * mmap_action_ioremap_full() - Same as mmap_action_ioremap(), only remaps
> >    the entire mapping from ``start_pfn`` onward.
> >
> > +* mmap_action_simple_ioremap() - Sets up an I/O remap from a specified
> > +  physical address and over a specified length.
> > +
> >  **NOTE:** The ``action`` field should never normally be manipulated directly,
> >  rather you ought to use one of these helpers.
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index ad1b8c3c0cfd..df8fa6e6402b 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -4337,11 +4337,33 @@ static inline void mmap_action_ioremap(struct vm_area_desc *desc,
> >   * @start_pfn: The first PFN in the range to remap.
> >   */
> >  static inline void mmap_action_ioremap_full(struct vm_area_desc *desc,
> > -                                         unsigned long start_pfn)
> > +                                           unsigned long start_pfn)
> >  {
> >         mmap_action_ioremap(desc, desc->start, start_pfn, vma_desc_size(desc));
> >  }
> >
> > +/**
> > + * mmap_action_simple_ioremap - helper for mmap_prepare hook to specify that the
> > + * physical range in [start_phys_addr, start_phys_addr + size) should be I/O
> > + * remapped.
> > + * @desc: The VMA descriptor for the VMA requiring remap.
> > + * @start_phys_addr: Start of the physical memory to be mapped.
> > + * @size: Size of the area to map.
> > + *
> > + * NOTE: Some drivers might want to tweak desc->page_prot for purposes of
> > + * write-combine or similar.
> > + */
> > +static inline void mmap_action_simple_ioremap(struct vm_area_desc *desc,
> > +                                             phys_addr_t start_phys_addr,
> > +                                             unsigned long size)
> > +{
> > +       struct mmap_action *action = &desc->action;
> > +
> > +       action->simple_ioremap.start_phys_addr = start_phys_addr;
> > +       action->simple_ioremap.size = size;
> > +       action->type = MMAP_SIMPLE_IO_REMAP;
> > +}
> > +
> >  int mmap_action_prepare(struct vm_area_desc *desc);
> >  int mmap_action_complete(struct vm_area_struct *vma,
> >                          struct mmap_action *action);
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index 4a229cc0a06b..50685cf29792 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -814,6 +814,7 @@ enum mmap_action_type {
> >         MMAP_NOTHING,           /* Mapping is complete, no further action. */
> >         MMAP_REMAP_PFN,         /* Remap PFN range. */
> >         MMAP_IO_REMAP_PFN,      /* I/O remap PFN range. */
> > +       MMAP_SIMPLE_IO_REMAP,   /* I/O remap with guardrails. */
> >  };
> >
> >  /*
> > @@ -822,13 +823,16 @@ enum mmap_action_type {
> >   */
> >  struct mmap_action {
> >         union {
> > -               /* Remap range. */
> >                 struct {
> >                         unsigned long start;
> >                         unsigned long start_pfn;
> >                         unsigned long size;
> >                         pgprot_t pgprot;
> >                 } remap;
> > +               struct {
> > +                       phys_addr_t start_phys_addr;
> > +                       unsigned long size;
> > +               } simple_ioremap;
> >         };
> >         enum mmap_action_type type;
> >
> > diff --git a/mm/internal.h b/mm/internal.h
> > index f5774892071e..0eaca2f0eb6a 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -1804,6 +1804,8 @@ int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm);
> >  int remap_pfn_range_prepare(struct vm_area_desc *desc);
> >  int remap_pfn_range_complete(struct vm_area_struct *vma,
> >                              struct mmap_action *action);
> > +int simple_ioremap_prepare(struct vm_area_desc *desc);
> > +/* No simple_ioremap_complete, is ultimately handled by remap complete. */
> >
> >  static inline int io_remap_pfn_range_prepare(struct vm_area_desc *desc)
> >  {
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 9dec67a18116..f3f4046aee97 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -3170,6 +3170,59 @@ int remap_pfn_range_complete(struct vm_area_struct *vma,
> >         return do_remap_pfn_range(vma, start, pfn, size, prot);
> >  }
> >
> > +static int __simple_ioremap_prep(unsigned long vm_start, unsigned long vm_end,
>
> nit: vm_start and vm_end are used only to calculate vm_len. You could
> reduce the number of arguments by just passing vm_len.

Ack will fixup!

>
> > +                                pgoff_t vm_pgoff, phys_addr_t start_phys,
> > +                                unsigned long size, unsigned long *pfnp)
> > +{
> > +       const unsigned long vm_len = vm_end - vm_start;
> > +       unsigned long pfn, pages;
> > +
> > +       /* Check that the physical memory area passed in looks valid */
> > +       if (start_phys + size < start_phys)
> > +               return -EINVAL;
> > +       /*
> > +        * You *really* shouldn't map things that aren't page-aligned,
> > +        * but we've historically allowed it because IO memory might
> > +        * just have smaller alignment.
> > +        */
> > +       size += start_phys & ~PAGE_MASK;
> > +       pfn = start_phys >> PAGE_SHIFT;
> > +       pages = (size + ~PAGE_MASK) >> PAGE_SHIFT;
> > +       if (pfn + pages < pfn)
> > +               return -EINVAL;
> > +
> > +       /* We start the mapping 'vm_pgoff' pages into the area */
> > +       if (vm_pgoff > pages)
> > +               return -EINVAL;
> > +       pfn += vm_pgoff;
> > +       pages -= vm_pgoff;
> > +
> > +       /* Can we fit all of the mapping? */
> > +       if ((vm_len >> PAGE_SHIFT) > pages)
> > +               return -EINVAL;
> > +
> > +       *pfnp = pfn;
> > +       return 0;
> > +}
> > +
> > +int simple_ioremap_prepare(struct vm_area_desc *desc)
> > +{
> > +       struct mmap_action *action = &desc->action;
> > +       const phys_addr_t start = action->simple_ioremap.start_phys_addr;
> > +       const unsigned long size = action->simple_ioremap.size;
> > +       unsigned long pfn;
> > +       int err;
> > +
> > +       err = __simple_ioremap_prep(desc->start, desc->end, desc->pgoff,
> > +                                   start, size, &pfn);
> > +       if (err)
> > +               return err;
> > +
> > +       /* The I/O remap logic does the heavy lifting. */
> > +       mmap_action_ioremap(desc, desc->start, pfn, vma_desc_size(desc));
>
> nit: Looks like a perfect opportunity to use mmap_action_ioremap_full() here.

Yeah can do!

>
> > +       return mmap_action_prepare(desc);
>
> Ok, so IIUC this uses recursion:
> mmap_action_prepare(MMAP_SIMPLE_IO_REMAP) -> simple_ioremap_prepare()
> -> mmap_action_prepare(MMAP_IO_REMAP_PFN).

Yep, it's one level, I think that should be ok? :)

>
> > +}
> > +
> >  /**
> >   * vm_iomap_memory - remap memory to userspace
> >   * @vma: user vma to map to
> > @@ -3187,32 +3240,16 @@ int remap_pfn_range_complete(struct vm_area_struct *vma,
> >   */
> >  int vm_iomap_memory(struct vm_area_struct *vma, phys_addr_t start, unsigned long len)
> >  {
> > -       unsigned long vm_len, pfn, pages;
> > -
> > -       /* Check that the physical memory area passed in looks valid */
> > -       if (start + len < start)
> > -               return -EINVAL;
> > -       /*
> > -        * You *really* shouldn't map things that aren't page-aligned,
> > -        * but we've historically allowed it because IO memory might
> > -        * just have smaller alignment.
> > -        */
> > -       len += start & ~PAGE_MASK;
> > -       pfn = start >> PAGE_SHIFT;
> > -       pages = (len + ~PAGE_MASK) >> PAGE_SHIFT;
> > -       if (pfn + pages < pfn)
> > -               return -EINVAL;
> > -
> > -       /* We start the mapping 'vm_pgoff' pages into the area */
> > -       if (vma->vm_pgoff > pages)
> > -               return -EINVAL;
> > -       pfn += vma->vm_pgoff;
> > -       pages -= vma->vm_pgoff;
> > +       const unsigned long vm_start = vma->vm_start;
> > +       const unsigned long vm_end = vma->vm_end;
> > +       const unsigned long vm_len = vm_end - vm_start;
> > +       unsigned long pfn;
> > +       int err;
> >
> > -       /* Can we fit all of the mapping? */
> > -       vm_len = vma->vm_end - vma->vm_start;
> > -       if (vm_len >> PAGE_SHIFT > pages)
> > -               return -EINVAL;
> > +       err = __simple_ioremap_prep(vm_start, vm_end, vma->vm_pgoff, start,
> > +                                   len, &pfn);
> > +       if (err)
> > +               return err;
> >
> >         /* Ok, let it rip */
> >         return io_remap_pfn_range(vma, vma->vm_start, pfn, vm_len, vma->vm_page_prot);
> > diff --git a/mm/util.c b/mm/util.c
> > index cdfba09e50d7..aa92e471afe1 100644
> > --- a/mm/util.c
> > +++ b/mm/util.c
> > @@ -1390,6 +1390,8 @@ int mmap_action_prepare(struct vm_area_desc *desc)
> >                 return remap_pfn_range_prepare(desc);
> >         case MMAP_IO_REMAP_PFN:
> >                 return io_remap_pfn_range_prepare(desc);
> > +       case MMAP_SIMPLE_IO_REMAP:
> > +               return simple_ioremap_prepare(desc);
> >         }
> >
> >         WARN_ON_ONCE(1);
> > @@ -1421,6 +1423,14 @@ int mmap_action_complete(struct vm_area_struct *vma,
> >         case MMAP_IO_REMAP_PFN:
> >                 err = io_remap_pfn_range_complete(vma, action);
> >                 break;
> > +       case MMAP_SIMPLE_IO_REMAP:
> > +               /*
> > +                * The simple I/O remap should have been delegated to an I/O
> > +                * remap.
> > +                */
> > +               WARN_ON_ONCE(1);
> > +               err = -EINVAL;
> > +               break;
> >         }
> >
> >         return mmap_action_finish(vma, action, err);
> > @@ -1434,6 +1444,7 @@ int mmap_action_prepare(struct vm_area_desc *desc)
> >                 break;
> >         case MMAP_REMAP_PFN:
> >         case MMAP_IO_REMAP_PFN:
> > +       case MMAP_SIMPLE_IO_REMAP:
> >                 WARN_ON_ONCE(1); /* nommu cannot handle these. */
> >                 break;
> >         }
> > @@ -1452,6 +1463,7 @@ int mmap_action_complete(struct vm_area_struct *vma,
> >                 break;
> >         case MMAP_REMAP_PFN:
> >         case MMAP_IO_REMAP_PFN:
> > +       case MMAP_SIMPLE_IO_REMAP:
> >                 WARN_ON_ONCE(1); /* nommu cannot handle this. */
> >
> >                 err = -EINVAL;
> > diff --git a/tools/testing/vma/include/dup.h b/tools/testing/vma/include/dup.h
> > index 4570ec77f153..114daaef4f73 100644
> > --- a/tools/testing/vma/include/dup.h
> > +++ b/tools/testing/vma/include/dup.h
> > @@ -453,6 +453,7 @@ enum mmap_action_type {
> >         MMAP_NOTHING,           /* Mapping is complete, no further action. */
> >         MMAP_REMAP_PFN,         /* Remap PFN range. */
> >         MMAP_IO_REMAP_PFN,      /* I/O remap PFN range. */
> > +       MMAP_SIMPLE_IO_REMAP,   /* I/O remap with guardrails. */
> >  };
> >> >  /*
> > @@ -461,13 +462,16 @@ enum mmap_action_type {
> >   */
> >  struct mmap_action {
> >         union {
> > -               /* Remap range. */
> >                 struct {
> >                         unsigned long start;
> >                         unsigned long start_pfn;
> >                         unsigned long size;
> >                         pgprot_t pgprot;
> >                 } remap;
> > +               struct {
> > +                       phys_addr_t start;
> > +                       unsigned long len;
> > +               } simple_ioremap;
> >         };
> >         enum mmap_action_type type;
> >
> > --
> > 2.53.0
> >

Cheers, Lorenzo