Re: [PATCH] arm64: mm: Move KPTI helpers to mmu.c

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

On 10/09/2025 16:33, Ryan Roberts wrote:
> On 10/09/2025 11:44, Kevin Brodsky wrote:
>> create_kpti_ng_temp_pgd() is currently defined (as an alias) in
>> mmu.c without matching declaration in a header; instead cpufeature.c
>> makes its own declaration. This is clearly not pretty, and as commit
>> ceca927c86e6 ("arm64: mm: Fix CFI failure due to kpti_ng_pgd_alloc
>> function signature") showed, it also makes it very easy for the
>> prototypes to go out of sync.
>>
>> All this would be much simpler if kpti_install_ng_mappings() and
>> associated functions lived in mmu.c, where they logically belong.
>> This is what this patch does:
>> - Move kpti_install_ng_mappings() and associated functions from
>>   cpufeature.c to mmu.c, add a declaration to <asm/mmu.h>
>> - Make create_kpti_ng_temp_pgd() a static function that simply calls
>>   __create_pgd_mapping_locked() instead of aliasing it
>> - Mark all these functions __init
>> - Move __initdata after kpti_ng_temp_alloc (as suggested by
>>   checkpatch)
> This is a great clean up IMHO; that alias has caught me out a few times in the
> past when hacking in this area. And this code clearly belongs in mmu.c.

Good to hear!

>> Signed-off-by: Kevin Brodsky <kevin.brodsky@arm.com>
>> ---
>> Note: as things stand, create_kpti_ng_temp_pgd() could be removed,
>> but a separate patch [1] will make use of it to add an
>> assertion.
> I'd vote for removing it and just calling __create_pgd_mapping_locked() direct.
> The next version of the other patch can just rebase on top of yours and add the
> assert in __kpti_install_ng_mappings().

Oh yes good point, not sure why I didn't consider that! It clearly makes
more sense, I'll send a v2.

> Either way:
>
> Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
>
>> [1] https://lore.kernel.org/all/20250813145607.1612234-3-chaitanyas.prakash@arm.com/
>>
>> [...]
>>
>> +static pgprot_t __init kernel_exec_prot(void)
> nit: this change (to add __init) is unrelated; does it deserve it's own patch?

I thought I might as well sneak it in since it's still related to KPTI
and in the same block... But happy to split it off if that feels more
appropriate.

- Kevin