mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mike Rapoport <rppt@kernel.org>
To: Kevin Brodsky <kevin.brodsky@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	"Liam R. Howlett" <liam@infradead.org>,
	Lorenzo Stoakes <ljs@kernel.org>, Michal Hocko <mhocko@suse.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Vlastimil Babka <vbabka@kernel.org>,
	linux-arch@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH] docs/mm: describe set_memory() and set_direct_map() APIs
Date: Fri, 18 Sep 2026 10:21:21 +0300	[thread overview]
Message-ID: <aqzmcdr7ISpa9r8k@kernel.org> (raw)
In-Reply-To: <2817ca55-cf04-4874-bed6-8408606e1a41@arm.com>

On Thu, Sep 17, 2026 at 08:44:17PM +0200, Kevin Brodsky wrote:
> On 09/09/2026 11:45, Mike Rapoport (Microsoft) wrote:
> > The set_memory() and set_direct_map() APIs change permissions of existing
> > kernel mappings, but their semantics are only described by the code, and
> > that code differs from architecture to architecture.
> >
> > Add Documentation/mm/kernel-page-tables.rst that briefly describes what the
> > kernel page tables consist of, defines the semantics both APIs have in
> > common, including the parts that are easy to get wrong, and lists the
> > differences between the architecture implementations.
> >
> > Add kernel-doc comments for the generic set_memory() and set_direct_map()
> > stubs and link them into Documentation/core-api/mm-api.rst.
> >
> > Assisted-by: copilot:claude-opus
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> 
> Thanks for doing this Mike, I wish there had been such a document when I
> started using those APIs!
> 
> Much of that document feels like an excruciating FIXME list... which is

It is :)

> exactly the state of set_memory/set_direct_map, and better to have it
> documented than letting every new user stumble upon the same gotchas.
> 
> Overall looks good to me, some minor comments below.
> 
> > [...]
> >
> > +Modifying the kernel page tables
> > +================================
> > +
> > +Except for the vmalloc area, the kernel page tables are mostly static. Still,
> > +there are cases when the permissions of existing kernel mappings have to be
> > +updated, for instance when a module is loaded and its text becomes read-only
> > +and executable, or when a page is temporarily removed from the direct map to
> > +reduce its exposure.
> > +
> > +There are two families of functions for this, both declared in
> > +`include/linux/set_memory.h`:
> > +
> > +* `set_memory_*()` change permissions of an arbitrary kernel mapping. They
> > +  take a kernel virtual address and the number of pages.
> > +
> > +* `set_direct_map_*()` change permissions of the direct map alias of a
> > +  `struct page`. They take a `struct page` pointer and the number of
> > +  pages.
> 
> "direct map alias of a struct page" is rather confusing, are we talking
> about the mapping of struct page itself?

How about "... permissions of the direct mapping of the page frame
represented by a struct page"?
 
> > +
> > +Architectures that implement `set_memory()` select `CONFIG_ARCH_HAS_SET_MEMORY`
> > +
> > +Architectures that implement `set_direct_map()` select
> > +`CONFIG_ARCH_HAS_SET_DIRECT_MAP`.
> > [...]
> >
> > +Architecture specific differences
> > +=================================
> > +
> > +The APIs are implemented by seven architectures and, beyond the common
> > +semantics described above, their behaviour differs in several respects.
> > +
> > +Which of the APIs are implemented:
> > +
> > +=========  =====================  =========================
> > +Arch       `ARCH_HAS_SET_MEMORY`  `ARCH_HAS_SET_DIRECT_MAP`
> > +=========  =====================  =========================
> > +arm        yes                    no
> > +arm64      yes                    yes
> > +loongarch  yes                    yes
> > +powerpc    yes                    no
> > +riscv      yes (MMU only)         yes (MMU only)
> > +s390       yes                    yes
> > +x86        yes                    yes
> > +=========  =====================  =========================
> > +
> > +Only set_memory_ro(), set_memory_rw(), set_memory_x() and set_memory_nx() are
> > +available everywhere, and even these are not universal: some architectures do
> > +not implement any of them for the direct map, and the architectures that may
> 
> "do not implement any of them for the direct map" feels ambiguous - I
> think what we really mean is that they reject direct map addresses, e.g.
> what arm64 does (only accept addresses to kernel VMAs)?

... some architectures restrict ranges that can be modified. For
instance arm64 rejects direct map addresses.
 
> > +run on hardware without an execute permission bit, like x86 and s390, silently
> > +skip the update of the executable bit there.
> > +
> > +set_memory_rox() has a generic implementation that calls set_memory_ro() and
> > +set_memory_x() in turn; PowerPC, s390 and x86 override it with a single-pass
> > +version.
> > +
> > +Making a mapping present or not present is spelled differently: set_memory_p()
> > +and set_memory_np() on x86 and PowerPC, set_memory_valid() on arm64 and arm.
> 
> I'm not sure we should even document set_memory_valid(). It doesn't at
> all behave like the other set_memory_* on amr64 (no check whatsoever, no
> handling of aliases) and is (fortunately) only used from arch code. I've
> been meaning to make its name scarier (__set_memory_valid?) for that reason.

