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

[Kevin Brodsky <kevin.brodsky@arm.com>]

On 10/11/2026 17:02, sashiko-bot@kernel.org wrote:
>> [...]
>>
>> +#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.

Will use CONFIG_ARCH_HAS_KPKEYS instead. The idea was that this
implementation is POE-specific, but in fact another implementation would
likely be runtime-selected so using the generic #ifdef makes more sense.

- Kevin