Re: [PATCH v6 01/30] mm: Introduce kpkeys

["David Hildenbrand (Arm)" <david@kernel.org>]

On 4/20/26 08:46, Kevin Brodsky wrote:
> On 17/04/2026 19:38, David Hildenbrand (Arm) wrote:
>> On 4/17/26 17:59, Kevin Brodsky wrote:
>>> The first two are not meant to be directly called, they're the
>>> arch-specific implementation of kpkeys_set_level() and
>>> kpkeys_restore_pkey_reg(), and those generic functions handle some
>>> generic logic.
>>>
>>> arch_kpkeys_enabled() is directly used in generic code, so I suppose it
>>> could be renamed to kpkeys_enabled()? It's actually implemented in an
>>> arch header so I wasn't too sure about it.
>> I was skimming over patch #13 and spotted:
>>
>> +void·__init·kpkeys_hardened_pgtables_init(void)
>> +{
>> +›	if·(!arch_kpkeys_enabled())
>> +›	›	return;
>> +
>> +›	static_branch_enable(&kpkeys_hardened_pgtables_key);
>> +}
>>
>> The arch_* there can just go IMHO.
>>
>> I'd also do it for the two ones used by the GUARD macros. If we don't
>> expect common code wrappers (arch_kpkeys_enabled() vs. kpkeys_enabled),
>> then the arch_ is unnecessary information -- IMHO
> 
> Makes sense. I could just rename arch_kpkeys_enabled() to
> kpkeys_enabled(), but I'm thinking having an arch abstraction could be
> clearer, after looking into protecting sparse-vmemmap page tables. The
> new version would look like this:
> 
> * <asm/kpkeys.h>:
>     - arch_supports_kpkeys()
>     - arch_supports_kpkeys_early() [can be called before features have
> been detected]
> 
> * <linux/kpkeys.h> defines:
>     - kpkeys_enabled() -> arch_supports_kpkeys()
>     - kpkeys_hardened_pgtables_enabled() -> static key
>     - kpkeys_hardened_pgtables_early_enabled() ->
> arch_supports_kpkeys_early() [called when setting up sparse-vmemmap,
> linear map, etc.]
> 
> There is extra #ifdef'ing going on in <linux/kpkeys.h>, but
> <asm/kpkeys.h> doesn't need to worry about it. I think this might be
> easier to follow, I don't like too much having an interface function
> like kpkeys_enabled() defined in an arch header (not great for
> kernel-doc comments either). Any thoughts?

No strong opinion on the indirection as long as we don't call arch_
stuff from ordinary pkey user code :)

-- 
Cheers,

David