Re: [PATCH v1 09/11] powerpc/64s: Make hash MMU code build configurable
From: Christophe Leroy <hidden>
Date: 2021-10-19 08:23:06
Le 15/10/2021 à 17:46, Nicholas Piggin a écrit :
Introduce a new option CONFIG_PPC_64S_HASH_MMU which allows the 64s hash MMU code to be compiled out if radix is selected and the minimum supported CPU type is POWER9 or higher, and KVM is not selected. This saves 128kB kernel image size (90kB text) on powernv_defconfig minus KVM, 350kB on pseries_defconfig minus KVM, 40kB on a tiny config.
This patch is huge, it could be split in several smaller patches ? I'm sure at least the Kconfig stuff can be do as a second step. In first step just make CONFIG_PPC_64S_HASH_MMU always y. I'm wondering if we could also reduce the amount of #ifdefs in C files, by using IS_ENABLED() and/or stubs defined in H files.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/include/asm/book3s/64/mmu.h | 22 ++++++++++++++++++-
.../include/asm/book3s/64/tlbflush-hash.h | 7 ++++++
arch/powerpc/include/asm/book3s/pgtable.h | 4 ++++
arch/powerpc/include/asm/mmu.h | 14 +++++++++---
arch/powerpc/include/asm/mmu_context.h | 2 ++
arch/powerpc/include/asm/paca.h | 8 +++++++
arch/powerpc/kernel/asm-offsets.c | 2 ++
arch/powerpc/kernel/dt_cpu_ftrs.c | 8 ++++++-
arch/powerpc/kernel/entry_64.S | 4 ++--
arch/powerpc/kernel/exceptions-64s.S | 16 ++++++++++++++
arch/powerpc/kernel/mce.c | 2 +-
arch/powerpc/kernel/mce_power.c | 10 ++++++---
arch/powerpc/kernel/paca.c | 18 ++++++---------
arch/powerpc/kernel/process.c | 13 ++++++-----
arch/powerpc/kernel/prom.c | 2 ++
arch/powerpc/kernel/setup_64.c | 4 ++++
arch/powerpc/kexec/core_64.c | 4 ++--
arch/powerpc/kexec/ranges.c | 4 ++++
arch/powerpc/kvm/Kconfig | 1 +
arch/powerpc/mm/book3s64/Makefile | 17 ++++++++------
arch/powerpc/mm/book3s64/hash_utils.c | 10 ---------
.../{hash_hugetlbpage.c => hugetlbpage.c} | 6 +++++
arch/powerpc/mm/book3s64/mmu_context.c | 16 ++++++++++++++
arch/powerpc/mm/book3s64/pgtable.c | 12 ++++++++++
arch/powerpc/mm/book3s64/radix_pgtable.c | 4 ++++
arch/powerpc/mm/copro_fault.c | 2 ++
arch/powerpc/mm/pgtable.c | 10 ++++++---
arch/powerpc/platforms/Kconfig.cputype | 21 +++++++++++++++++-
arch/powerpc/platforms/cell/Kconfig | 1 +
arch/powerpc/platforms/maple/Kconfig | 1 +
arch/powerpc/platforms/microwatt/Kconfig | 2 +-
arch/powerpc/platforms/pasemi/Kconfig | 1 +
arch/powerpc/platforms/powermac/Kconfig | 1 +
arch/powerpc/platforms/powernv/Kconfig | 2 +-
arch/powerpc/platforms/powernv/idle.c | 2 ++
arch/powerpc/platforms/powernv/setup.c | 2 ++
arch/powerpc/platforms/pseries/lpar.c | 11 ++++++++--
arch/powerpc/platforms/pseries/lparcfg.c | 2 +-
arch/powerpc/platforms/pseries/mobility.c | 6 +++++
arch/powerpc/platforms/pseries/ras.c | 2 ++
arch/powerpc/platforms/pseries/reconfig.c | 2 ++
arch/powerpc/platforms/pseries/setup.c | 6 +++--
arch/powerpc/xmon/xmon.c | 8 +++++--
44 files changed, 233 insertions(+), 60 deletions(-)
rename arch/powerpc/mm/book3s64/{hash_hugetlbpage.c => hugetlbpage.c} (95%)quoted hunk ↗ jump to hunk
diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h index 8abe8e42e045..0f89fcab834d 100644 --- a/arch/powerpc/include/asm/mmu.h +++ b/arch/powerpc/include/asm/mmu.h@@ -157,7 +157,7 @@ DECLARE_PER_CPU(int, next_tlbcam_idx); enum { MMU_FTRS_POSSIBLE = -#if defined(CONFIG_PPC_BOOK3S_64) || defined(CONFIG_PPC_BOOK3S_604) +#if defined(CONFIG_PPC_BOOK3S_604) MMU_FTR_HPTE_TABLE | #endif #ifdef CONFIG_PPC_8xx@@ -184,15 +184,18 @@ enum { MMU_FTR_USE_TLBRSRV | MMU_FTR_USE_PAIRED_MAS | #endif #ifdef CONFIG_PPC_BOOK3S_64 + MMU_FTR_KERNEL_RO | +#ifdef CONFIG_PPC_64S_HASH_MMU MMU_FTR_NO_SLBIE_B | MMU_FTR_16M_PAGE | MMU_FTR_TLBIEL | MMU_FTR_LOCKLESS_TLBIE | MMU_FTR_CI_LARGE_PAGE | MMU_FTR_1T_SEGMENT | MMU_FTR_TLBIE_CROP_VA | - MMU_FTR_KERNEL_RO | MMU_FTR_68_BIT_VA | + MMU_FTR_68_BIT_VA | MMU_FTR_HPTE_TABLE | #endif #ifdef CONFIG_PPC_RADIX_MMU MMU_FTR_TYPE_RADIX | MMU_FTR_GTSE | #endif /* CONFIG_PPC_RADIX_MMU */ +#endif #ifdef CONFIG_PPC_KUAP MMU_FTR_BOOK3S_KUAP | #endif /* CONFIG_PPC_KUAP */@@ -223,6 +226,11 @@ enum { #ifdef CONFIG_E500 #define MMU_FTRS_ALWAYS MMU_FTR_TYPE_FSL_E #endif +#ifdef CONFIG_PPC_BOOK3S_64 +#if defined(CONFIG_PPC_RADIX_MMU) && !defined(CONFIG_PPC_64S_HASH_MMU) +#define MMU_FTRS_ALWAYS MMU_FTR_TYPE_RADIX +#endif +#endif
Should you also set MMU_FTR_HPTE_TABLE in MMU_FTRS_ALWAYS when HAS_MMU && !RADIX ?
quoted hunk ↗ jump to hunk
#ifndef MMU_FTRS_ALWAYS #define MMU_FTRS_ALWAYS 0@@ -329,7 +337,7 @@ static __always_inline bool radix_enabled(void) return mmu_has_feature(MMU_FTR_TYPE_RADIX); } -static inline bool early_radix_enabled(void) +static __always_inline bool early_radix_enabled(void) { return early_mmu_has_feature(MMU_FTR_TYPE_RADIX); }
quoted hunk ↗ jump to hunk
diff --git a/arch/powerpc/mm/book3s64/mmu_context.c b/arch/powerpc/mm/book3s64/mmu_context.c index c10fc8a72fb3..642cabc25e99 100644 --- a/arch/powerpc/mm/book3s64/mmu_context.c +++ b/arch/powerpc/mm/book3s64/mmu_context.c@@ -31,6 +31,7 @@ static int alloc_context_id(int min_id, int max_id) return ida_alloc_range(&mmu_context_ida, min_id, max_id, GFP_KERNEL); } +#ifdef CONFIG_PPC_64S_HASH_MMU void hash__reserve_context_id(int id) { int result = ida_alloc_range(&mmu_context_ida, id, id, GFP_KERNEL);@@ -50,7 +51,9 @@ int hash__alloc_context_id(void) return alloc_context_id(MIN_USER_CONTEXT, max); } EXPORT_SYMBOL_GPL(hash__alloc_context_id); +#endif +#ifdef CONFIG_PPC_64S_HASH_MMU static int realloc_context_ids(mm_context_t *ctx) { int i, id;@@ -144,12 +147,15 @@ static int hash__init_new_context(struct mm_struct *mm) return index; } +void slb_setup_new_exec(void); + void hash__setup_new_exec(void) { slice_setup_new_exec(); slb_setup_new_exec(); } +#endif static int radix__init_new_context(struct mm_struct *mm) {@@ -175,7 +181,9 @@ static int radix__init_new_context(struct mm_struct *mm) */ asm volatile("ptesync;isync" : : : "memory"); +#ifdef CONFIG_PPC_64S_HASH_MMU mm->context.hash_context = NULL; +#endif return index; }@@ -186,8 +194,10 @@ int init_new_context(struct task_struct *tsk, struct mm_struct *mm) if (radix_enabled()) index = radix__init_new_context(mm); +#ifdef CONFIG_PPC_64S_HASH_MMU else index = hash__init_new_context(mm); +#endif
I really dislike #ifdef nested in if/else. Can you do something like if (radix_enabled() index = radix__init_new_context(mm); else if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) index = hash__init_new_context(mm);
quoted hunk ↗ jump to hunk
if (index < 0) return index;@@ -211,6 +221,7 @@ void __destroy_context(int context_id) } EXPORT_SYMBOL_GPL(__destroy_context); +#ifdef CONFIG_PPC_64S_HASH_MMU static void destroy_contexts(mm_context_t *ctx) { int index, context_id;@@ -222,6 +233,7 @@ static void destroy_contexts(mm_context_t *ctx) } kfree(ctx->hash_context); } +#endif static void pmd_frag_destroy(void *pmd_frag) {@@ -274,7 +286,11 @@ void destroy_context(struct mm_struct *mm) process_tb[mm->context.id].prtb0 = 0; else subpage_prot_free(mm); +#ifdef CONFIG_PPC_64S_HASH_MMU destroy_contexts(&mm->context); +#else + ida_free(&mmu_context_ida, mm->context.id);
Is that correct ? Was it done somewhere else before ?
quoted hunk ↗ jump to hunk
+#endif mm->context.id = MMU_NO_CONTEXT; }