Re: [PATCH v2 13/16] powerpc/64s: Move hash MMU code under a new Kconfig name
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2021-10-21 07:34:41
Excerpts from Christophe Leroy's message of October 21, 2021 3:43 pm:
Le 21/10/2021 à 05:54, Nicholas Piggin a écrit :quoted
Introduce a new option CONFIG_PPC_64S_HASH_MMU, and make 64s hash code depend on it. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- arch/powerpc/Kconfig | 2 +- arch/powerpc/include/asm/book3s/64/mmu.h | 19 +++++++++-- .../include/asm/book3s/64/tlbflush-hash.h | 7 ++++ arch/powerpc/include/asm/book3s/pgtable.h | 4 +++ arch/powerpc/include/asm/mmu.h | 16 +++++++-- 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 | 14 +++++--- 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 | 5 +++ arch/powerpc/kexec/core_64.c | 4 +-- arch/powerpc/kexec/ranges.c | 4 +++ arch/powerpc/mm/book3s64/Makefile | 15 ++++---- arch/powerpc/mm/book3s64/hugetlbpage.c | 2 ++ arch/powerpc/mm/book3s64/mmu_context.c | 34 +++++++++++++++---- arch/powerpc/mm/book3s64/radix_pgtable.c | 4 +++ arch/powerpc/mm/copro_fault.c | 2 ++ arch/powerpc/mm/ioremap.c | 13 ++++--- arch/powerpc/mm/pgtable.c | 10 ++++-- arch/powerpc/mm/ptdump/Makefile | 2 +- arch/powerpc/platforms/Kconfig.cputype | 4 +++ 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 +++-- drivers/misc/lkdtm/Makefile | 2 +- drivers/misc/lkdtm/core.c | 2 +- 39 files changed, 219 insertions(+), 64 deletions(-)I'm still unconfortable with the quantity of files impacted in that commit.
Hmm. Splitting it into N partial patches that have the same result doesn't seem better to me. There's a few other little things that are be better split out, but size of the patch will not shrink much.
quoted
diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h index c02f42d1031e..d94ebae386b6 100644 --- a/arch/powerpc/include/asm/book3s/64/mmu.h +++ b/arch/powerpc/include/asm/book3s/64/mmu.hquoted
@@ -193,8 +198,15 @@ static inline struct subpage_prot_table *mm_ctx_subpage_prot(mm_context_t *ctx) extern int mmu_linear_psize; extern int mmu_virtual_psize; extern int mmu_vmalloc_psize; -extern int mmu_vmemmap_psize; extern int mmu_io_psize; +#else /* CONFIG_PPC_64S_HASH_MMU */ +#ifdef CONFIG_PPC_64K_PAGESAvoid nested #ifdefs and do #elif defined(CONFIG_PPC_64K_PAGES)
I sort of like the nesting because this is the !HASH block. But I don't care much so I can do it your way if you prefer.
quoted
@@ -223,6 +226,13 @@ enum { #ifdef CONFIG_E500 #define MMU_FTRS_ALWAYS MMU_FTR_TYPE_FSL_E #endif +#ifdef CONFIG_PPC_BOOK3S_64No need of this CONFIG_PPC_BOOK3S_64 ifdef, it is necessarily defined if either CONFIG_PPC_RADIX_MMU or CONFIG_PPC_64S_HASH_MMU is defined
Good point.
quoted
+#if defined(CONFIG_PPC_RADIX_MMU) && !defined(CONFIG_PPC_64S_HASH_MMU) +#define MMU_FTRS_ALWAYS MMU_FTR_TYPE_RADIX +#elif !defined(CONFIG_PPC_RADIX_MMU) && defined(CONFIG_PPC_64S_HASH_MMU) +#define MMU_FTRS_ALWAYS MMU_FTR_HPTE_TABLE +#endif +#endif #ifndef MMU_FTRS_ALWAYS #define MMU_FTRS_ALWAYS 0quoted
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S index 046c99e31d01..65b695e9401e 100644 --- a/arch/powerpc/kernel/exceptions-64s.S +++ b/arch/powerpc/kernel/exceptions-64s.S@@ -1369,11 +1369,15 @@ EXC_COMMON_BEGIN(data_access_common) addi r3,r1,STACK_FRAME_OVERHEAD andis. r0,r4,DSISR_DABRMATCH@h bne- 1f +#ifdef CONFIG_PPC_64S_HASH_MMU BEGIN_MMU_FTR_SECTION bl do_hash_fault MMU_FTR_SECTION_ELSE bl do_page_fault ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX) +#else + bl do_page_fault +#endifMaybe we could always branch to do_page_fault() and get redirected to do_hash_fault() from do_page_fault() if BOOK3S64 && !radix_enabled() ?
Not trivial because of do_hash_fault(). A book3s_64_do_page_fault() raw hander which calls do_hash_fault or do_page_fault if necessary, but that's less optimal and we already have this gunk so might as well keep it.
Another solution is to make it a GAS macro ?
For this I don't find it too bad. Another macro means another thing to look up. Maybe if we had more callers but for now this isn't too terrible.
quoted
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c index 7d4bcbc3124e..59b1a1833143 100644 --- a/arch/powerpc/kernel/setup_64.c +++ b/arch/powerpc/kernel/setup_64.c@@ -887,6 +887,8 @@ void __init setup_per_cpu_areas(void) } else if (radix_enabled()) { atom_size = PAGE_SIZE; } else { +#ifdef CONFIG_PPC_64S_HASH_MMUUse IS_ENABLED()
Can't because of mmu_linear_psize.
quoted
diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c index cd16b407f47e..ab105d33e0b0 100644 --- a/arch/powerpc/mm/pgtable.c +++ b/arch/powerpc/mm/pgtable.c@@ -81,9 +81,6 @@ static struct page *maybe_pte_to_page(pte_t pte) static pte_t set_pte_filter_hash(pte_t pte) { - if (radix_enabled()) - return pte; - pte = __pte(pte_val(pte) & ~_PAGE_HPTEFLAGS); if (pte_looks_normal(pte) && !(cpu_has_feature(CPU_FTR_COHERENT_ICACHE) || cpu_has_feature(CPU_FTR_NOEXECUTE))) {@@ -112,6 +109,9 @@ static inline pte_t set_pte_filter(pte_t pte) { struct page *pg; + if (radix_enabled()) + return pte; + if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) return set_pte_filter_hash(pte);@@ -144,6 +144,10 @@ static pte_t set_access_flags_filter(pte_t pte, struct vm_area_struct *vma, { struct page *pg; +#ifdef CONFIG_PPC_BOOK3S_64 + return pte; +#endifAren't you changing the behaviour here for RADIX ?
No, 64s always has MMU_FTR_HPTE_TABLE until now. I should actually split this change out.
Anyway, can use IS_ENABLED()
Good point.
quoted
+ if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) return pte;diff --git a/arch/powerpc/mm/ptdump/Makefile b/arch/powerpc/mm/ptdump/Makefile index 4050cbb55acf..b533caaf0910 100644 --- a/arch/powerpc/mm/ptdump/Makefile +++ b/arch/powerpc/mm/ptdump/Makefile@@ -10,5 +10,5 @@ obj-$(CONFIG_PPC_BOOK3S_64) += book3s64.o ifdef CONFIG_PTDUMP_DEBUGFS obj-$(CONFIG_PPC_BOOK3S_32) += bats.o segment_regs.o -obj-$(CONFIG_PPC_BOOK3S_64) += hashpagetable.o +obj-$(CONFIG_PPC_64S_HASH_MMU) += hashpagetable.o endifdiff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype index a208997ade88..01726e7f2c7f 100644 --- a/arch/powerpc/platforms/Kconfig.cputype +++ b/arch/powerpc/platforms/Kconfig.cputype@@ -105,6 +105,7 @@ config PPC_BOOK3S_64 select HAVE_MOVE_PMD select HAVE_MOVE_PUD select IRQ_WORK + select PPC_64S_HASH_MMUNot neededquoted
select PPC_MM_SLICES select PPC_HAVE_KUEP select PPC_HAVE_KUAP@@ -364,6 +365,9 @@ config SPE If in doubt, say Y here. +config PPC_64S_HASH_MMU + boolAdd depends on PPC_BOOK3S_64 default yquoted
+ config PPC_RADIX_MMU bool "Radix MMU Support" depends on PPC_BOOK3S_64quoted
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c index dd8241c009e5..33de8d798c95 100644 --- a/arch/powerpc/xmon/xmon.c +++ b/arch/powerpc/xmon/xmon.c@@ -1160,9 +1160,11 @@ cmds(struct pt_regs *excp) show_tasks(); break; #ifdef CONFIG_PPC_BOOK3S +#ifdef CONFIG_PPC_64S_HASH_MMUAnd BOOK3S/32 ???
Oops, good catch. Thanks, Nick
quoted
case 'u': dump_segments(); break; +#endif #elif defined(CONFIG_44x) case 'u': dump_tlb_44x();