On Mon, Oct 12, 2026 at 08:25:54PM +0200, David Hildenbrand (Arm) wrote:
> On 10/9/26 20:04, Lorenzo Stoakes (ARM) wrote:
> > It makes no sense for a mapping whose contents the kernel does not own to
> > specify that the range is MMIO.
> >
> > Prior to this patch, all in-tree drivers which did so have been updated
> > such that they are marked as not being mm-managed. The check WARNs and
> > fails the mmap for any out-of-tree driver that still sets VMA_IO_BIT
> > without a kernel mapping.
> >
> > No functional change intended for in-tree code.
> >
> > Reviewed-by: Zi Yan <[email protected]>
> > Signed-off-by: Lorenzo Stoakes (ARM) <[email protected]>
> > ---
> >  mm/vma.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/mm/vma.c b/mm/vma.c
> > index 4f09fb98a2fd..51bdbe08bb02 100644
> > --- a/mm/vma.c
> > +++ b/mm/vma.c
> > @@ -2796,6 +2796,12 @@ static int mmap_validate_vma_flags(const vma_flags_t 
> > *flags)
> >             return -EINVAL;
> >  #endif
> >
> > +   if (!vma_flags_test_any(flags, VMA_PFNMAP_BIT, VMA_MIXEDMAP_BIT)) {
> > +           /* mm-managed mappings may not set VMA_IO_BIT. */
> > +           if (WARN_ON_ONCE(vma_flags_test(flags, VMA_IO_BIT)))
> > +                   return -EINVAL;
>
> We do have a helper (vma_flags_is_mm_managed()) but we're not using it because
> of the VM_IO situation ...

That gets removed.

>
> That's confusing ...
>
> Could we do something like testing vma_flags_is_mm_managed(), but removing the
> VM_IO flag from the checked flags?

I mean we do that in a subsequent commit, I think even the next one?


>
> Then we can add a comment about VM_IO in the future getting remove from
> vma_flags_is_mm_managed() ...

Yeah, but to do that the code would now be something like:

        vma_flags_t masked = flags;

        /*
          * vma_flags_is_mm_managed() tests VMA_IO_BIT, which will be removed
         * in the next commit.
         */
        vma_flags_clear(&flags, VMA_IO_BIT);
        if (!vma_flags_is_mm_managed(masked)) {
                ...
        }

Is that really worth it?

I don't think there's really too much confusion intra-patch about that is there?

--
Cheers, Lorenzo

Reply via email to