linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] powerpc/64s: POWER9 DD2.3 CPU feature flag fixes
@ 2021-07-26  3:17 Nicholas Piggin
  2021-07-26  3:17 ` [PATCH 2/2] powerpc/64s: Rename CPU_FTR_POWER9_DD2_1 to CPU_FTR_P9_STOP_FIXED Nicholas Piggin
  2022-08-03  8:57 ` [PATCH 1/2] powerpc/64s: POWER9 DD2.3 CPU feature flag fixes Michael Ellerman
  0 siblings, 2 replies; 3+ messages in thread
From: Nicholas Piggin @ 2021-07-26  3:17 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

DD2.3 missed out on getting its feature flag bits.

This meant when booting with dt-cpu-ftrs, CPU_FTR_P9_TM_HV_ASSIST is
missing (unless the firmware contains it, which mine does not seem to).
And when booting without, CPU_FTR_P9_TM_XER_SO_BUG is set.

In practice this doesn't make any difference to pseries guests, only
powernv.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/cputable.h |  2 ++
 arch/powerpc/kernel/cputable.c      | 22 ++++++++++++++++++++--
 arch/powerpc/kernel/dt_cpu_ftrs.c   | 14 +++++---------
 3 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index e85c849214a2..46bae9624784 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -440,6 +440,8 @@ static inline void cpu_feature_keys_init(void) { }
 #define CPU_FTRS_POWER9_DD2_2 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD2_1 | \
 			       CPU_FTR_P9_TM_HV_ASSIST | \
 			       CPU_FTR_P9_TM_XER_SO_BUG)
+#define CPU_FTRS_POWER9_DD2_3 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD2_1 | \
+			       CPU_FTR_P9_TM_HV_ASSIST)
 #define CPU_FTRS_POWER10 (CPU_FTR_LWSYNC | \
 	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\
 	    CPU_FTR_MMCRA | CPU_FTR_SMT | \
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index ae0fdef0ac11..9ab97d1fd5a2 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -487,11 +487,29 @@ static struct cpu_spec __initdata cpu_specs[] = {
 		.machine_check_early	= __machine_check_early_realmode_p9,
 		.platform		= "power9",
 	},
