Re: [PATCH 6/6] Revert "arch: introduce set_direct_map_valid_noflush()"

On Mon, Aug 17, 2026 at 05:14:19PM +0200, Brendan Jackman wrote:
> On Sun Aug 16, 2026 at 12:59 PM CEST, Mike Rapoport (Microsoft) wrote:
> > Commit 0c6378a71574 ("arch: introduce set_direct_map_valid_noflush()")
> > added set_direct_map_valid_noflush() to allow updating the direct map
> > for a physically contiguous range in execmem.
> >
> > As Brendan recently pointed out [1], this API is confusing because on
> > arm64 it means that is sets VALID bit in ptes, while on other
> > architectures it is an analog of set_direct_map_default_noflush().
> >
> > The only user of set_direct_map_valid_noflush() was execmem's ROX cache
> > freeing path and it was switched to utilize VM_FLUSH_RESET_PERMS for
> > resetting permissions of the direct map alias.
> >
> > With the last user gone and with set_direct_map_{invalid,default}_noflush()
> > accepting number of pages as a parameter, set_direct_map_valid_noflush()
> > become a copy of set_memory_valid() on arm64 and a duplicate of
> > set_direct_map_{invalid,default}_noflush() on other architecture, it is
> > safe to remove set_direct_map_valid_noflush().
> >
> > Also drop a stale comment in arm64::__kernel_map_pages() that Linus
> > bothered to add when merging changes containing set_direct_map_valid_noflush()
> > to his tree.
> >
> > This reverts commit 0c6378a71574daa6cd1534ad42a956e3262756c7.
> >
> > [1] https://lore.kernel.org/all/[email protected]
> >
> > Signed-off-by: Mike Rapoport (Microsoft) <[email protected]>
> 
> So basically the big issue here is specifically that
> set_direct_map_valid_noflush(..., true) is special on arm64 and not x86.

More precisely arm64 differers from all the rest.
 
> And we fix that by jut deleting the API. SGTM!
> 
> The other issue I can see here is that set_direct_map_invalid_noflush()
> clears RW on x86 but doesn't set RDONLY on arm64. So if you unmap
> something using set_direct_map_invalid_noflush(), then map it again
> using something other than set_direct_map_default_noflush(), you get
> different behaviour between the archs.
> 
> I think the answer to that is probably: doing that is a bug, i.e.
> _invalid_noflush() and _default_noflush() are a pair that you have to
> use together. But I haven't checked if this is currently the case. Maybe
> it would still make sense to just align these fully?

All set_memory + set_direct_map functions very much rely on caller to know
how to use them right. There were only handful of users outside of arch/ so
it was kinda sustainable. With the increased demand for those, security and
whatnot, the whole bunch of set_memory functions needs some love.
 
> Anyway, aside from all this yapping, getting rid of _valid_noflush()
> seems like an unambiguous win here so thanks for the cleanup!
> 
> Reviewed-by: Brendan Jackman <[email protected]>

-- 
Sincerely yours,
Mike.