[PATCH RFC v8 01/24] mm: Introduce kpkeys

[Linus Walleij]

Hi Kevin,

here is just a few drive-by review comments pertaining to the
API which will be used for all architectures wanting to use
pkeys.

On Tue, May 26, 2026 at 1:17 PM Kevin Brodsky <kevin.brodsky at arm.com> wrote:

> kpkeys is a simple framework to enable the use of protection keys
> (pkeys) to harden the kernel itself. This patch introduces the basic
> API in <linux/kpkeys.h>: a couple of functions to set and restore
> the pkey register and macros to define guard objects.

Itäs a nice idea to use guards for this!

> kpkeys introduces a new concept on top of pkeys: the kpkeys context.
> Each context is associated to a set of permissions for the pkeys
> managed by the kpkeys framework. kpkeys_set_context(ctx) sets those
> permissions according to ctx, and returns the original pkey
> register, to be later restored by kpkeys_restore_pkey_reg(). To
> start with, only KPKEYS_CTX_DEFAULT is available, which is meant to
> grant RW access to KPKEYS_PKEY_DEFAULT (i.e. all memory since this
> is the only available pkey for now).

OK!

> Because each architecture implementing pkeys uses a different
> representation for the pkey register, and may reserve certain pkeys
> for specific uses, support for kpkeys must be explicitly indicated
> by selecting ARCH_HAS_KPKEYS and defining the following functions in
> <asm/kpkeys.h>, in addition to the macros provided in
> <asm-generic/kpkeys.h>:
>
> - arch_kpkeys_set_context()
> - arch_kpkeys_restore_pkey_reg()
> - arch_supports_kpkeys()
>
> Signed-off-by: Kevin Brodsky <kevin.brodsky at arm.com>

I'm following so far.

Is there no need for something like

ctx = arch_kpkeys_request_context(KEY_TYPE) ?

I'm just thinking of the scenario that e.g. two architectures want
to request the same type of key for some generic resource, but
the underlying implementation is vastly different for the two
architectures, so relying on hard-coded arch-specific constants
may not work. I'm also thinking about archs supporting different
versions/types of kpkeys in a multi-platform executable kernel
that may execute on several different silicon with the same ISA.

Maybe this is too much upfront design, I understand it needs to
be kept simple.

> +#ifndef KPKEYS_PKEY_DEFAULT
> +#define KPKEYS_PKEY_DEFAULT    0
> +#endif
(...)
> +#define KPKEYS_CTX_DEFAULT     0
> +
> +#define KPKEYS_CTX_MIN         KPKEYS_CTX_DEFAULT
> +#define KPKEYS_CTX_MAX         KPKEYS_CTX_DEFAULT

I was thinking an enum:

enum kpkey_type {
    KPKEY_DEFAULT = 0,
    KPKEY_MMU_TABLE = 1,
   ....
};

since this is what is used later.

> +/**
> + * kpkeys_set_context() - switch kpkeys context
> + * @ctx: the context to switch to
> + *
> + * Switches to specified kpkeys context. @ctx must be a compile-time
> + * constant. The arch-specific pkey register will be updated accordingly, and
> + * the original value returned.
> + *
> + * Return: the original pkey register value if the register was written to, or
> + *         KPKEYS_PKEY_REG_INVAL otherwise (no write to the register was
> + *         required).
> + */
> +static __always_inline u64 kpkeys_set_context(int ctx)

Should ctx be unsigned here? I'm nor sure what a negativ context
would mean.
kpkeys_set_context(unsigned int ctx)

And then I was thinking:

unsigned int kpkeys_request_context(enum kpkey_type type) {}...

+ arch_ -prefixed version of the same.

unsigned int ctx;
ctx = kpkeys_request_context(KPKEY_MMU_TABLE);
(...)

Constant sinking in the compiler will optimize the arch
function to a constant in the object code so it will still
be fast just a bit more talkative in source form.

Yours,
Linus Walleij