Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Rapoport <rppt@kernel.org>
To: Brendan Jackman <brendan.jackman@linux.dev>
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
	"Adrian Barnaś" <abarnas@google.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Alexander Gordeev" <agordeev@linux.ibm.com>,
	"Alexandre Ghiti" <alex@ghiti.fr>,
	"Andy Lutomirski" <luto@kernel.org>,
	"Borislav Petkov" <bp@alien8.de>,
	"Catalin Marinas" <catalin.marinas@arm.com>,
	"Christian Borntraeger" <borntraeger@linux.ibm.com>,
	"Dave Hansen" <dave.hansen@linux.intel.com>,
	"David Hildenbrand" <david@kernel.org>,
	"Gerald Schaefer" <gerald.schaefer@linux.ibm.com>,
	"Heiko Carstens" <hca@linux.ibm.com>,
	"Huacai Chen" <chenhuacai@kernel.org>,
	"Ingo Molnar" <mingo@redhat.com>, "Len Brown" <lenb@kernel.org>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Paul Walmsley" <pjw@kernel.org>,
	"Pavel Machek" <pavel@kernel.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Ryan Roberts" <ryan.roberts@arm.com>,
	"Sven Schnelle" <svens@linux.ibm.com>,
	"Thomas Gleixner" <tglx@kernel.org>,
	"Uladzislau Rezki" <urezki@gmail.com>,
	"Vasily Gorbik" <gor@linux.ibm.com>,
	"WANG Xuerui" <kernel@xen0n.name>,
	"Will Deacon" <will@kernel.org>,
	x86@kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-pm@vger.kernel.org, linux-riscv@lists.infradead.org,
	linux-s390@vger.kernel.org, loongarch@lists.linux.dev,
	yosry@kernel.org
Subject: Re: [PATCH 6/6] Revert "arch: introduce set_direct_map_valid_noflush()"
Date: Wed, 19 Aug 2026 10:13:38 +0300	[thread overview]
Message-ID: <aoVXolZ1pEJBZNjF@kernel.org> (raw)
In-Reply-To: <DKRBF6LQY1L7.13CIBHAU0A87D@linux.dev>

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/DJ69RCVRBO0Y.3JCYSW50IC4RC@linux.dev
> >
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> 
> 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 <brendan.jackman@linux.dev>

-- 
Sincerely yours,
Mike.


      reply	other threads:[~2026-08-19  7:14 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-16 10:59 [PATCH 0/6] arch, mm/execmem: resolve confusion about set_direct_map_valid_noflush() Mike Rapoport (Microsoft)
2026-08-16 10:59 ` [PATCH 1/6] set_memory: add number of pages parameter to set_direct_map APIs Mike Rapoport (Microsoft)
2026-08-17 12:10   ` Brendan Jackman
2026-08-19  6:52     ` Mike Rapoport
2026-08-16 10:59 ` [PATCH 2/6] mm/vmalloc: set area's page_order after allocation succeeds Mike Rapoport (Microsoft)
2026-08-17 17:31   ` Uladzislau Rezki
2026-08-19  6:58     ` Mike Rapoport
2026-08-16 10:59 ` [PATCH 3/6] mm/vmalloc: constify vm parameter of get_vm_area_page_order() Mike Rapoport (Microsoft)
2026-08-16 10:59 ` [PATCH 4/6] mm/vmalloc: make set_area_direct_map HUGE_VMAP friendly Mike Rapoport (Microsoft)
2026-08-17 12:21   ` Brendan Jackman
2026-08-19  7:01     ` Mike Rapoport
2026-08-16 10:59 ` [PATCH 5/6] mm/execmem: use VM_FLUSH_RESET_PERMS for ROX cache allocations Mike Rapoport (Microsoft)
2026-08-16 10:59 ` [PATCH 6/6] Revert "arch: introduce set_direct_map_valid_noflush()" Mike Rapoport (Microsoft)
2026-08-17 15:14   ` Brendan Jackman
2026-08-19  7:13     ` Mike Rapoport [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aoVXolZ1pEJBZNjF@kernel.org \
    --to=rppt@kernel.org \
    --cc=abarnas@google.com \
    --cc=agordeev@linux.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex@ghiti.fr \
    --cc=aou@eecs.berkeley.edu \
    --cc=borntraeger@linux.ibm.com \
    --cc=bp@alien8.de \
    --cc=brendan.jackman@linux.dev \
    --cc=catalin.marinas@arm.com \
    --cc=chenhuacai@kernel.org \
    --cc=dave.hansen@linux.intel.com \
    --cc=david@kernel.org \
    --cc=gerald.schaefer@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=hpa@zytor.com \
    --cc=kernel@xen0n.name \
    --cc=lenb@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=loongarch@lists.linux.dev \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=palmer@dabbelt.com \
    --cc=pavel@kernel.org \
    --cc=peterz@infradead.org \
    --cc=pjw@kernel.org \
    --cc=rafael@kernel.org \
    --cc=ryan.roberts@arm.com \
    --cc=svens@linux.ibm.com \
    --cc=tglx@kernel.org \
    --cc=urezki@gmail.com \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    --cc=yosry@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox