All of 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.


WARNING: multiple messages have this Message-ID (diff)
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.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

Thread overview: 41+ 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 ` 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-16 10:59   ` Mike Rapoport (Microsoft)
2026-08-16 11:12   ` sashiko-bot
2026-08-17 12:10   ` Brendan Jackman
2026-08-17 12:10     ` Brendan Jackman
2026-08-19  6:52     ` Mike Rapoport
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-16 10:59   ` Mike Rapoport (Microsoft)
2026-08-16 11:08   ` sashiko-bot
2026-08-17 17:31   ` Uladzislau Rezki
2026-08-17 17:31     ` Uladzislau Rezki
2026-08-19  6:58     ` Mike Rapoport
2026-08-19  6:58       ` Mike Rapoport
2026-08-19 11:48       ` Uladzislau Rezki
2026-08-19 11:48         ` 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   ` Mike Rapoport (Microsoft)
2026-08-16 11:03   ` sashiko-bot
2026-08-19 11:49   ` Uladzislau Rezki
2026-08-19 11:49     ` Uladzislau Rezki
2026-08-16 10:59 ` [PATCH 4/6] mm/vmalloc: make set_area_direct_map HUGE_VMAP friendly Mike Rapoport (Microsoft)
2026-08-16 10:59   ` Mike Rapoport (Microsoft)
2026-08-16 11:10   ` sashiko-bot
2026-08-17 12:21   ` Brendan Jackman
2026-08-17 12:21     ` Brendan Jackman
2026-08-19  7:01     ` Mike Rapoport
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   ` Mike Rapoport (Microsoft)
2026-08-16 11:17   ` sashiko-bot
2026-08-19  7:03     ` Mike Rapoport
2026-08-16 10:59 ` [PATCH 6/6] Revert "arch: introduce set_direct_map_valid_noflush()" Mike Rapoport (Microsoft)
2026-08-16 10:59   ` Mike Rapoport (Microsoft)
2026-08-16 11:03   ` sashiko-bot
2026-08-17 15:14   ` Brendan Jackman
2026-08-17 15:14     ` Brendan Jackman
2026-08-19  7:13     ` Mike Rapoport [this message]
2026-08-19  7:13       ` Mike Rapoport

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.