-	{	/* Power9 DD2.2 or later */
+	{	/* Power9 DD 2.2 */
+		.pvr_mask		= 0xffffefff,
+		.pvr_value		= 0x004e0202,
+		.cpu_name		= "POWER9 (raw)",
+		.cpu_features		= CPU_FTRS_POWER9_DD2_2,
+		.cpu_user_features	= COMMON_USER_POWER9,
+		.cpu_user_features2	= COMMON_USER2_POWER9,
+		.mmu_features		= MMU_FTRS_POWER9,
+		.icache_bsize		= 128,
+		.dcache_bsize		= 128,
+		.num_pmcs		= 6,
+		.pmc_type		= PPC_PMC_IBM,
+		.oprofile_cpu_type	= "ppc64/power9",
+		.cpu_setup		= __setup_cpu_power9,
+		.cpu_restore		= __restore_cpu_power9,
+		.machine_check_early	= __machine_check_early_realmode_p9,
+		.platform		= "power9",
+	},
+	{	/* Power9 DD 2.3 or later */
 		.pvr_mask		= 0xffff0000,
 		.pvr_value		= 0x004e0000,
 		.cpu_name		= "POWER9 (raw)",
-		.cpu_features		= CPU_FTRS_POWER9_DD2_2,
+		.cpu_features		= CPU_FTRS_POWER9_DD2_3,
 		.cpu_user_features	= COMMON_USER_POWER9,
 		.cpu_user_features2	= COMMON_USER2_POWER9,
 		.mmu_features		= MMU_FTRS_POWER9,
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 358aee7c2d79..af95f337e54b 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -764,18 +764,14 @@ static __init void cpufeatures_cpu_quirks(void)
 	 * Not all quirks can be derived from the cpufeatures device tree.
 	 */
 	if ((version & 0xffffefff) == 0x004e0200) {
-		/* DD2.0 has no feature flag */
-		cur_cpu_spec->cpu_features |= CPU_FTR_P9_RADIX_PREFETCH_BUG;
+		cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_0;
 	} else if ((version & 0xffffefff) == 0x004e0201) {
-		cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD2_1;
-		cur_cpu_spec->cpu_features |= CPU_FTR_P9_RADIX_PREFETCH_BUG;
+		cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_1;
 	} else if ((version & 0xffffefff) == 0x004e0202) {
-		cur_cpu_spec->cpu_features |= CPU_FTR_P9_TM_HV_ASSIST;
-		cur_cpu_spec->cpu_features |= CPU_FTR_P9_TM_XER_SO_BUG;
-		cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD2_1;
+		cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_2;
 	} else if ((version & 0xffff0000) == 0x004e0000) {
-		/* DD2.1 and up have DD2_1 */
-		cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD2_1;
+		/* DD2.3 and up */
+		cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_3;
 	}
 
 	if ((version & 0xffff0000) == 0x004e0000) {
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* [PATCH 2/2] powerpc/64s: Rename CPU_FTR_POWER9_DD2_1 to CPU_FTR_P9_STOP_FIXED
  2021-07-26  3:17 [PATCH 1/2] powerpc/64s: POWER9 DD2.3 CPU feature flag fixes Nicholas Piggin
@ 2021-07-26  3:17 ` Nicholas Piggin
  2022-08-03  8:57 ` [PATCH 1/2] powerpc/64s: POWER9 DD2.3 CPU feature flag fixes Michael Ellerman
  1 sibling, 0 replies; 3+ messages in thread
From: Nicholas Piggin @ 2021-07-26  3:17 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

CPU feature flags work best when they are named for behaviour, not for
the CPU variant that first introduced them. Later revisions might also
contain the behaviour, for example. It's confusing for a POWER9 DD2.2
to have CPU_FTR_POWER9_DD2_1, but it's not confusing if DD2.1 and DD2.2
both have CPU_FTR_P9_STOP_FIXED.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/cputable.h   | 8 ++++----
 arch/powerpc/platforms/powernv/idle.c | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 46bae9624784..cb9948f318f7 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -186,7 +186,7 @@ static inline void cpu_feature_keys_init(void) { }
 #define CPU_FTR_DAWR			LONG_ASM_CONST(0x0000008000000000)
 #define CPU_FTR_DABRX			LONG_ASM_CONST(0x0000010000000000)
 #define CPU_FTR_PMAO_BUG		LONG_ASM_CONST(0x0000020000000000)
-#define CPU_FTR_POWER9_DD2_1		LONG_ASM_CONST(0x0000080000000000)
+#define CPU_FTR_P9_STOP_FIXED		LONG_ASM_CONST(0x0000080000000000)
 #define CPU_FTR_P9_TM_HV_ASSIST		LONG_ASM_CONST(0x0000100000000000)
 #define CPU_FTR_P9_TM_XER_SO_BUG	LONG_ASM_CONST(0x0000200000000000)
 #define CPU_FTR_P9_TLBIE_STQ_BUG	LONG_ASM_CONST(0x0000400000000000)
@@ -436,11 +436,11 @@ static inline void cpu_feature_keys_init(void) { }
 #define CPU_FTRS_POWER9_DD2_0 (CPU_FTRS_POWER9 | CPU_FTR_P9_RADIX_PREFETCH_BUG)
 #define CPU_FTRS_POWER9_DD2_1 (CPU_FTRS_POWER9 | \
 			       CPU_FTR_P9_RADIX_PREFETCH_BUG | \
-			       CPU_FTR_POWER9_DD2_1)
-#define CPU_FTRS_POWER9_DD2_2 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD2_1 | \
+			       CPU_FTR_P9_STOP_FIXED)
+#define CPU_FTRS_POWER9_DD2_2 (CPU_FTRS_POWER9 | CPU_FTR_P9_STOP_FIXED | \
 			       CPU_FTR_P9_TM_HV_ASSIST | \
 			       CPU_FTR_P9_TM_XER_SO_BUG)
