| Message ID | 20240207035220.339726-1-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8b338061065b1871fc9ec53bd772321c15363123 |
| Headers | show |
| Series | [1/2] powerpc/pseries: Add a clear modifier to ibm,pa/pi-features parser | expand |
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>
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
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;
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(-)