On Mon, Oct 12, 2026 at 08:39:35PM +0200, David Hildenbrand (Arm) wrote:
> On 10/12/26 20:35, Lorenzo Stoakes (ARM) wrote:
> > 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?
>
> I would do it as above (- the comment, as that is the next patch indeed) and
> only remove the vma_flags_clear() in the next patch.
>
> But in any case, you should spell out that this is only temporary in the patch
> description :)
And then there's a 'masked' variable that is unused?
You need to have two vma_flags_t variables to be able both clear VMA_IO_BIT
and use vma_flags_is_mm_managed().
So the change would have to be
+ vma_flags_t masked = *flags;
+
+ vma_flags_clear(&flags, VMA_IO_BIT);
+ if (!vma_flags_is_mm_managed(&masked)) {
+ ...
+ }
Then:
- vma_flags_t masked = *flags;
-
- vma_flags_clear(&flags, VMA_IO_BIT);
- if (!vma_flags_is_mm_managed(&masked)) {
+ if (!vma_flags_is_mm_managed(flags)) {
...
}
Which doesn't really seem worth it for somebody who is confused between two
patches mid-series.
But if you feel strongly I can make the change like that?
--
Cheers, Lorenzo