-#define CPU_FTRS_POWER9_DD2_3 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD2_1 | \
+#define CPU_FTRS_POWER9_DD2_3 (CPU_FTRS_POWER9 | CPU_FTR_P9_STOP_FIXED | \
 			       CPU_FTR_P9_TM_HV_ASSIST)
 #define CPU_FTRS_POWER10 (CPU_FTR_LWSYNC | \
 	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 528a7e0cf83a..1e908536890b 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -650,7 +650,7 @@ static unsigned long power9_idle_stop(unsigned long psscr)
 	}
 #endif
 
-	if (!cpu_has_feature(CPU_FTR_POWER9_DD2_1)) {
+	if (!cpu_has_feature(CPU_FTR_P9_STOP_FIXED)) {
 		 /*
 		  * POWER9 DD2 can incorrectly set PMAO when waking up
 		  * after a state-loss idle. Saving and restoring MMCR0
@@ -717,7 +717,7 @@ static unsigned long power9_idle_stop(unsigned long psscr)
 		 * might have been corrupted and needs flushing. We also need
 		 * to reload MMCR0 (see mmcr0 comment above).
 		 */
-		if (!cpu_has_feature(CPU_FTR_POWER9_DD2_1)) {
+		if (!cpu_has_feature(CPU_FTR_P9_STOP_FIXED)) {
 			asm volatile(PPC_ISA_3_0_INVALIDATE_ERAT);
 			mtspr(SPRN_MMCR0, mmcr0);
 		}
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH 1/2] powerpc/64s: POWER9 DD2.3 CPU feature flag fixes
  2021-07-26  3:17 [PATCH 1/2] powerpc/64s: POWER9 DD2.3 CPU feature flag fixes Nicholas Piggin
  2021-07-26  3:17 ` [PATCH 2/2] powerpc/64s: Rename CPU_FTR_POWER9_DD2_1 to CPU_FTR_P9_STOP_FIXED Nicholas Piggin
@ 2022-08-03  8:57 ` Michael Ellerman
  1 sibling, 0 replies; 3+ messages in thread
From: Michael Ellerman @ 2022-08-03  8:57 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin

Nicholas Piggin <npiggin@gmail.com> writes:
> DD2.3 missed out on getting its feature flag bits.
>
> This meant when booting with dt-cpu-ftrs, CPU_FTR_P9_TM_HV_ASSIST is
> missing (unless the firmware contains it, which mine does not seem to).
> And when booting without, CPU_FTR_P9_TM_XER_SO_BUG is set.
>
> In practice this doesn't make any difference to pseries guests, only
> powernv.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/include/asm/cputable.h |  2 ++
>  arch/powerpc/kernel/cputable.c      | 22 ++++++++++++++++++++--
>  arch/powerpc/kernel/dt_cpu_ftrs.c   | 14 +++++---------
>  3 files changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
> index e85c849214a2..46bae9624784 100644
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -440,6 +440,8 @@ static inline void cpu_feature_keys_init(void) { }
>  #define CPU_FTRS_POWER9_DD2_2 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD2_1 | \
>  			       CPU_FTR_P9_TM_HV_ASSIST | \
>  			       CPU_FTR_P9_TM_XER_SO_BUG)
> +#define CPU_FTRS_POWER9_DD2_3 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD2_1 | \
> +			       CPU_FTR_P9_TM_HV_ASSIST)
>  #define CPU_FTRS_POWER10 (CPU_FTR_LWSYNC | \
>  	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\
>  	    CPU_FTR_MMCRA | CPU_FTR_SMT | \
> diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
> index ae0fdef0ac11..9ab97d1fd5a2 100644
> --- a/arch/powerpc/kernel/cputable.c
> +++ b/arch/powerpc/kernel/cputable.c
> @@ -487,11 +487,29 @@ static struct cpu_spec __initdata cpu_specs[] = {
>  		.machine_check_early	= __machine_check_early_realmode_p9,
>  		.platform		= "power9",
>  	},
> -	{	/* Power9 DD2.2 or later */
> +	{	/* Power9 DD 2.2 */
> +		.pvr_mask		= 0xffffefff,
> +		.pvr_value		= 0x004e0202,
> +		.cpu_name		= "POWER9 (raw)",
> +		.cpu_features		= CPU_FTRS_POWER9_DD2_2,
> +		.cpu_user_features	= COMMON_USER_POWER9,
> +		.cpu_user_features2	= COMMON_USER2_POWER9,
> +		.mmu_features		= MMU_FTRS_POWER9,
> +		.icache_bsize		= 128,
> +		.dcache_bsize		= 128,
> +		.num_pmcs		= 6,
> +		.pmc_type		= PPC_PMC_IBM,
> +		.oprofile_cpu_type	= "ppc64/power9",
> +		.cpu_setup		= __setup_cpu_power9,
> +		.cpu_restore		= __restore_cpu_power9,
> +		.machine_check_early	= __machine_check_early_realmode_p9,
> +		.platform		= "power9",
> +	},
> +	{	/* Power9 DD 2.3 or later */
>  		.pvr_mask		= 0xffff0000,
>  		.pvr_value		= 0x004e0000,
>  		.cpu_name		= "POWER9 (raw)",
> -		.cpu_features		= CPU_FTRS_POWER9_DD2_2,
> +		.cpu_features		= CPU_FTRS_POWER9_DD2_3,
>  		.cpu_user_features	= COMMON_USER_POWER9,
>  		.cpu_user_features2	= COMMON_USER2_POWER9,
>  		.mmu_features		= MMU_FTRS_POWER9,
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index 358aee7c2d79..af95f337e54b 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -764,18 +764,14 @@ static __init void cpufeatures_cpu_quirks(void)
>  	 * Not all quirks can be derived from the cpufeatures device tree.
>  	 */
>  	if ((version & 0xffffefff) == 0x004e0200) {
> -		/* DD2.0 has no feature flag */
> -		cur_cpu_spec->cpu_features |= CPU_FTR_P9_RADIX_PREFETCH_BUG;
> +		cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_0;
>  	} else if ((version & 0xffffefff) == 0x004e0201) {
> -		cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD2_1;
> -		cur_cpu_spec->cpu_features |= CPU_FTR_P9_RADIX_PREFETCH_BUG;
> +		cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_1;
>  	} else if ((version & 0xffffefff) == 0x004e0202) {
> -		cur_cpu_spec->cpu_features |= CPU_FTR_P9_TM_HV_ASSIST;
> -		cur_cpu_spec->cpu_features |= CPU_FTR_P9_TM_XER_SO_BUG;
> -		cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD2_1;
> +		cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_2;
>  	} else if ((version & 0xffff0000) == 0x004e0000) {
> -		/* DD2.1 and up have DD2_1 */
> -		cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD2_1;
> +		/* DD2.3 and up */
> +		cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_3;
>  	}

As we discussed on slack, this part is wrong.

Or'ing in the full CPU_FTRS_POWER9_DDx mask sets bits that may have been
turned off, or never enabled, by the device tree CPU features.

In particular it causes CPU_FTR_TM to be turned on (from
CPU_FTRS_POWER9), even though the machine may not have TM enabled.

We can probably just drop the dt_cpu_ftrs.c change, but if you don't
mind I'll get you to test that and resubmit the series.

cheers

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-03  8:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-26  3:17 [PATCH 1/2] powerpc/64s: POWER9 DD2.3 CPU feature flag fixes Nicholas Piggin
2021-07-26  3:17 ` [PATCH 2/2] powerpc/64s: Rename CPU_FTR_POWER9_DD2_1 to CPU_FTR_P9_STOP_FIXED Nicholas Piggin
2022-08-03  8:57 ` [PATCH 1/2] powerpc/64s: POWER9 DD2.3 CPU feature flag fixes Michael Ellerman

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).