Re: [PATCH RFC v10 04/22] arm64: Implement asm/kpkeys.h using POE

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] arm64: kpkeys: mismatched CONFIG_ARM64_POE guard in
  asm/kpkeys.h
--

commit 6f45a834f4b090307d508db166b798a5f825afea
Author: Kevin Brodsky <kevin.brodsky@arm.com>

arm64: Implement asm/kpkeys.h using POE

This commit implements the arm64 kpkeys headers and context-switching
helpers using the Permission Overlay Extension (POE) and POR_EL1.

> diff --git a/arch/arm64/include/asm/kpkeys.h b/arch/arm64/include/asm/kpkeys.h
> new file mode 100644
> index 0000000000000..4f9bdb369b47e
> --- /dev/null
> +++ b/arch/arm64/include/asm/kpkeys.h
> @@ -0,0 +1,73 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __ASM_KPKEYS_H
> +#define __ASM_KPKEYS_H
> +
> +#include <linux/kpkeys_types.h>
> +
> +#include <asm/barrier.h>
> +#include <asm/cpufeature.h>
> +#include <asm/por.h>
> +
> +/*
> + * Equivalent to por_set_kpkeys_context(0, KPKEYS_CTX_DEFAULT), but can also be
> + * used in assembly.
> + */
> +#define POR_EL1_INIT	POR_ELx_PERM_PREP(0, POE_RWX)
> +
> +#ifndef __ASSEMBLY__
> +
> +static inline bool arch_supports_kpkeys(void)
> +{
> +	return system_supports_poe();
> +}
> +
> +#ifdef CONFIG_ARM64_POE
> +
> +static inline u64 por_set_kpkeys_context(u64 por, enum kpkeys_ctx ctx)
> +{
> +	por = por_elx_set_pkey_perms(por, 0, POE_RWX);
> +
> +	return por;
> +}
> +
> +static __always_inline void __kpkeys_set_pkey_reg_nosync(u64 pkey_reg)
> +{
> +	/*
> +	 * Writing to POR_EL1 potentially affects all memory accesses, so
> +	 * use barrier() to ensure load/store instructions cannot be reordered
> +	 * past the write.
> +	 */
> +	barrier();
> +	write_sysreg_s(pkey_reg, SYS_POR_EL1);
> +	barrier();
> +}
> +
> +static __always_inline
> +bool arch_kpkeys_enter_context(enum kpkeys_ctx ctx,
> +			       struct arch_kpkeys_state *state)
> +{
> +	const u64 prev_por = read_sysreg_s(SYS_POR_EL1);
> +	const u64 new_por = por_set_kpkeys_context(prev_por, ctx);
> +
> +	if (prev_por == new_por)
> +		return false;
> +
> +	state->por = prev_por;

[Severity: Medium]
Should this block be guarded by CONFIG_ARCH_HAS_KPKEYS instead of
CONFIG_ARM64_POE?

include/linux/kpkeys_types.h only includes <asm/kpkeys_types.h> (for
struct arch_kpkeys_state) and defines enum kpkeys_ctx when
CONFIG_ARCH_HAS_KPKEYS is enabled:

include/linux/kpkeys_types.h:
    #ifdef CONFIG_ARCH_HAS_KPKEYS
    #include <asm/kpkeys_types.h>
    #endif

    #if defined(CONFIG_ARCH_HAS_KPKEYS) && !defined(__ASSEMBLY__)

    enum kpkeys_ctx {
    	KPKEYS_CTX_DEFAULT = 0,
    	KPKEYS_CTX_COUNT,
    };

Because CONFIG_ARM64_POE is enabled by default on arm64 and does not
select ARCH_HAS_KPKEYS until the later commit "arm64: Enable kpkeys",
when commit "arm64: Context-switch POR_EL1" includes <asm/kpkeys.h> in
arch/arm64/kernel/process.c, the build fails with incomplete type errors
for enum kpkeys_ctx and struct arch_kpkeys_state, breaking bisection.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-kpkeys-v10-0-aa6c4d9603bc@arm.com?part=4