Here it's an example of arch-private APIs, so I'd rather keep it. I'll
extend the intro sentence to make it more clear.
 
> > +
> > +The direct map and the kernel image are normally mapped with the largest
> > +possible pages, and changing the permissions of a single page inside such a
> > +mapping requires splitting it, which not every architecture can do.
> > +
> > +arm
> > +---
> > +
> > +* Does not implement `set_direct_map()`.
> > +* Provides set_memory_valid().
> > +* set_memory_ro(), set_memory_rw(), set_memory_x() and set_memory_nx() accept
> > +  only vmalloc and module addresses.
> > +* set_memory_valid() accepts any address.
> > +* Does not update mapping aliases.
> > +
> > +arm64
> > +-----
> > +
> > +* Provides set_memory_valid().
> > +* Provides the memory encryption helpers, which are effective only when the
> > +  kernel runs as a confidential guest.
> > +* set_memory_ro(), set_memory_rw(), set_memory_x() and set_memory_nx() accept
> > +  only vmalloc and module addresses:
> > +
> > +  - the range must fit in the VM area that contains its start
> > +  - the VM area must have `VM_ALLOC` set and `VM_ALLOW_HUGE_VMAP` clear
> > +
> > +* set_memory_valid() accepts any address.
> > +* The encryption helpers accept only the direct map addresses.
> 
> s/the//
> 
> > +* Propagates the read-only and the read-write changes to the direct map alias
> > +  when `rodata=full` is in effect.
> 
> That is, set_memory_<perms> propagate those changes.

Ack.

> 
> > +* Splits leaf mappings before the update on the hardware that supports it.
> > +  Without such support an update that covers a leaf entry only partially fails
> > +  with a WARN()ing and `-EINVAL`.
> 
> What does "partially fails" mean?  The splitting may be partial, but no
> permission change should occur.

Yes, the split may be partial and. I'll make it clearer.
 
> > +* The `set_direct_map()` functions return 0 without doing anything when the
> > +  direct map cannot be modified, see can_set_direct_map().
> > +* Skips the TLB flush in `set_memory()` when the update only turns an invalid
> > +  mapping into a valid one.
> > +* Does not flush TLB in `set_direct_map()`.
> > +
> > +LoongArch
> > +---------
> > +
> > +* Accepts only the addresses above the hardware window and silently returns
> > +  success for the rest, see `Direct map`_.
> > +* Does not update mapping aliases.
> > +* Does not split anything: a leaf entry is updated as a whole, which changes
> > +  the permissions of the entire large mapping.
> 
> Ouch!

I liked more their set_direct_map part :)
 
> > +* Flushes the TLB in `set_direct_map()`.
> > +
> > +x86
> > +---
> > +
> > +* Provides the largest set of operations on top of the common ones:
> > +
> > +  - the cache attribute helpers: set_memory_uc(), set_memory_wc(),
> > +    set_memory_wb()
> > +  - presence control: set_memory_np() and set_memory_p()
> > +  - set_memory_4k()
> > +  - set_memory_global() and set_memory_nonglobal()
> > +  - the array variants that operate on `struct page` arrays or arrays of
> > +    virtual addresses
> > +  - memory encryption: set_memory_encrypted() and set_memory_decrypted()
> > +
> > +* The `set_memory()` functions accept any mapped kernel address, including the
> > +  direct map, and silently succeed for the unmapped holes inside it.
> > +* Does nothing in set_memory_x() and set_memory_nx() when the CPU has no
> > +  execute permission bit.
> > +* Applies the change to the direct map alias and, for the kernel image, to the
> > +  high kernel mapping. The NX bit is never propagated, so that the direct map
> > +  stays non-executable.
> 
> Same as above, bettermake the subject explicit (set_memory()?).

Ack.
 
> - Kevin

-- 
Sincerely yours,
Mike.

      reply	other threads:[~2026-09-18  7:21 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-09  9:45 Mike Rapoport (Microsoft)
2026-09-09 10:30 ` David Hildenbrand (Arm)
2026-09-09 10:44   ` Mike Rapoport
2026-09-09 13:49     ` David Hildenbrand (Arm)
2026-09-17 18:44 ` Kevin Brodsky
2026-09-18  7:21   ` 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=aqzmcdr7ISpa9r8k@kernel.org \
    --to=rppt@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=corbet@lwn.net \
    --cc=david@kernel.org \
    --cc=kevin.brodsky@arm.com \
    --cc=liam@infradead.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@suse.com \
    --cc=rdunlap@infradead.org \
    --cc=skhan@linuxfoundation.org \
    --cc=surenb@google.com \
    --cc=vbabka@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

all inboxes | Powered by JetHome®