arch/arm64/include/asm/mmu.h | 6 ++ arch/arm64/kernel/cpufeature.c | 97 ------------------------------ arch/arm64/mm/mmu.c | 106 ++++++++++++++++++++++++++++++--- 3 files changed, 103 insertions(+), 106 deletions(-)
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)
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.
[1] https://lore.kernel.org/all/20250813145607.1612234-3-chaitanyas.prakash@arm.com/
---
Cc: Anshuman Khandual <anshuman.khandual@arm.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Kees Cook <kees@kernel.org>,
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Yeoreum Yun <yeoreum.yun@arm.com>
---
arch/arm64/include/asm/mmu.h | 6 ++
arch/arm64/kernel/cpufeature.c | 97 ------------------------------
arch/arm64/mm/mmu.c | 106 ++++++++++++++++++++++++++++++---
3 files changed, 103 insertions(+), 106 deletions(-)
diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index 49f1a810df16..624edd6c4964 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -104,5 +104,11 @@ static inline bool kaslr_requires_kpti(void)
return true;
}
+#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
+void kpti_install_ng_mappings(void);
+#else
+static inline void kpti_install_ng_mappings(void) {}
+#endif
+
#endif /* !__ASSEMBLY__ */
#endif
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index ef269a5a37e1..b99eaad48c14 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1940,103 +1940,6 @@ static bool has_pmuv3(const struct arm64_cpu_capabilities *entry, int scope)
}
#endif
-#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
-#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT))
-
-extern
-void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
- phys_addr_t size, pgprot_t prot,
- phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags);
-
-static phys_addr_t __initdata kpti_ng_temp_alloc;
-
-static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
-{
- kpti_ng_temp_alloc -= PAGE_SIZE;
- return kpti_ng_temp_alloc;
-}
-
-static int __init __kpti_install_ng_mappings(void *__unused)
-{
- typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
- extern kpti_remap_fn idmap_kpti_install_ng_mappings;
- kpti_remap_fn *remap_fn;
-
- int cpu = smp_processor_id();
- int levels = CONFIG_PGTABLE_LEVELS;
- int order = order_base_2(levels);
- u64 kpti_ng_temp_pgd_pa = 0;
- pgd_t *kpti_ng_temp_pgd;
- u64 alloc = 0;
-
- if (levels == 5 && !pgtable_l5_enabled())
- levels = 4;
- else if (levels == 4 && !pgtable_l4_enabled())
- levels = 3;
-
- remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
-
- if (!cpu) {
- alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order);
- kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE);
- kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd);
-
- //
- // Create a minimal page table hierarchy that permits us to map
- // the swapper page tables temporarily as we traverse them.
- //
- // The physical pages are laid out as follows:
- //
- // +--------+-/-------+-/------ +-/------ +-\\\--------+
- // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] :
- // +--------+-\-------+-\------ +-\------ +-///--------+
- // ^
- // The first page is mapped into this hierarchy at a PMD_SHIFT
- // aligned virtual address, so that we can manipulate the PTE
- // level entries while the mapping is active. The first entry
- // covers the PTE[] page itself, the remaining entries are free
- // to be used as a ad-hoc fixmap.
- //
- create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc),
- KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL,
- kpti_ng_pgd_alloc, 0);
- }
-
- cpu_install_idmap();
- remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA);
- cpu_uninstall_idmap();
-
- if (!cpu) {
- free_pages(alloc, order);
- arm64_use_ng_mappings = true;
- }
-
- return 0;
-}
-
-static void __init kpti_install_ng_mappings(void)
-{
- /* Check whether KPTI is going to be used */
- if (!arm64_kernel_unmapped_at_el0())
- return;
-
- /*
- * We don't need to rewrite the page-tables if either we've done
- * it already or we have KASLR enabled and therefore have not
- * created any global mappings at all.
- */
- if (arm64_use_ng_mappings)
- return;
-
- stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask);
-}
-
-#else
-static inline void kpti_install_ng_mappings(void)
-{
-}
-#endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
-
static void cpu_enable_kpti(struct arm64_cpu_capabilities const *cap)
{
if (__this_cpu_read(this_cpu_vector) == vectors) {
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 183801520740..eff3295393ee 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -27,6 +27,7 @@
#include <linux/kfence.h>
#include <linux/pkeys.h>
#include <linux/mm_inline.h>
+#include <linux/stop_machine.h>
#include <asm/barrier.h>
#include <asm/cputype.h>
@@ -466,14 +467,6 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
mutex_unlock(&fixmap_lock);
}
-#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
-extern __alias(__create_pgd_mapping_locked)
-void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
- phys_addr_t size, pgprot_t prot,
- phys_addr_t (*pgtable_alloc)(enum pgtable_type),
- int flags);
-#endif
-
static phys_addr_t __pgd_pgtable_alloc(struct mm_struct *mm,
enum pgtable_type pgtable_type)
{
@@ -735,7 +728,102 @@ static void __init declare_vma(struct vm_struct *vma,
}
#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
-static pgprot_t kernel_exec_prot(void)
+#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT))
+
+static phys_addr_t kpti_ng_temp_alloc __initdata;
+
+static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
+{
+ kpti_ng_temp_alloc -= PAGE_SIZE;
+ return kpti_ng_temp_alloc;
+}
+
+static void __init create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys,
+ unsigned long virt, phys_addr_t size,
+ pgprot_t prot,
+ phys_addr_t (*pgtable_alloc)(enum pgtable_type),
+ int flags)
+{
+ __create_pgd_mapping_locked(pgdir, phys, virt, size, prot,
+ pgtable_alloc, flags);
+}
+
+static int __init __kpti_install_ng_mappings(void *__unused)
+{
+ typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
+ extern kpti_remap_fn idmap_kpti_install_ng_mappings;
+ kpti_remap_fn *remap_fn;
+
+ int cpu = smp_processor_id();
+ int levels = CONFIG_PGTABLE_LEVELS;
+ int order = order_base_2(levels);
+ u64 kpti_ng_temp_pgd_pa = 0;
+ pgd_t *kpti_ng_temp_pgd;
+ u64 alloc = 0;
+
+ if (levels == 5 && !pgtable_l5_enabled())
+ levels = 4;
+ else if (levels == 4 && !pgtable_l4_enabled())
+ levels = 3;
+
+ remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
+
+ if (!cpu) {
+ alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order);
+ kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE);
+ kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd);
+
+ //
+ // Create a minimal page table hierarchy that permits us to map
+ // the swapper page tables temporarily as we traverse them.
+ //
+ // The physical pages are laid out as follows:
+ //
+ // +--------+-/-------+-/------ +-/------ +-\\\--------+
+ // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] :
+ // +--------+-\-------+-\------ +-\------ +-///--------+
+ // ^
+ // The first page is mapped into this hierarchy at a PMD_SHIFT
+ // aligned virtual address, so that we can manipulate the PTE
+ // level entries while the mapping is active. The first entry
+ // covers the PTE[] page itself, the remaining entries are free
+ // to be used as a ad-hoc fixmap.
+ //
+ create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc),
+ KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL,
+ kpti_ng_pgd_alloc, 0);
+ }
+
+ cpu_install_idmap();
+ remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA);
+ cpu_uninstall_idmap();
+
+ if (!cpu) {
+ free_pages(alloc, order);
+ arm64_use_ng_mappings = true;
+ }
+
+ return 0;
+}
+
+void __init kpti_install_ng_mappings(void)
+{
+ /* Check whether KPTI is going to be used */
+ if (!arm64_kernel_unmapped_at_el0())
+ return;
+
+ /*
+ * We don't need to rewrite the page-tables if either we've done
+ * it already or we have KASLR enabled and therefore have not
+ * created any global mappings at all.
+ */
+ if (arm64_use_ng_mappings)
+ return;
+
+ stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask);
+}
+
+static pgprot_t __init kernel_exec_prot(void)
{
return rodata_enabled ? PAGE_KERNEL_ROX : PAGE_KERNEL_EXEC;
}
base-commit: 76eeb9b8de9880ca38696b2fb56ac45ac0a25c6c
--
2.47.0
On 10/09/25 4:14 PM, 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)
>
> 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.
>
> [1] https://lore.kernel.org/all/20250813145607.1612234-3-chaitanyas.prakash@arm.com/
> ---
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Kees Cook <kees@kernel.org>,
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> arch/arm64/include/asm/mmu.h | 6 ++
> arch/arm64/kernel/cpufeature.c | 97 ------------------------------
> arch/arm64/mm/mmu.c | 106 ++++++++++++++++++++++++++++++---
> 3 files changed, 103 insertions(+), 106 deletions(-)
>
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 49f1a810df16..624edd6c4964 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -104,5 +104,11 @@ static inline bool kaslr_requires_kpti(void)
> return true;
> }
>
> +#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> +void kpti_install_ng_mappings(void);
> +#else
> +static inline void kpti_install_ng_mappings(void) {}
> +#endif
> +
> #endif /* !__ASSEMBLY__ */
> #endif
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index ef269a5a37e1..b99eaad48c14 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1940,103 +1940,6 @@ static bool has_pmuv3(const struct arm64_cpu_capabilities *entry, int scope)
> }
> #endif
>
> -#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> -#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT))
> -
> -extern
> -void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
> - phys_addr_t size, pgprot_t prot,
> - phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags);
> -
> -static phys_addr_t __initdata kpti_ng_temp_alloc;
> -
> -static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
> -{
> - kpti_ng_temp_alloc -= PAGE_SIZE;
> - return kpti_ng_temp_alloc;
> -}
> -
> -static int __init __kpti_install_ng_mappings(void *__unused)
> -{
> - typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
> - extern kpti_remap_fn idmap_kpti_install_ng_mappings;
> - kpti_remap_fn *remap_fn;
> -
> - int cpu = smp_processor_id();
> - int levels = CONFIG_PGTABLE_LEVELS;
> - int order = order_base_2(levels);
> - u64 kpti_ng_temp_pgd_pa = 0;
> - pgd_t *kpti_ng_temp_pgd;
> - u64 alloc = 0;
> -
> - if (levels == 5 && !pgtable_l5_enabled())
> - levels = 4;
> - else if (levels == 4 && !pgtable_l4_enabled())
> - levels = 3;
> -
> - remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
> -
> - if (!cpu) {
> - alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order);
> - kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE);
> - kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd);
> -
> - //
> - // Create a minimal page table hierarchy that permits us to map
> - // the swapper page tables temporarily as we traverse them.
> - //
> - // The physical pages are laid out as follows:
> - //
> - // +--------+-/-------+-/------ +-/------ +-\\\--------+
> - // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] :
> - // +--------+-\-------+-\------ +-\------ +-///--------+
> - // ^
> - // The first page is mapped into this hierarchy at a PMD_SHIFT
> - // aligned virtual address, so that we can manipulate the PTE
> - // level entries while the mapping is active. The first entry
> - // covers the PTE[] page itself, the remaining entries are free
> - // to be used as a ad-hoc fixmap.
> - //
> - create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc),
> - KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL,
> - kpti_ng_pgd_alloc, 0);
> - }
> -
> - cpu_install_idmap();
> - remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA);
> - cpu_uninstall_idmap();
> -
> - if (!cpu) {
> - free_pages(alloc, order);
> - arm64_use_ng_mappings = true;
> - }
> -
> - return 0;
> -}
> -
> -static void __init kpti_install_ng_mappings(void)
> -{
> - /* Check whether KPTI is going to be used */
> - if (!arm64_kernel_unmapped_at_el0())
> - return;
> -
> - /*
> - * We don't need to rewrite the page-tables if either we've done
> - * it already or we have KASLR enabled and therefore have not
> - * created any global mappings at all.
> - */
> - if (arm64_use_ng_mappings)
> - return;
> -
> - stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask);
> -}
> -
> -#else
> -static inline void kpti_install_ng_mappings(void)
> -{
> -}
> -#endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> -
> static void cpu_enable_kpti(struct arm64_cpu_capabilities const *cap)
> {
> if (__this_cpu_read(this_cpu_vector) == vectors) {
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 183801520740..eff3295393ee 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -27,6 +27,7 @@
> #include <linux/kfence.h>
> #include <linux/pkeys.h>
> #include <linux/mm_inline.h>
> +#include <linux/stop_machine.h>
>
> #include <asm/barrier.h>
> #include <asm/cputype.h>
> @@ -466,14 +467,6 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
> mutex_unlock(&fixmap_lock);
> }
>
> -#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> -extern __alias(__create_pgd_mapping_locked)
> -void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
> - phys_addr_t size, pgprot_t prot,
> - phys_addr_t (*pgtable_alloc)(enum pgtable_type),
> - int flags);
> -#endif
> -
> static phys_addr_t __pgd_pgtable_alloc(struct mm_struct *mm,
> enum pgtable_type pgtable_type)
> {
> @@ -735,7 +728,102 @@ static void __init declare_vma(struct vm_struct *vma,
> }
>
> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> -static pgprot_t kernel_exec_prot(void)
> +#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT))
> +
> +static phys_addr_t kpti_ng_temp_alloc __initdata;
> +
> +static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
> +{
> + kpti_ng_temp_alloc -= PAGE_SIZE;
> + return kpti_ng_temp_alloc;
> +}
> +
> +static void __init create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys,
> + unsigned long virt, phys_addr_t size,
> + pgprot_t prot,
> + phys_addr_t (*pgtable_alloc)(enum pgtable_type),
> + int flags)
> +{
> + __create_pgd_mapping_locked(pgdir, phys, virt, size, prot,
> + pgtable_alloc, flags);
> +}
> +
> +static int __init __kpti_install_ng_mappings(void *__unused)
> +{
> + typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
> + extern kpti_remap_fn idmap_kpti_install_ng_mappings;
> + kpti_remap_fn *remap_fn;
> +
> + int cpu = smp_processor_id();
> + int levels = CONFIG_PGTABLE_LEVELS;
> + int order = order_base_2(levels);
> + u64 kpti_ng_temp_pgd_pa = 0;
> + pgd_t *kpti_ng_temp_pgd;
> + u64 alloc = 0;
> +
> + if (levels == 5 && !pgtable_l5_enabled())
> + levels = 4;
> + else if (levels == 4 && !pgtable_l4_enabled())
> + levels = 3;
> +
> + remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
> +
> + if (!cpu) {
> + alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order);
> + kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE);
> + kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd);
> +
> + //
> + // Create a minimal page table hierarchy that permits us to map
> + // the swapper page tables temporarily as we traverse them.
> + //
> + // The physical pages are laid out as follows:
> + //
> + // +--------+-/-------+-/------ +-/------ +-\\\--------+
> + // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] :
> + // +--------+-\-------+-\------ +-\------ +-///--------+
> + // ^
> + // The first page is mapped into this hierarchy at a PMD_SHIFT
> + // aligned virtual address, so that we can manipulate the PTE
> + // level entries while the mapping is active. The first entry
> + // covers the PTE[] page itself, the remaining entries are free
> + // to be used as a ad-hoc fixmap.
> + //
> + create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc),
> + KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL,
> + kpti_ng_pgd_alloc, 0);
> + }
> +
> + cpu_install_idmap();
> + remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA);
> + cpu_uninstall_idmap();
> +
> + if (!cpu) {
> + free_pages(alloc, order);
> + arm64_use_ng_mappings = true;
> + }
> +
> + return 0;
> +}
> +
> +void __init kpti_install_ng_mappings(void)
> +{
> + /* Check whether KPTI is going to be used */
> + if (!arm64_kernel_unmapped_at_el0())
> + return;
> +
> + /*
> + * We don't need to rewrite the page-tables if either we've done
> + * it already or we have KASLR enabled and therefore have not
> + * created any global mappings at all.
> + */
> + if (arm64_use_ng_mappings)
> + return;
> +
> + stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask);
> +}
> +
> +static pgprot_t __init kernel_exec_prot(void)
> {
> return rodata_enabled ? PAGE_KERNEL_ROX : PAGE_KERNEL_EXEC;
> }
>
> base-commit: 76eeb9b8de9880ca38696b2fb56ac45ac0a25c6c
On Wed, 10 Sept 2025 at 12:45, Kevin Brodsky <kevin.brodsky@arm.com> 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)
>
> 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.
>
> [1] https://lore.kernel.org/all/20250813145607.1612234-3-chaitanyas.prakash@arm.com/
> ---
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Kees Cook <kees@kernel.org>,
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Yeoreum Yun <yeoreum.yun@arm.com>
> ---
> arch/arm64/include/asm/mmu.h | 6 ++
> arch/arm64/kernel/cpufeature.c | 97 ------------------------------
> arch/arm64/mm/mmu.c | 106 ++++++++++++++++++++++++++++++---
> 3 files changed, 103 insertions(+), 106 deletions(-)
>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 49f1a810df16..624edd6c4964 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -104,5 +104,11 @@ static inline bool kaslr_requires_kpti(void)
> return true;
> }
>
> +#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> +void kpti_install_ng_mappings(void);
> +#else
> +static inline void kpti_install_ng_mappings(void) {}
> +#endif
> +
Nit: you might just keep the former declaration, and check for
IS_ENABLED(CONFIG_UNMAP_KERNEL_AT_EL0) in the function, rather than
propagating this distinction to the header file.
But either is fine with me.
> #endif /* !__ASSEMBLY__ */
> #endif
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index ef269a5a37e1..b99eaad48c14 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1940,103 +1940,6 @@ static bool has_pmuv3(const struct arm64_cpu_capabilities *entry, int scope)
> }
> #endif
>
> -#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> -#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT))
> -
> -extern
> -void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
> - phys_addr_t size, pgprot_t prot,
> - phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags);
> -
> -static phys_addr_t __initdata kpti_ng_temp_alloc;
> -
> -static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
> -{
> - kpti_ng_temp_alloc -= PAGE_SIZE;
> - return kpti_ng_temp_alloc;
> -}
> -
> -static int __init __kpti_install_ng_mappings(void *__unused)
> -{
> - typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
> - extern kpti_remap_fn idmap_kpti_install_ng_mappings;
> - kpti_remap_fn *remap_fn;
> -
> - int cpu = smp_processor_id();
> - int levels = CONFIG_PGTABLE_LEVELS;
> - int order = order_base_2(levels);
> - u64 kpti_ng_temp_pgd_pa = 0;
> - pgd_t *kpti_ng_temp_pgd;
> - u64 alloc = 0;
> -
> - if (levels == 5 && !pgtable_l5_enabled())
> - levels = 4;
> - else if (levels == 4 && !pgtable_l4_enabled())
> - levels = 3;
> -
> - remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
> -
> - if (!cpu) {
> - alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order);
> - kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE);
> - kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd);
> -
> - //
> - // Create a minimal page table hierarchy that permits us to map
> - // the swapper page tables temporarily as we traverse them.
> - //
> - // The physical pages are laid out as follows:
> - //
> - // +--------+-/-------+-/------ +-/------ +-\\\--------+
> - // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] :
> - // +--------+-\-------+-\------ +-\------ +-///--------+
> - // ^
> - // The first page is mapped into this hierarchy at a PMD_SHIFT
> - // aligned virtual address, so that we can manipulate the PTE
> - // level entries while the mapping is active. The first entry
> - // covers the PTE[] page itself, the remaining entries are free
> - // to be used as a ad-hoc fixmap.
> - //
> - create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc),
> - KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL,
> - kpti_ng_pgd_alloc, 0);
> - }
> -
> - cpu_install_idmap();
> - remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA);
> - cpu_uninstall_idmap();
> -
> - if (!cpu) {
> - free_pages(alloc, order);
> - arm64_use_ng_mappings = true;
> - }
> -
> - return 0;
> -}
> -
> -static void __init kpti_install_ng_mappings(void)
> -{
> - /* Check whether KPTI is going to be used */
> - if (!arm64_kernel_unmapped_at_el0())
> - return;
> -
> - /*
> - * We don't need to rewrite the page-tables if either we've done
> - * it already or we have KASLR enabled and therefore have not
> - * created any global mappings at all.
> - */
> - if (arm64_use_ng_mappings)
> - return;
> -
> - stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask);
> -}
> -
> -#else
> -static inline void kpti_install_ng_mappings(void)
> -{
> -}
> -#endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> -
> static void cpu_enable_kpti(struct arm64_cpu_capabilities const *cap)
> {
> if (__this_cpu_read(this_cpu_vector) == vectors) {
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 183801520740..eff3295393ee 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -27,6 +27,7 @@
> #include <linux/kfence.h>
> #include <linux/pkeys.h>
> #include <linux/mm_inline.h>
> +#include <linux/stop_machine.h>
>
> #include <asm/barrier.h>
> #include <asm/cputype.h>
> @@ -466,14 +467,6 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
> mutex_unlock(&fixmap_lock);
> }
>
> -#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> -extern __alias(__create_pgd_mapping_locked)
> -void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
> - phys_addr_t size, pgprot_t prot,
> - phys_addr_t (*pgtable_alloc)(enum pgtable_type),
> - int flags);
> -#endif
> -
> static phys_addr_t __pgd_pgtable_alloc(struct mm_struct *mm,
> enum pgtable_type pgtable_type)
> {
> @@ -735,7 +728,102 @@ static void __init declare_vma(struct vm_struct *vma,
> }
>
> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> -static pgprot_t kernel_exec_prot(void)
> +#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT))
> +
> +static phys_addr_t kpti_ng_temp_alloc __initdata;
> +
> +static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
> +{
> + kpti_ng_temp_alloc -= PAGE_SIZE;
> + return kpti_ng_temp_alloc;
> +}
> +
> +static void __init create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys,
> + unsigned long virt, phys_addr_t size,
> + pgprot_t prot,
> + phys_addr_t (*pgtable_alloc)(enum pgtable_type),
> + int flags)
> +{
> + __create_pgd_mapping_locked(pgdir, phys, virt, size, prot,
> + pgtable_alloc, flags);
> +}
> +
> +static int __init __kpti_install_ng_mappings(void *__unused)
> +{
> + typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
> + extern kpti_remap_fn idmap_kpti_install_ng_mappings;
> + kpti_remap_fn *remap_fn;
> +
> + int cpu = smp_processor_id();
> + int levels = CONFIG_PGTABLE_LEVELS;
> + int order = order_base_2(levels);
> + u64 kpti_ng_temp_pgd_pa = 0;
> + pgd_t *kpti_ng_temp_pgd;
> + u64 alloc = 0;
> +
> + if (levels == 5 && !pgtable_l5_enabled())
> + levels = 4;
> + else if (levels == 4 && !pgtable_l4_enabled())
> + levels = 3;
> +
> + remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
> +
> + if (!cpu) {
> + alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order);
> + kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE);
> + kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd);
> +
> + //
> + // Create a minimal page table hierarchy that permits us to map
> + // the swapper page tables temporarily as we traverse them.
> + //
> + // The physical pages are laid out as follows:
> + //
> + // +--------+-/-------+-/------ +-/------ +-\\\--------+
> + // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] :
> + // +--------+-\-------+-\------ +-\------ +-///--------+
> + // ^
> + // The first page is mapped into this hierarchy at a PMD_SHIFT
> + // aligned virtual address, so that we can manipulate the PTE
> + // level entries while the mapping is active. The first entry
> + // covers the PTE[] page itself, the remaining entries are free
> + // to be used as a ad-hoc fixmap.
> + //
> + create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc),
> + KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL,
> + kpti_ng_pgd_alloc, 0);
> + }
> +
> + cpu_install_idmap();
> + remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA);
> + cpu_uninstall_idmap();
> +
> + if (!cpu) {
> + free_pages(alloc, order);
> + arm64_use_ng_mappings = true;
> + }
> +
> + return 0;
> +}
> +
> +void __init kpti_install_ng_mappings(void)
> +{
> + /* Check whether KPTI is going to be used */
> + if (!arm64_kernel_unmapped_at_el0())
> + return;
> +
> + /*
> + * We don't need to rewrite the page-tables if either we've done
> + * it already or we have KASLR enabled and therefore have not
> + * created any global mappings at all.
> + */
> + if (arm64_use_ng_mappings)
> + return;
> +
> + stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask);
> +}
> +
> +static pgprot_t __init kernel_exec_prot(void)
> {
> return rodata_enabled ? PAGE_KERNEL_ROX : PAGE_KERNEL_EXEC;
> }
>
> base-commit: 76eeb9b8de9880ca38696b2fb56ac45ac0a25c6c
> --
> 2.47.0
>
On 11/09/2025 11:19, Ard Biesheuvel wrote:
>> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
>> index 49f1a810df16..624edd6c4964 100644
>> --- a/arch/arm64/include/asm/mmu.h
>> +++ b/arch/arm64/include/asm/mmu.h
>> @@ -104,5 +104,11 @@ static inline bool kaslr_requires_kpti(void)
>> return true;
>> }
>>
>> +#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
>> +void kpti_install_ng_mappings(void);
>> +#else
>> +static inline void kpti_install_ng_mappings(void) {}
>> +#endif
>> +
> Nit: you might just keep the former declaration, and check for
> IS_ENABLED(CONFIG_UNMAP_KERNEL_AT_EL0) in the function, rather than
> propagating this distinction to the header file.
> But either is fine with me.
That's an option, but that would mean removing the #ifdef around the
functions defined in mmu.c. They would probably get eliminated by the
linker if the CONFIG isn't defined, but I'm not so sure about the static
variable (kpti_ng_temp_alloc). Probably not a big deal but I feel
keeping the #ifdef is more self-documenting as well.
- Kevin
On Thu, 11 Sept 2025 at 12:52, Kevin Brodsky <kevin.brodsky@arm.com> wrote:
>
> On 11/09/2025 11:19, Ard Biesheuvel wrote:
> >> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> >> index 49f1a810df16..624edd6c4964 100644
> >> --- a/arch/arm64/include/asm/mmu.h
> >> +++ b/arch/arm64/include/asm/mmu.h
> >> @@ -104,5 +104,11 @@ static inline bool kaslr_requires_kpti(void)
> >> return true;
> >> }
> >>
> >> +#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> >> +void kpti_install_ng_mappings(void);
> >> +#else
> >> +static inline void kpti_install_ng_mappings(void) {}
> >> +#endif
> >> +
> > Nit: you might just keep the former declaration, and check for
> > IS_ENABLED(CONFIG_UNMAP_KERNEL_AT_EL0) in the function, rather than
> > propagating this distinction to the header file.
> > But either is fine with me.
>
> That's an option, but that would mean removing the #ifdef around the
> functions defined in mmu.c. They would probably get eliminated by the
> linker if the CONFIG isn't defined, but I'm not so sure about the static
> variable (kpti_ng_temp_alloc). Probably not a big deal but I feel
> keeping the #ifdef is more self-documenting as well.
>
Fair enough.
On 10/09/25 4:14 PM, 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)
>
> 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.
>
> [1] https://lore.kernel.org/all/20250813145607.1612234-3-chaitanyas.prakash@arm.com/
> ---
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Kees Cook <kees@kernel.org>,
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Yeoreum Yun <yeoreum.yun@arm.com>
> ---
> arch/arm64/include/asm/mmu.h | 6 ++
> arch/arm64/kernel/cpufeature.c | 97 ------------------------------
> arch/arm64/mm/mmu.c | 106 ++++++++++++++++++++++++++++++---
> 3 files changed, 103 insertions(+), 106 deletions(-)
>
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 49f1a810df16..624edd6c4964 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -104,5 +104,11 @@ static inline bool kaslr_requires_kpti(void)
> return true;
> }
>
> +#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> +void kpti_install_ng_mappings(void);
Could the declarations be moved here instead ? Otherwise LGTM.
diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index 624edd6c4964..062465939192 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -106,6 +106,8 @@ static inline bool kaslr_requires_kpti(void)
#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
void kpti_install_ng_mappings(void);
+typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
+kpti_remap_fn idmap_kpti_install_ng_mappings;
#else
static inline void kpti_install_ng_mappings(void) {}
#endif
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index eff3295393ee..1b5c3c590e95 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -750,8 +750,6 @@ static void __init create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys,
static int __init __kpti_install_ng_mappings(void *__unused)
{
- typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
- extern kpti_remap_fn idmap_kpti_install_ng_mappings;
kpti_remap_fn *remap_fn;
int cpu = smp_processor_id();
> +#else
> +static inline void kpti_install_ng_mappings(void) {}
> +#endif
> +
> #endif /* !__ASSEMBLY__ */
> #endif
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index ef269a5a37e1..b99eaad48c14 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1940,103 +1940,6 @@ static bool has_pmuv3(const struct arm64_cpu_capabilities *entry, int scope)
> }
> #endif
>
> -#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> -#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT))
> -
> -extern
> -void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
> - phys_addr_t size, pgprot_t prot,
> - phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags);
> -
> -static phys_addr_t __initdata kpti_ng_temp_alloc;
> -
> -static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
> -{
> - kpti_ng_temp_alloc -= PAGE_SIZE;
> - return kpti_ng_temp_alloc;
> -}
> -
> -static int __init __kpti_install_ng_mappings(void *__unused)
> -{
> - typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
> - extern kpti_remap_fn idmap_kpti_install_ng_mappings;
> - kpti_remap_fn *remap_fn;
> -
> - int cpu = smp_processor_id();
> - int levels = CONFIG_PGTABLE_LEVELS;
> - int order = order_base_2(levels);
> - u64 kpti_ng_temp_pgd_pa = 0;
> - pgd_t *kpti_ng_temp_pgd;
> - u64 alloc = 0;
> -
> - if (levels == 5 && !pgtable_l5_enabled())
> - levels = 4;
> - else if (levels == 4 && !pgtable_l4_enabled())
> - levels = 3;
> -
> - remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
> -
> - if (!cpu) {
> - alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order);
> - kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE);
> - kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd);
> -
> - //
> - // Create a minimal page table hierarchy that permits us to map
> - // the swapper page tables temporarily as we traverse them.
> - //
> - // The physical pages are laid out as follows:
> - //
> - // +--------+-/-------+-/------ +-/------ +-\\\--------+
> - // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] :
> - // +--------+-\-------+-\------ +-\------ +-///--------+
> - // ^
> - // The first page is mapped into this hierarchy at a PMD_SHIFT
> - // aligned virtual address, so that we can manipulate the PTE
> - // level entries while the mapping is active. The first entry
> - // covers the PTE[] page itself, the remaining entries are free
> - // to be used as a ad-hoc fixmap.
> - //
> - create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc),
> - KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL,
> - kpti_ng_pgd_alloc, 0);
> - }
> -
> - cpu_install_idmap();
> - remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA);
> - cpu_uninstall_idmap();
> -
> - if (!cpu) {
> - free_pages(alloc, order);
> - arm64_use_ng_mappings = true;
> - }
> -
> - return 0;
> -}
> -
> -static void __init kpti_install_ng_mappings(void)
> -{
> - /* Check whether KPTI is going to be used */
> - if (!arm64_kernel_unmapped_at_el0())
> - return;
> -
> - /*
> - * We don't need to rewrite the page-tables if either we've done
> - * it already or we have KASLR enabled and therefore have not
> - * created any global mappings at all.
> - */
> - if (arm64_use_ng_mappings)
> - return;
> -
> - stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask);
> -}
> -
> -#else
> -static inline void kpti_install_ng_mappings(void)
> -{
> -}
> -#endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> -
> static void cpu_enable_kpti(struct arm64_cpu_capabilities const *cap)
> {
> if (__this_cpu_read(this_cpu_vector) == vectors) {
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 183801520740..eff3295393ee 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -27,6 +27,7 @@
> #include <linux/kfence.h>
> #include <linux/pkeys.h>
> #include <linux/mm_inline.h>
> +#include <linux/stop_machine.h>
>
> #include <asm/barrier.h>
> #include <asm/cputype.h>
> @@ -466,14 +467,6 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
> mutex_unlock(&fixmap_lock);
> }
>
> -#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> -extern __alias(__create_pgd_mapping_locked)
> -void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
> - phys_addr_t size, pgprot_t prot,
> - phys_addr_t (*pgtable_alloc)(enum pgtable_type),
> - int flags);
> -#endif
> -
> static phys_addr_t __pgd_pgtable_alloc(struct mm_struct *mm,
> enum pgtable_type pgtable_type)
> {
> @@ -735,7 +728,102 @@ static void __init declare_vma(struct vm_struct *vma,
> }
>
> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> -static pgprot_t kernel_exec_prot(void)
> +#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT))
> +
> +static phys_addr_t kpti_ng_temp_alloc __initdata;
> +
> +static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
> +{
> + kpti_ng_temp_alloc -= PAGE_SIZE;
> + return kpti_ng_temp_alloc;
> +}
> +
> +static void __init create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys,
> + unsigned long virt, phys_addr_t size,
> + pgprot_t prot,
> + phys_addr_t (*pgtable_alloc)(enum pgtable_type),
> + int flags)
> +{
> + __create_pgd_mapping_locked(pgdir, phys, virt, size, prot,
> + pgtable_alloc, flags);
> +}
> +
> +static int __init __kpti_install_ng_mappings(void *__unused)
> +{
> + typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
> + extern kpti_remap_fn idmap_kpti_install_ng_mappings;
> + kpti_remap_fn *remap_fn;
> +
> + int cpu = smp_processor_id();
> + int levels = CONFIG_PGTABLE_LEVELS;
> + int order = order_base_2(levels);
> + u64 kpti_ng_temp_pgd_pa = 0;
> + pgd_t *kpti_ng_temp_pgd;
> + u64 alloc = 0;
> +
> + if (levels == 5 && !pgtable_l5_enabled())
> + levels = 4;
> + else if (levels == 4 && !pgtable_l4_enabled())
> + levels = 3;
> +
> + remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
> +
> + if (!cpu) {
> + alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order);
> + kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE);
> + kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd);
> +
> + //
> + // Create a minimal page table hierarchy that permits us to map
> + // the swapper page tables temporarily as we traverse them.
> + //
> + // The physical pages are laid out as follows:
> + //
> + // +--------+-/-------+-/------ +-/------ +-\\\--------+
> + // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] :
> + // +--------+-\-------+-\------ +-\------ +-///--------+
> + // ^
> + // The first page is mapped into this hierarchy at a PMD_SHIFT
> + // aligned virtual address, so that we can manipulate the PTE
> + // level entries while the mapping is active. The first entry
> + // covers the PTE[] page itself, the remaining entries are free
> + // to be used as a ad-hoc fixmap.
> + //
> + create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc),
> + KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL,
> + kpti_ng_pgd_alloc, 0);
> + }
> +
> + cpu_install_idmap();
> + remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA);
> + cpu_uninstall_idmap();
> +
> + if (!cpu) {
> + free_pages(alloc, order);
> + arm64_use_ng_mappings = true;
> + }
> +
> + return 0;
> +}
> +
> +void __init kpti_install_ng_mappings(void)
> +{
> + /* Check whether KPTI is going to be used */
> + if (!arm64_kernel_unmapped_at_el0())
> + return;
> +
> + /*
> + * We don't need to rewrite the page-tables if either we've done
> + * it already or we have KASLR enabled and therefore have not
> + * created any global mappings at all.
> + */
> + if (arm64_use_ng_mappings)
> + return;
> +
> + stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask);
> +}
> +
> +static pgprot_t __init kernel_exec_prot(void)
> {
> return rodata_enabled ? PAGE_KERNEL_ROX : PAGE_KERNEL_EXEC;
> }
>
> base-commit: 76eeb9b8de9880ca38696b2fb56ac45ac0a25c6c
On Thu, 11 Sept 2025 at 07:13, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
>
>
> On 10/09/25 4:14 PM, 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)
> >
> > 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.
> >
> > [1] https://lore.kernel.org/all/20250813145607.1612234-3-chaitanyas.prakash@arm.com/
> > ---
> > Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Kees Cook <kees@kernel.org>,
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Ryan Roberts <ryan.roberts@arm.com>
> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Yeoreum Yun <yeoreum.yun@arm.com>
> > ---
> > arch/arm64/include/asm/mmu.h | 6 ++
> > arch/arm64/kernel/cpufeature.c | 97 ------------------------------
> > arch/arm64/mm/mmu.c | 106 ++++++++++++++++++++++++++++++---
> > 3 files changed, 103 insertions(+), 106 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> > index 49f1a810df16..624edd6c4964 100644
> > --- a/arch/arm64/include/asm/mmu.h
> > +++ b/arch/arm64/include/asm/mmu.h
> > @@ -104,5 +104,11 @@ static inline bool kaslr_requires_kpti(void)
> > return true;
> > }
> >
> > +#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> > +void kpti_install_ng_mappings(void);
>
> Could the declarations be moved here instead ?
Why?
> Otherwise LGTM.
>
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 624edd6c4964..062465939192 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -106,6 +106,8 @@ static inline bool kaslr_requires_kpti(void)
>
> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> void kpti_install_ng_mappings(void);
> +typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
> +kpti_remap_fn idmap_kpti_install_ng_mappings;
> #else
> static inline void kpti_install_ng_mappings(void) {}
> #endif
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index eff3295393ee..1b5c3c590e95 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -750,8 +750,6 @@ static void __init create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys,
>
> static int __init __kpti_install_ng_mappings(void *__unused)
> {
> - typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
> - extern kpti_remap_fn idmap_kpti_install_ng_mappings;
> kpti_remap_fn *remap_fn;
>
> int cpu = smp_processor_id();
>
> > +#else
> > +static inline void kpti_install_ng_mappings(void) {}
> > +#endif
> > +
> > #endif /* !__ASSEMBLY__ */
> > #endif
> > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > index ef269a5a37e1..b99eaad48c14 100644
> > --- a/arch/arm64/kernel/cpufeature.c
> > +++ b/arch/arm64/kernel/cpufeature.c
> > @@ -1940,103 +1940,6 @@ static bool has_pmuv3(const struct arm64_cpu_capabilities *entry, int scope)
> > }
> > #endif
> >
> > -#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> > -#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT))
> > -
> > -extern
> > -void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
> > - phys_addr_t size, pgprot_t prot,
> > - phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags);
> > -
> > -static phys_addr_t __initdata kpti_ng_temp_alloc;
> > -
> > -static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
> > -{
> > - kpti_ng_temp_alloc -= PAGE_SIZE;
> > - return kpti_ng_temp_alloc;
> > -}
> > -
> > -static int __init __kpti_install_ng_mappings(void *__unused)
> > -{
> > - typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
> > - extern kpti_remap_fn idmap_kpti_install_ng_mappings;
> > - kpti_remap_fn *remap_fn;
> > -
> > - int cpu = smp_processor_id();
> > - int levels = CONFIG_PGTABLE_LEVELS;
> > - int order = order_base_2(levels);
> > - u64 kpti_ng_temp_pgd_pa = 0;
> > - pgd_t *kpti_ng_temp_pgd;
> > - u64 alloc = 0;
> > -
> > - if (levels == 5 && !pgtable_l5_enabled())
> > - levels = 4;
> > - else if (levels == 4 && !pgtable_l4_enabled())
> > - levels = 3;
> > -
> > - remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
> > -
> > - if (!cpu) {
> > - alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order);
> > - kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE);
> > - kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd);
> > -
> > - //
> > - // Create a minimal page table hierarchy that permits us to map
> > - // the swapper page tables temporarily as we traverse them.
> > - //
> > - // The physical pages are laid out as follows:
> > - //
> > - // +--------+-/-------+-/------ +-/------ +-\\\--------+
> > - // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] :
> > - // +--------+-\-------+-\------ +-\------ +-///--------+
> > - // ^
> > - // The first page is mapped into this hierarchy at a PMD_SHIFT
> > - // aligned virtual address, so that we can manipulate the PTE
> > - // level entries while the mapping is active. The first entry
> > - // covers the PTE[] page itself, the remaining entries are free
> > - // to be used as a ad-hoc fixmap.
> > - //
> > - create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc),
> > - KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL,
> > - kpti_ng_pgd_alloc, 0);
> > - }
> > -
> > - cpu_install_idmap();
> > - remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA);
> > - cpu_uninstall_idmap();
> > -
> > - if (!cpu) {
> > - free_pages(alloc, order);
> > - arm64_use_ng_mappings = true;
> > - }
> > -
> > - return 0;
> > -}
> > -
> > -static void __init kpti_install_ng_mappings(void)
> > -{
> > - /* Check whether KPTI is going to be used */
> > - if (!arm64_kernel_unmapped_at_el0())
> > - return;
> > -
> > - /*
> > - * We don't need to rewrite the page-tables if either we've done
> > - * it already or we have KASLR enabled and therefore have not
> > - * created any global mappings at all.
> > - */
> > - if (arm64_use_ng_mappings)
> > - return;
> > -
> > - stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask);
> > -}
> > -
> > -#else
> > -static inline void kpti_install_ng_mappings(void)
> > -{
> > -}
> > -#endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> > -
> > static void cpu_enable_kpti(struct arm64_cpu_capabilities const *cap)
> > {
> > if (__this_cpu_read(this_cpu_vector) == vectors) {
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > index 183801520740..eff3295393ee 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -27,6 +27,7 @@
> > #include <linux/kfence.h>
> > #include <linux/pkeys.h>
> > #include <linux/mm_inline.h>
> > +#include <linux/stop_machine.h>
> >
> > #include <asm/barrier.h>
> > #include <asm/cputype.h>
> > @@ -466,14 +467,6 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
> > mutex_unlock(&fixmap_lock);
> > }
> >
> > -#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> > -extern __alias(__create_pgd_mapping_locked)
> > -void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
> > - phys_addr_t size, pgprot_t prot,
> > - phys_addr_t (*pgtable_alloc)(enum pgtable_type),
> > - int flags);
> > -#endif
> > -
> > static phys_addr_t __pgd_pgtable_alloc(struct mm_struct *mm,
> > enum pgtable_type pgtable_type)
> > {
> > @@ -735,7 +728,102 @@ static void __init declare_vma(struct vm_struct *vma,
> > }
> >
> > #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> > -static pgprot_t kernel_exec_prot(void)
> > +#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT))
> > +
> > +static phys_addr_t kpti_ng_temp_alloc __initdata;
> > +
> > +static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
> > +{
> > + kpti_ng_temp_alloc -= PAGE_SIZE;
> > + return kpti_ng_temp_alloc;
> > +}
> > +
> > +static void __init create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys,
> > + unsigned long virt, phys_addr_t size,
> > + pgprot_t prot,
> > + phys_addr_t (*pgtable_alloc)(enum pgtable_type),
> > + int flags)
> > +{
> > + __create_pgd_mapping_locked(pgdir, phys, virt, size, prot,
> > + pgtable_alloc, flags);
> > +}
> > +
> > +static int __init __kpti_install_ng_mappings(void *__unused)
> > +{
> > + typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
> > + extern kpti_remap_fn idmap_kpti_install_ng_mappings;
> > + kpti_remap_fn *remap_fn;
> > +
> > + int cpu = smp_processor_id();
> > + int levels = CONFIG_PGTABLE_LEVELS;
> > + int order = order_base_2(levels);
> > + u64 kpti_ng_temp_pgd_pa = 0;
> > + pgd_t *kpti_ng_temp_pgd;
> > + u64 alloc = 0;
> > +
> > + if (levels == 5 && !pgtable_l5_enabled())
> > + levels = 4;
> > + else if (levels == 4 && !pgtable_l4_enabled())
> > + levels = 3;
> > +
> > + remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
> > +
> > + if (!cpu) {
> > + alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order);
> > + kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE);
> > + kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd);
> > +
> > + //
> > + // Create a minimal page table hierarchy that permits us to map
> > + // the swapper page tables temporarily as we traverse them.
> > + //
> > + // The physical pages are laid out as follows:
> > + //
> > + // +--------+-/-------+-/------ +-/------ +-\\\--------+
> > + // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] :
> > + // +--------+-\-------+-\------ +-\------ +-///--------+
> > + // ^
> > + // The first page is mapped into this hierarchy at a PMD_SHIFT
> > + // aligned virtual address, so that we can manipulate the PTE
> > + // level entries while the mapping is active. The first entry
> > + // covers the PTE[] page itself, the remaining entries are free
> > + // to be used as a ad-hoc fixmap.
> > + //
> > + create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc),
> > + KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL,
> > + kpti_ng_pgd_alloc, 0);
> > + }
> > +
> > + cpu_install_idmap();
> > + remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA);
> > + cpu_uninstall_idmap();
> > +
> > + if (!cpu) {
> > + free_pages(alloc, order);
> > + arm64_use_ng_mappings = true;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +void __init kpti_install_ng_mappings(void)
> > +{
> > + /* Check whether KPTI is going to be used */
> > + if (!arm64_kernel_unmapped_at_el0())
> > + return;
> > +
> > + /*
> > + * We don't need to rewrite the page-tables if either we've done
> > + * it already or we have KASLR enabled and therefore have not
> > + * created any global mappings at all.
> > + */
> > + if (arm64_use_ng_mappings)
> > + return;
> > +
> > + stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask);
> > +}
> > +
> > +static pgprot_t __init kernel_exec_prot(void)
> > {
> > return rodata_enabled ? PAGE_KERNEL_ROX : PAGE_KERNEL_EXEC;
> > }
> >
> > base-commit: 76eeb9b8de9880ca38696b2fb56ac45ac0a25c6c
>
On 11/09/25 11:38 AM, Ard Biesheuvel wrote:
> On Thu, 11 Sept 2025 at 07:13, Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
>>
>>
>> On 10/09/25 4:14 PM, 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)
>>>
>>> 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.
>>>
>>> [1] https://lore.kernel.org/all/20250813145607.1612234-3-chaitanyas.prakash@arm.com/
>>> ---
>>> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
>>> Cc: Ard Biesheuvel <ardb@kernel.org>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Kees Cook <kees@kernel.org>,
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Ryan Roberts <ryan.roberts@arm.com>
>>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Yeoreum Yun <yeoreum.yun@arm.com>
>>> ---
>>> arch/arm64/include/asm/mmu.h | 6 ++
>>> arch/arm64/kernel/cpufeature.c | 97 ------------------------------
>>> arch/arm64/mm/mmu.c | 106 ++++++++++++++++++++++++++++++---
>>> 3 files changed, 103 insertions(+), 106 deletions(-)
>>>
>>> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
>>> index 49f1a810df16..624edd6c4964 100644
>>> --- a/arch/arm64/include/asm/mmu.h
>>> +++ b/arch/arm64/include/asm/mmu.h
>>> @@ -104,5 +104,11 @@ static inline bool kaslr_requires_kpti(void)
>>> return true;
>>> }
>>>
>>> +#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
>>> +void kpti_install_ng_mappings(void);
>>
>> Could the declarations be moved here instead ?
>
> Why?
To avoid both typedef and external instance declaration in the C
code even though there is just a single call site in there. Also
is not bit cleaner as well ?
>
>> Otherwise LGTM.
>>
>> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
>> index 624edd6c4964..062465939192 100644
>> --- a/arch/arm64/include/asm/mmu.h
>> +++ b/arch/arm64/include/asm/mmu.h
>> @@ -106,6 +106,8 @@ static inline bool kaslr_requires_kpti(void)
>>
>> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
>> void kpti_install_ng_mappings(void);
>> +typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
>> +kpti_remap_fn idmap_kpti_install_ng_mappings;
>> #else
>> static inline void kpti_install_ng_mappings(void) {}
>> #endif
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index eff3295393ee..1b5c3c590e95 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -750,8 +750,6 @@ static void __init create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys,
>>
>> static int __init __kpti_install_ng_mappings(void *__unused)
>> {
>> - typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
>> - extern kpti_remap_fn idmap_kpti_install_ng_mappings;
>> kpti_remap_fn *remap_fn;
>>
>> int cpu = smp_processor_id();
>>
>>> +#else
>>> +static inline void kpti_install_ng_mappings(void) {}
>>> +#endif
>>> +
>>> #endif /* !__ASSEMBLY__ */
>>> #endif
>>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>>> index ef269a5a37e1..b99eaad48c14 100644
>>> --- a/arch/arm64/kernel/cpufeature.c
>>> +++ b/arch/arm64/kernel/cpufeature.c
>>> @@ -1940,103 +1940,6 @@ static bool has_pmuv3(const struct arm64_cpu_capabilities *entry, int scope)
>>> }
>>> #endif
>>>
>>> -#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
>>> -#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT))
>>> -
>>> -extern
>>> -void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
>>> - phys_addr_t size, pgprot_t prot,
>>> - phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags);
>>> -
>>> -static phys_addr_t __initdata kpti_ng_temp_alloc;
>>> -
>>> -static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
>>> -{
>>> - kpti_ng_temp_alloc -= PAGE_SIZE;
>>> - return kpti_ng_temp_alloc;
>>> -}
>>> -
>>> -static int __init __kpti_install_ng_mappings(void *__unused)
>>> -{
>>> - typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
>>> - extern kpti_remap_fn idmap_kpti_install_ng_mappings;
>>> - kpti_remap_fn *remap_fn;
>>> -
>>> - int cpu = smp_processor_id();
>>> - int levels = CONFIG_PGTABLE_LEVELS;
>>> - int order = order_base_2(levels);
>>> - u64 kpti_ng_temp_pgd_pa = 0;
>>> - pgd_t *kpti_ng_temp_pgd;
>>> - u64 alloc = 0;
>>> -
>>> - if (levels == 5 && !pgtable_l5_enabled())
>>> - levels = 4;
>>> - else if (levels == 4 && !pgtable_l4_enabled())
>>> - levels = 3;
>>> -
>>> - remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
>>> -
>>> - if (!cpu) {
>>> - alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order);
>>> - kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE);
>>> - kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd);
>>> -
>>> - //
>>> - // Create a minimal page table hierarchy that permits us to map
>>> - // the swapper page tables temporarily as we traverse them.
>>> - //
>>> - // The physical pages are laid out as follows:
>>> - //
>>> - // +--------+-/-------+-/------ +-/------ +-\\\--------+
>>> - // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] :
>>> - // +--------+-\-------+-\------ +-\------ +-///--------+
>>> - // ^
>>> - // The first page is mapped into this hierarchy at a PMD_SHIFT
>>> - // aligned virtual address, so that we can manipulate the PTE
>>> - // level entries while the mapping is active. The first entry
>>> - // covers the PTE[] page itself, the remaining entries are free
>>> - // to be used as a ad-hoc fixmap.
>>> - //
>>> - create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc),
>>> - KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL,
>>> - kpti_ng_pgd_alloc, 0);
>>> - }
>>> -
>>> - cpu_install_idmap();
>>> - remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA);
>>> - cpu_uninstall_idmap();
>>> -
>>> - if (!cpu) {
>>> - free_pages(alloc, order);
>>> - arm64_use_ng_mappings = true;
>>> - }
>>> -
>>> - return 0;
>>> -}
>>> -
>>> -static void __init kpti_install_ng_mappings(void)
>>> -{
>>> - /* Check whether KPTI is going to be used */
>>> - if (!arm64_kernel_unmapped_at_el0())
>>> - return;
>>> -
>>> - /*
>>> - * We don't need to rewrite the page-tables if either we've done
>>> - * it already or we have KASLR enabled and therefore have not
>>> - * created any global mappings at all.
>>> - */
>>> - if (arm64_use_ng_mappings)
>>> - return;
>>> -
>>> - stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask);
>>> -}
>>> -
>>> -#else
>>> -static inline void kpti_install_ng_mappings(void)
>>> -{
>>> -}
>>> -#endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
>>> -
>>> static void cpu_enable_kpti(struct arm64_cpu_capabilities const *cap)
>>> {
>>> if (__this_cpu_read(this_cpu_vector) == vectors) {
>>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>>> index 183801520740..eff3295393ee 100644
>>> --- a/arch/arm64/mm/mmu.c
>>> +++ b/arch/arm64/mm/mmu.c
>>> @@ -27,6 +27,7 @@
>>> #include <linux/kfence.h>
>>> #include <linux/pkeys.h>
>>> #include <linux/mm_inline.h>
>>> +#include <linux/stop_machine.h>
>>>
>>> #include <asm/barrier.h>
>>> #include <asm/cputype.h>
>>> @@ -466,14 +467,6 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
>>> mutex_unlock(&fixmap_lock);
>>> }
>>>
>>> -#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
>>> -extern __alias(__create_pgd_mapping_locked)
>>> -void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
>>> - phys_addr_t size, pgprot_t prot,
>>> - phys_addr_t (*pgtable_alloc)(enum pgtable_type),
>>> - int flags);
>>> -#endif
>>> -
>>> static phys_addr_t __pgd_pgtable_alloc(struct mm_struct *mm,
>>> enum pgtable_type pgtable_type)
>>> {
>>> @@ -735,7 +728,102 @@ static void __init declare_vma(struct vm_struct *vma,
>>> }
>>>
>>> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
>>> -static pgprot_t kernel_exec_prot(void)
>>> +#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT))
>>> +
>>> +static phys_addr_t kpti_ng_temp_alloc __initdata;
>>> +
>>> +static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
>>> +{
>>> + kpti_ng_temp_alloc -= PAGE_SIZE;
>>> + return kpti_ng_temp_alloc;
>>> +}
>>> +
>>> +static void __init create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys,
>>> + unsigned long virt, phys_addr_t size,
>>> + pgprot_t prot,
>>> + phys_addr_t (*pgtable_alloc)(enum pgtable_type),
>>> + int flags)
>>> +{
>>> + __create_pgd_mapping_locked(pgdir, phys, virt, size, prot,
>>> + pgtable_alloc, flags);
>>> +}
>>> +
>>> +static int __init __kpti_install_ng_mappings(void *__unused)
>>> +{
>>> + typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
>>> + extern kpti_remap_fn idmap_kpti_install_ng_mappings;
>>> + kpti_remap_fn *remap_fn;
>>> +
>>> + int cpu = smp_processor_id();
>>> + int levels = CONFIG_PGTABLE_LEVELS;
>>> + int order = order_base_2(levels);
>>> + u64 kpti_ng_temp_pgd_pa = 0;
>>> + pgd_t *kpti_ng_temp_pgd;
>>> + u64 alloc = 0;
>>> +
>>> + if (levels == 5 && !pgtable_l5_enabled())
>>> + levels = 4;
>>> + else if (levels == 4 && !pgtable_l4_enabled())
>>> + levels = 3;
>>> +
>>> + remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
>>> +
>>> + if (!cpu) {
>>> + alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order);
>>> + kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE);
>>> + kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd);
>>> +
>>> + //
>>> + // Create a minimal page table hierarchy that permits us to map
>>> + // the swapper page tables temporarily as we traverse them.
>>> + //
>>> + // The physical pages are laid out as follows:
>>> + //
>>> + // +--------+-/-------+-/------ +-/------ +-\\\--------+
>>> + // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] :
>>> + // +--------+-\-------+-\------ +-\------ +-///--------+
>>> + // ^
>>> + // The first page is mapped into this hierarchy at a PMD_SHIFT
>>> + // aligned virtual address, so that we can manipulate the PTE
>>> + // level entries while the mapping is active. The first entry
>>> + // covers the PTE[] page itself, the remaining entries are free
>>> + // to be used as a ad-hoc fixmap.
>>> + //
>>> + create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc),
>>> + KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL,
>>> + kpti_ng_pgd_alloc, 0);
>>> + }
>>> +
>>> + cpu_install_idmap();
>>> + remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA);
>>> + cpu_uninstall_idmap();
>>> +
>>> + if (!cpu) {
>>> + free_pages(alloc, order);
>>> + arm64_use_ng_mappings = true;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +void __init kpti_install_ng_mappings(void)
>>> +{
>>> + /* Check whether KPTI is going to be used */
>>> + if (!arm64_kernel_unmapped_at_el0())
>>> + return;
>>> +
>>> + /*
>>> + * We don't need to rewrite the page-tables if either we've done
>>> + * it already or we have KASLR enabled and therefore have not
>>> + * created any global mappings at all.
>>> + */
>>> + if (arm64_use_ng_mappings)
>>> + return;
>>> +
>>> + stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask);
>>> +}
>>> +
>>> +static pgprot_t __init kernel_exec_prot(void)
>>> {
>>> return rodata_enabled ? PAGE_KERNEL_ROX : PAGE_KERNEL_EXEC;
>>> }
>>>
>>> base-commit: 76eeb9b8de9880ca38696b2fb56ac45ac0a25c6c
>>
On Thu, 11 Sept 2025 at 08:18, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
>
>
> On 11/09/25 11:38 AM, Ard Biesheuvel wrote:
> > On Thu, 11 Sept 2025 at 07:13, Anshuman Khandual
> > <anshuman.khandual@arm.com> wrote:
> >>
> >>
> >>
> >> On 10/09/25 4:14 PM, 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)
> >>>
> >>> 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.
> >>>
> >>> [1] https://lore.kernel.org/all/20250813145607.1612234-3-chaitanyas.prakash@arm.com/
> >>> ---
> >>> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> >>> Cc: Ard Biesheuvel <ardb@kernel.org>
> >>> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >>> Cc: Kees Cook <kees@kernel.org>,
> >>> Cc: Mark Rutland <mark.rutland@arm.com>
> >>> Cc: Ryan Roberts <ryan.roberts@arm.com>
> >>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> >>> Cc: Will Deacon <will@kernel.org>
> >>> Cc: Yeoreum Yun <yeoreum.yun@arm.com>
> >>> ---
> >>> arch/arm64/include/asm/mmu.h | 6 ++
> >>> arch/arm64/kernel/cpufeature.c | 97 ------------------------------
> >>> arch/arm64/mm/mmu.c | 106 ++++++++++++++++++++++++++++++---
> >>> 3 files changed, 103 insertions(+), 106 deletions(-)
> >>>
> >>> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> >>> index 49f1a810df16..624edd6c4964 100644
> >>> --- a/arch/arm64/include/asm/mmu.h
> >>> +++ b/arch/arm64/include/asm/mmu.h
> >>> @@ -104,5 +104,11 @@ static inline bool kaslr_requires_kpti(void)
> >>> return true;
> >>> }
> >>>
> >>> +#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> >>> +void kpti_install_ng_mappings(void);
> >>
> >> Could the declarations be moved here instead ?
> >
> > Why?
>
> To avoid both typedef and external instance declaration in the C
> code even though there is just a single call site in there.
But why would we want to avoid those in the first place?
Moving these into mmu.h pollutes the global namespace with
declarations that must never be used outside of
__kpti_install_ng_mappings() to begin with.
> Also
> is not bit cleaner as well ?
That is highly subjective, but personally, I think it only adds confusion.
On 11/09/2025 08:24, Ard Biesheuvel wrote:
> On Thu, 11 Sept 2025 at 08:18, Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
>>
>> On 11/09/25 11:38 AM, Ard Biesheuvel wrote:
>>> On Thu, 11 Sept 2025 at 07:13, Anshuman Khandual
>>> <anshuman.khandual@arm.com> wrote:
>>>>
>>>>
>>>> On 10/09/25 4:14 PM, 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)
>>>>>
>>>>> 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.
>>>>>
>>>>> [1] https://lore.kernel.org/all/20250813145607.1612234-3-chaitanyas.prakash@arm.com/
>>>>> ---
>>>>> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
>>>>> Cc: Ard Biesheuvel <ardb@kernel.org>
>>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>>> Cc: Kees Cook <kees@kernel.org>,
>>>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>>>> Cc: Ryan Roberts <ryan.roberts@arm.com>
>>>>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>>> Cc: Will Deacon <will@kernel.org>
>>>>> Cc: Yeoreum Yun <yeoreum.yun@arm.com>
>>>>> ---
>>>>> arch/arm64/include/asm/mmu.h | 6 ++
>>>>> arch/arm64/kernel/cpufeature.c | 97 ------------------------------
>>>>> arch/arm64/mm/mmu.c | 106 ++++++++++++++++++++++++++++++---
>>>>> 3 files changed, 103 insertions(+), 106 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
>>>>> index 49f1a810df16..624edd6c4964 100644
>>>>> --- a/arch/arm64/include/asm/mmu.h
>>>>> +++ b/arch/arm64/include/asm/mmu.h
>>>>> @@ -104,5 +104,11 @@ static inline bool kaslr_requires_kpti(void)
>>>>> return true;
>>>>> }
>>>>>
>>>>> +#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
>>>>> +void kpti_install_ng_mappings(void);
>>>> Could the declarations be moved here instead ?
>>> Why?
>> To avoid both typedef and external instance declaration in the C
>> code even though there is just a single call site in there.
> But why would we want to avoid those in the first place?
>
> Moving these into mmu.h pollutes the global namespace with
> declarations that must never be used outside of
> __kpti_install_ng_mappings() to begin with.
That makes sense to me - no need for these declarations to be in a
header as they're implementation details of
__kpti_install_ng_mappings(). What is more common is to have them at the
top-level in the .c file instead of inside a function, but I think
either way is fine.
- Kevin
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.
>
> 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().
Either way:
Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
>
> [1] https://lore.kernel.org/all/20250813145607.1612234-3-chaitanyas.prakash@arm.com/
> ---
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Kees Cook <kees@kernel.org>,
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ryan Roberts <ryan.roberts@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Yeoreum Yun <yeoreum.yun@arm.com>
> ---
> arch/arm64/include/asm/mmu.h | 6 ++
> arch/arm64/kernel/cpufeature.c | 97 ------------------------------
> arch/arm64/mm/mmu.c | 106 ++++++++++++++++++++++++++++++---
> 3 files changed, 103 insertions(+), 106 deletions(-)
>
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 49f1a810df16..624edd6c4964 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -104,5 +104,11 @@ static inline bool kaslr_requires_kpti(void)
> return true;
> }
>
> +#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> +void kpti_install_ng_mappings(void);
> +#else
> +static inline void kpti_install_ng_mappings(void) {}
> +#endif
> +
> #endif /* !__ASSEMBLY__ */
> #endif
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index ef269a5a37e1..b99eaad48c14 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1940,103 +1940,6 @@ static bool has_pmuv3(const struct arm64_cpu_capabilities *entry, int scope)
> }
> #endif
>
> -#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> -#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT))
> -
> -extern
> -void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
> - phys_addr_t size, pgprot_t prot,
> - phys_addr_t (*pgtable_alloc)(enum pgtable_type), int flags);
> -
> -static phys_addr_t __initdata kpti_ng_temp_alloc;
> -
> -static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
> -{
> - kpti_ng_temp_alloc -= PAGE_SIZE;
> - return kpti_ng_temp_alloc;
> -}
> -
> -static int __init __kpti_install_ng_mappings(void *__unused)
> -{
> - typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
> - extern kpti_remap_fn idmap_kpti_install_ng_mappings;
> - kpti_remap_fn *remap_fn;
> -
> - int cpu = smp_processor_id();
> - int levels = CONFIG_PGTABLE_LEVELS;
> - int order = order_base_2(levels);
> - u64 kpti_ng_temp_pgd_pa = 0;
> - pgd_t *kpti_ng_temp_pgd;
> - u64 alloc = 0;
> -
> - if (levels == 5 && !pgtable_l5_enabled())
> - levels = 4;
> - else if (levels == 4 && !pgtable_l4_enabled())
> - levels = 3;
> -
> - remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
> -
> - if (!cpu) {
> - alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order);
> - kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE);
> - kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd);
> -
> - //
> - // Create a minimal page table hierarchy that permits us to map
> - // the swapper page tables temporarily as we traverse them.
> - //
> - // The physical pages are laid out as follows:
> - //
> - // +--------+-/-------+-/------ +-/------ +-\\\--------+
> - // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] :
> - // +--------+-\-------+-\------ +-\------ +-///--------+
> - // ^
> - // The first page is mapped into this hierarchy at a PMD_SHIFT
> - // aligned virtual address, so that we can manipulate the PTE
> - // level entries while the mapping is active. The first entry
> - // covers the PTE[] page itself, the remaining entries are free
> - // to be used as a ad-hoc fixmap.
> - //
> - create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc),
> - KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL,
> - kpti_ng_pgd_alloc, 0);
> - }
> -
> - cpu_install_idmap();
> - remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA);
> - cpu_uninstall_idmap();
> -
> - if (!cpu) {
> - free_pages(alloc, order);
> - arm64_use_ng_mappings = true;
> - }
> -
> - return 0;
> -}
> -
> -static void __init kpti_install_ng_mappings(void)
> -{
> - /* Check whether KPTI is going to be used */
> - if (!arm64_kernel_unmapped_at_el0())
> - return;
> -
> - /*
> - * We don't need to rewrite the page-tables if either we've done
> - * it already or we have KASLR enabled and therefore have not
> - * created any global mappings at all.
> - */
> - if (arm64_use_ng_mappings)
> - return;
> -
> - stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask);
> -}
> -
> -#else
> -static inline void kpti_install_ng_mappings(void)
> -{
> -}
> -#endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */
> -
> static void cpu_enable_kpti(struct arm64_cpu_capabilities const *cap)
> {
> if (__this_cpu_read(this_cpu_vector) == vectors) {
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 183801520740..eff3295393ee 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -27,6 +27,7 @@
> #include <linux/kfence.h>
> #include <linux/pkeys.h>
> #include <linux/mm_inline.h>
> +#include <linux/stop_machine.h>
>
> #include <asm/barrier.h>
> #include <asm/cputype.h>
> @@ -466,14 +467,6 @@ static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys,
> mutex_unlock(&fixmap_lock);
> }
>
> -#ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> -extern __alias(__create_pgd_mapping_locked)
> -void create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys, unsigned long virt,
> - phys_addr_t size, pgprot_t prot,
> - phys_addr_t (*pgtable_alloc)(enum pgtable_type),
> - int flags);
> -#endif
> -
> static phys_addr_t __pgd_pgtable_alloc(struct mm_struct *mm,
> enum pgtable_type pgtable_type)
> {
> @@ -735,7 +728,102 @@ static void __init declare_vma(struct vm_struct *vma,
> }
>
> #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
> -static pgprot_t kernel_exec_prot(void)
> +#define KPTI_NG_TEMP_VA (-(1UL << PMD_SHIFT))
> +
> +static phys_addr_t kpti_ng_temp_alloc __initdata;
> +
> +static phys_addr_t __init kpti_ng_pgd_alloc(enum pgtable_type type)
> +{
> + kpti_ng_temp_alloc -= PAGE_SIZE;
> + return kpti_ng_temp_alloc;
> +}
> +
> +static void __init create_kpti_ng_temp_pgd(pgd_t *pgdir, phys_addr_t phys,
> + unsigned long virt, phys_addr_t size,
> + pgprot_t prot,
> + phys_addr_t (*pgtable_alloc)(enum pgtable_type),
> + int flags)
> +{
> + __create_pgd_mapping_locked(pgdir, phys, virt, size, prot,
> + pgtable_alloc, flags);
> +}
> +
> +static int __init __kpti_install_ng_mappings(void *__unused)
> +{
> + typedef void (kpti_remap_fn)(int, int, phys_addr_t, unsigned long);
> + extern kpti_remap_fn idmap_kpti_install_ng_mappings;
> + kpti_remap_fn *remap_fn;
> +
> + int cpu = smp_processor_id();
> + int levels = CONFIG_PGTABLE_LEVELS;
> + int order = order_base_2(levels);
> + u64 kpti_ng_temp_pgd_pa = 0;
> + pgd_t *kpti_ng_temp_pgd;
> + u64 alloc = 0;
> +
> + if (levels == 5 && !pgtable_l5_enabled())
> + levels = 4;
> + else if (levels == 4 && !pgtable_l4_enabled())
> + levels = 3;
> +
> + remap_fn = (void *)__pa_symbol(idmap_kpti_install_ng_mappings);
> +
> + if (!cpu) {
> + alloc = __get_free_pages(GFP_ATOMIC | __GFP_ZERO, order);
> + kpti_ng_temp_pgd = (pgd_t *)(alloc + (levels - 1) * PAGE_SIZE);
> + kpti_ng_temp_alloc = kpti_ng_temp_pgd_pa = __pa(kpti_ng_temp_pgd);
> +
> + //
> + // Create a minimal page table hierarchy that permits us to map
> + // the swapper page tables temporarily as we traverse them.
> + //
> + // The physical pages are laid out as follows:
> + //
> + // +--------+-/-------+-/------ +-/------ +-\\\--------+
> + // : PTE[] : | PMD[] : | PUD[] : | P4D[] : ||| PGD[] :
> + // +--------+-\-------+-\------ +-\------ +-///--------+
> + // ^
> + // The first page is mapped into this hierarchy at a PMD_SHIFT
> + // aligned virtual address, so that we can manipulate the PTE
> + // level entries while the mapping is active. The first entry
> + // covers the PTE[] page itself, the remaining entries are free
> + // to be used as a ad-hoc fixmap.
> + //
> + create_kpti_ng_temp_pgd(kpti_ng_temp_pgd, __pa(alloc),
> + KPTI_NG_TEMP_VA, PAGE_SIZE, PAGE_KERNEL,
> + kpti_ng_pgd_alloc, 0);
> + }
> +
> + cpu_install_idmap();
> + remap_fn(cpu, num_online_cpus(), kpti_ng_temp_pgd_pa, KPTI_NG_TEMP_VA);
> + cpu_uninstall_idmap();
> +
> + if (!cpu) {
> + free_pages(alloc, order);
> + arm64_use_ng_mappings = true;
> + }
> +
> + return 0;
> +}
> +
> +void __init kpti_install_ng_mappings(void)
> +{
> + /* Check whether KPTI is going to be used */
> + if (!arm64_kernel_unmapped_at_el0())
> + return;
> +
> + /*
> + * We don't need to rewrite the page-tables if either we've done
> + * it already or we have KASLR enabled and therefore have not
> + * created any global mappings at all.
> + */
> + if (arm64_use_ng_mappings)
> + return;
> +
> + stop_machine(__kpti_install_ng_mappings, NULL, cpu_online_mask);
> +}
> +
> +static pgprot_t __init kernel_exec_prot(void)
nit: this change (to add __init) is unrelated; does it deserve it's own patch?
> {
> return rodata_enabled ? PAGE_KERNEL_ROX : PAGE_KERNEL_EXEC;
> }
>
> base-commit: 76eeb9b8de9880ca38696b2fb56ac45ac0a25c6c
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
© 2016 - 2025 Red Hat, Inc.