From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Nicholas Piggin <npiggin@gmail.com>, linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v1 09/11] powerpc/64s: Make hash MMU code build configurable
Date: Tue, 19 Oct 2021 10:05:56 +0200 [thread overview]
Message-ID: <eb342e34-8a97-a7f1-ee56-c8874b1bcd85@csgroup.eu> (raw)
In-Reply-To: <20211015154624.922960-10-npiggin@gmail.com>
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%)
>
> 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 ?
>
> #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);
> }
> 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);
>
> 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 ?
> +#endif
> mm->context.id = MMU_NO_CONTEXT;
> }
>
next prev parent reply other threads:[~2021-10-19 8:23 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-15 15:46 [PATCH v1 00/11] powerpc: Make hash MMU code build configurable Nicholas Piggin
2021-10-15 15:46 ` [PATCH v1 01/11] powerpc: Remove unused FW_FEATURE_NATIVE references Nicholas Piggin
2021-10-15 15:46 ` [PATCH v1 02/11] powerpc: Rename PPC_NATIVE to PPC_HASH_MMU_NATIVE Nicholas Piggin
2021-10-15 15:46 ` [PATCH v1 03/11] powerpc/pseries: Stop selecting PPC_HASH_MMU_NATIVE Nicholas Piggin
2021-10-15 15:46 ` [PATCH v1 04/11] powerpc/64s: Move and rename do_bad_slb_fault as it is not hash specific Nicholas Piggin
2021-10-18 17:09 ` Christophe Leroy
2021-10-20 5:07 ` Nicholas Piggin
2021-10-15 15:46 ` [PATCH v1 05/11] powerpc/pseries: move pseries_lpar_register_process_table() out from hash specific code Nicholas Piggin
2021-10-15 15:46 ` [PATCH v1 06/11] powerpc/pseries: lparcfg don't include slb_size line in radix mode Nicholas Piggin
2021-10-15 15:46 ` [PATCH v1 07/11] powerpc/64s: move THP trace point creation out of hash specific file Nicholas Piggin
2021-10-15 15:46 ` [PATCH v1 08/11] powerpc/64s: Make flush_and_reload_slb a no-op when radix is enabled Nicholas Piggin
2021-10-15 15:46 ` [PATCH v1 09/11] powerpc/64s: Make hash MMU code build configurable Nicholas Piggin
2021-10-19 8:05 ` Christophe Leroy [this message]
2021-10-20 5:20 ` Nicholas Piggin
2021-10-15 15:46 ` [PATCH v1 10/11] powerpc/configs/microwatt: add POWER9_CPU Nicholas Piggin
2021-10-15 15:46 ` [PATCH v1 11/11] powerpc/microwatt: Don't select the hash MMU code Nicholas Piggin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=eb342e34-8a97-a7f1-ee56-c8874b1bcd85@csgroup.eu \
--to=christophe.leroy@csgroup.eu \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=npiggin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).