[PATCH 1/2] powerpc/pseries: Add a clear modifier to ibm,pa/pi-features parser

[PATCH 1/2] powerpc/pseries: Add a clear modifier to ibm,pa/pi-features parser

[Nicholas Piggin]

When a new ibm,pa/pi-features bit is introduced that is intended to
apply to existing systems and features, it may have an "inverted"
meaning (i.e., bit clear => feature available; bit set => unavailable).
Depending on the nature of the feature, this may give the best
backward compatibility result where old firmware will continue to
have that bit clear and therefore the feature available.

The 'invert' modifier presumably was introduced for this type of
feature bit. However it invert will set the feature if the bit is
clear, which prevents it being used in the situation where an old
CPU lacks a feature that a new CPU has, then a new firmware comes
out to disable that feature on the new CPU if the bit is set.
Adding an 'invert' entry for that feature would incorrectly enable
it for the old CPU.

So add a 'clear' modifier that clears the feature if the bit is set,
but it does not set the feature if the bit is clear. The feature
is expected to be set in the cpu table.

This replaces the 'invert' modifier, which is unused since commit
7d4703455168 ("powerpc/feature: Remove CPU_FTR_NODSISRALIGN").

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/prom.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 0b5878c3125b..62f4a0229fae 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -151,6 +151,9 @@ static void __init move_device_tree(void)
  * pa-features property is missing, or a 1/0 to indicate if the feature
  * is supported/not supported.  Note that the bit numbers are
  * big-endian to match the definition in PAPR.
