From: "Brendan Jackman" <brendan.jackman@linux.dev>
To: "Mike Rapoport (Microsoft)" <rppt@kernel.org>,
"Andrew Morton" <akpm@linux-foundation.org>
Cc: "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>,
"Brendan Jackman" <brendan.jackman@linux.dev>,
"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: Mon, 17 Aug 2026 17:14:19 +0200 [thread overview]
Message-ID: <DKRBF6LQY1L7.13CIBHAU0A87D@linux.dev> (raw)
In-Reply-To: <20260816-execmem-set-vm-perms-v0-2-v1-6-90944a3ad43f@kernel.org>
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>
Quick dump of my understanding (ignoring the NG bit on arm64)
- set_direct_map_invalid_noflush():
x86: clear P and RW (and DIRTY)
arm64: clear VALID
So these look out of sync to me
- set_direct_map_default_noflush():
x86: set P and RW
arm64: set VALID and WRITE, clear RDONLY
- set_direct_map_valid_noflush(..., true):
x86: exactly the same as set_direct_map_default_noflush()
(but with a `nr` arg)
arm64: set VALID
- set_direct_map_valid_noflush(..., false):
both archs: exactly the same as set_direct_map_invalid_noflush()
(but with a `nr` arg)
So basically the big issue here is specifically that
set_direct_map_valid_noflush(..., true) is special on arm64 and not x86.
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?
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>
prev parent reply other threads:[~2026-08-17 15:14 UTC|newest]
Thread overview: 11+ 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-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-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-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 [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=DKRBF6LQY1L7.13CIBHAU0A87D@linux.dev \
--to=brendan.jackman@linux.dev \
--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=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=rppt@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