+ * Note: the 'clear' flag clears the feature if the bit is set in the
+ * the ibm,pa/pi-features property, it does not set the feature if the
+ * bit is clear.
  */
 struct ibm_feature {
 	unsigned long	cpu_features;	/* CPU_FTR_xxx bit */
@@ -159,7 +162,7 @@ struct ibm_feature {
 	unsigned int	cpu_user_ftrs2;	/* PPC_FEATURE2_xxx bit */
 	unsigned char	pabyte;		/* byte number in ibm,pa/pi-features */
 	unsigned char	pabit;		/* bit number (big-endian) */
-	unsigned char	invert;		/* if 1, pa bit set => clear feature */
+	unsigned char	clear;		/* if 1, pa bit set => clear feature */
 };
 
 static struct ibm_feature ibm_pa_features[] __initdata = {
@@ -220,12 +223,12 @@ static void __init scan_features(unsigned long node, const unsigned char *ftrs,
 		if (fp->pabyte >= ftrs[0])
 			continue;
 		bit = (ftrs[2 + fp->pabyte] >> (7 - fp->pabit)) & 1;
-		if (bit ^ fp->invert) {
+		if (bit && !fp->clear) {
 			cur_cpu_spec->cpu_features |= fp->cpu_features;
 			cur_cpu_spec->cpu_user_features |= fp->cpu_user_ftrs;
 			cur_cpu_spec->cpu_user_features2 |= fp->cpu_user_ftrs2;
 			cur_cpu_spec->mmu_features |= fp->mmu_features;
-		} else {
+		} else if (bit == fp->clear) {
 			cur_cpu_spec->cpu_features &= ~fp->cpu_features;
 			cur_cpu_spec->cpu_user_features &= ~fp->cpu_user_ftrs;
 			cur_cpu_spec->cpu_user_features2 &= ~fp->cpu_user_ftrs2;
-- 
2.42.0

[PATCH 2/2] powerpc/pseries: Set CPU_FTR_DBELL according to ibm,pi-features

[Nicholas Piggin]

PAPR will define a new ibm,pi-features bit which says that doorbells
should not be used even on architectures where they exist. This could be
because they are emulated and slower than using the interrupt controller
directly for IPIs.

Wire this bit into the pi-features parser to clear CPU_FTR_DBELL, and
ensure CPU_FTR_DBELL is not in CPU_FTRS_ALWAYS.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/cputable.h | 11 ++++++-----
 arch/powerpc/kernel/prom.c          |  1 +
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 8765d5158324..48471ca388dd 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -542,19 +542,20 @@ enum {
 #define CPU_FTRS_DT_CPU_BASE	(~0ul)
 #endif
 
+/* pseries may disable DBELL with ibm,pi-features */
 #ifdef CONFIG_CPU_LITTLE_ENDIAN
 #define CPU_FTRS_ALWAYS \
-	    (CPU_FTRS_POSSIBLE & ~CPU_FTR_HVMODE & CPU_FTRS_POWER7 & \
-	     CPU_FTRS_POWER8E & CPU_FTRS_POWER8 & CPU_FTRS_POWER9 & \
-	     CPU_FTRS_POWER9_DD2_1 & CPU_FTRS_POWER9_DD2_2 & \
+	    (CPU_FTRS_POSSIBLE & ~CPU_FTR_HVMODE & ~CPU_FTR_DBELL & \
+	     CPU_FTRS_POWER7 & CPU_FTRS_POWER8E & CPU_FTRS_POWER8 & \
+	     CPU_FTRS_POWER9 & CPU_FTRS_POWER9_DD2_1 & CPU_FTRS_POWER9_DD2_2 & \
 	     CPU_FTRS_POWER10 & CPU_FTRS_DT_CPU_BASE)
 #else
 #define CPU_FTRS_ALWAYS		\
 	    (CPU_FTRS_PPC970 & CPU_FTRS_POWER5 & \
 	     CPU_FTRS_POWER6 & CPU_FTRS_POWER7 & CPU_FTRS_CELL & \
 	     CPU_FTRS_PA6T & CPU_FTRS_POWER8 & CPU_FTRS_POWER8E & \
-	     ~CPU_FTR_HVMODE & CPU_FTRS_POSSIBLE & CPU_FTRS_POWER9 & \
-	     CPU_FTRS_POWER9_DD2_1 & CPU_FTRS_POWER9_DD2_2 & \
+	     ~CPU_FTR_HVMODE & ~CPU_FTR_DBELL & CPU_FTRS_POSSIBLE & \
+	     CPU_FTRS_POWER9 & CPU_FTRS_POWER9_DD2_1 & CPU_FTRS_POWER9_DD2_2 & \
 	     CPU_FTRS_POWER10 & CPU_FTRS_DT_CPU_BASE)
 #endif /* CONFIG_CPU_LITTLE_ENDIAN */
 #endif
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 62f4a0229fae..e3e8fe70475f 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -196,6 +196,7 @@ static struct ibm_feature ibm_pa_features[] __initdata = {
  */
 static struct ibm_feature ibm_pi_features[] __initdata = {
 	{ .pabyte = 0, .pabit = 3, .mmu_features  = MMU_FTR_NX_DSI },
+	{ .pabyte = 0, .pabit = 4, .cpu_features  = CPU_FTR_DBELL, .clear = 1 },
 };
 
 static void __init scan_features(unsigned long node, const unsigned char *ftrs,
-- 
2.42.0

Re: [PATCH 1/2] powerpc/pseries: Add a clear modifier to ibm,pa/pi-features parser

[Vaibhav Jain]

Nicholas Piggin <npiggin@gmail.com> writes:

> When a new ibm,pa/pi-features bit is introduced that is intended to
> apply to existing systems and features, it may have an "inverted"
> meaning (i.e., bit clear => feature available; bit set => unavailable).
> Depending on the nature of the feature, this may give the best
> backward compatibility result where old firmware will continue to
> have that bit clear and therefore the feature available.
>
> The 'invert' modifier presumably was introduced for this type of
> feature bit. However it invert will set the feature if the bit is
> clear, which prevents it being used in the situation where an old
> CPU lacks a feature that a new CPU has, then a new firmware comes
> out to disable that feature on the new CPU if the bit is set.
> Adding an 'invert' entry for that feature would incorrectly enable
> it for the old CPU.
>
> So add a 'clear' modifier that clears the feature if the bit is set,
> but it does not set the feature if the bit is clear. The feature
> is expected to be set in the cpu table.
>
> This replaces the 'invert' modifier, which is unused since commit
> 7d4703455168 ("powerpc/feature: Remove CPU_FTR_NODSISRALIGN").
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
<snip>

Tested this patch on a PP64-LE lpar with the patch[1] and seeing the
relevant pi-feature bit CPU_FTR_DBELL getting cleared.

[1]
https://lore.kernel.org/all/20240207035220.339726-2-npiggin@gmail.com

Hence,

Tested-by: Vaibhav Jain <vaibhav@linux.ibm.com>

-- 
Cheers
~ Vaibhav

Re: [PATCH 2/2] powerpc/pseries: Set CPU_FTR_DBELL according to ibm,pi-features

[Vaibhav Jain]

Nicholas Piggin <npiggin@gmail.com> writes:

> PAPR will define a new ibm,pi-features bit which says that doorbells
> should not be used even on architectures where they exist. This could be
> because they are emulated and slower than using the interrupt controller
> directly for IPIs.
>
> Wire this bit into the pi-features parser to clear CPU_FTR_DBELL, and
> ensure CPU_FTR_DBELL is not in CPU_FTRS_ALWAYS.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Tested this patch on a PP64-LE lpar with the patch[1] and seeing the
relevant pi-feature bit CPU_FTR_DBELL in `cur_cpu_spec->cpu_features`
getting cleared.


[1]
https://lore.kernel.org/all/20240207035220.339726-1-npiggin@gmail.com

Hence,
Tested-by: Vaibhav Jain <vaibhav@linux.ibm.com>

-- 
Cheers
~ Vaibhav

Re: [PATCH 1/2] powerpc/pseries: Add a clear modifier to ibm,pa/pi-features parser

[Michael Ellerman]

On Wed, 07 Feb 2024 13:52:18 +1000, Nicholas Piggin wrote:
> When a new ibm,pa/pi-features bit is introduced that is intended to
> apply to existing systems and features, it may have an "inverted"
> meaning (i.e., bit clear => feature available; bit set => unavailable).
> Depending on the nature of the feature, this may give the best
> backward compatibility result where old firmware will continue to
> have that bit clear and therefore the feature available.
> 
> [...]

Applied to powerpc/next.

[1/2] powerpc/pseries: Add a clear modifier to ibm,pa/pi-features parser
      https://git.kernel.org/powerpc/c/8b338061065b1871fc9ec53bd772321c15363123
[2/2] powerpc/pseries: Set CPU_FTR_DBELL according to ibm,pi-features
      https://git.kernel.org/powerpc/c/6e9de2054eb417d6e05561b19c825c29b424b475

cheers