[PATCH 1/3] powerpc: scan_features() updates incorrect bits
The real LE feature entry in the ibm_pa_feature struct has the
wrong number of elements. Instead of checking for byte 5, bit 0,
we check for byte 0, bit 0, and we also incorrectly update cpu user
feature bit 5.
Fixes: 44ae3ab3358e ("powerpc: Free up some CPU feature bits by moving out
MMU-related features")
Signed-off-by: Anton Blanchard
Cc: sta...@vger.kernel.org
---
arch/powerpc/kernel/prom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 7030b03..9a3a7c6 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -158,7 +158,7 @@ static struct ibm_pa_feature {
{CPU_FTR_NOEXECUTE, 0, 0, 0, 6, 0},
{CPU_FTR_NODSISRALIGN, 0, 0,1, 1, 1},
{0, MMU_FTR_CI_LARGE_PAGE, 0, 1, 2, 0},
- {CPU_FTR_REAL_LE, PPC_FEATURE_TRUE_LE, 5, 0, 0},
+ {CPU_FTR_REAL_LE, PPC_FEATURE_TRUE_LE, 0, 5, 0, 0},
/*
* If the kernel doesn't support TM (ie.
CONFIG_PPC_TRANSACTIONAL_MEM=n),
* we don't want to turn on CPU_FTR_TM here, so we use CPU_FTR_TM_COMP
--
2.7.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [1/3] powerpc: scan_features() updates incorrect bits
On Fri, 2016-15-04 at 02:06:13 UTC, Unknown sender due to SPF wrote:
> The real LE feature entry in the ibm_pa_feature struct has the
> wrong number of elements. Instead of checking for byte 5, bit 0,
> we check for byte 0, bit 0, and we also incorrectly update cpu user
> feature bit 5.
>
> Fixes: 44ae3ab3358e ("powerpc: Free up some CPU feature bits by moving out
> MMU-related features")
So pulling that apart a bit.
> - {CPU_FTR_REAL_LE, PPC_FEATURE_TRUE_LE, 5, 0, 0}, (invert 0)
^^^ ^^
cpu feat mmu feat user feat byte bit
By checking byte 0 bit 0 (IBM numbering), means we're looking at the "Memory
Management Unit (MMU)" feature - ie. does the CPU have an MMU.
As you'd expect that bit is set on most platforms we care about. I see it set on
a P6 & P7 here running PowerVM, as well as by recent Qemu on P8.
Which means on all those systems we'll be enabling those bits.
As far as the cpu feature bits:
#define CPU_FTR_REAL_LE 0x0040
On a guest running on P8 on older qemu which has no ibm,pa-features property I
see:
cpu_features = 0x17fc7aec18500249
^
So it's set, but not by this logic, because it's already in cpu_features for P8.
On Power6, which does have the ibm,pa-feature property with bit 0,0 set:
cpu_features = 0x090d1ae018500049
^
Also set, from the property, but it's also in the Power6 mask by default. So all
we've lost there is the ability to turn it off.
For the MMU feature, we're setting:
#define PPC_FEATURE_TRUE_LE 0x0002
Which matches:
#define MMU_FTR_TYPE_8xx 0x0002
And on the Power6 I do indeed see:
mmu_features = 0x7c03
Which says I have MMU_FTR_HPTE_TABLE and MMU_FTR_TYPE_8xx.
Luckily it looks like the only place that looks at MMU_FTR_TYPE_8xx is in Book3E
code, so will never be affected by this bug.
Finally the user feature, we're setting 5, ie. 0x4 | 0x1, which is:
#define PPC_FEATURE_PPC_LE 0x0001
And nothing else, 0x4 is free.
On the Power6, I can see it in /proc/pid/auxv:
0060 00 00 00 00 00 00 00 10 00 00 00 00 dc 00 74 47 |..tG|
^
0x4 | 0x2 | 0x1
LD_SHOW_AUXV=1 doesn't show it, but that's just because my glibc is old.
Net result:
- on P6 & P7 (& P8 with newer Qemu) we can't disable CPU_FTR_REAL_LE - but
luckily we've never wanted to.
- we're stuffing up mmu_features but it doesn't matter
- we're advertising PPC_LE when we shouldn't be, but *probably* nothing cares.
- we're advertising 0x4 in HWCAP which is undefined, but *almost certainly*
nothing cares.
So that could have been a doozy but turns out not actually that bad. Phew :)
cheers
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -158,7 +158,7 @@ static struct ibm_pa_feature {
> {CPU_FTR_NOEXECUTE, 0, 0, 0, 6, 0},
> {CPU_FTR_NODSISRALIGN, 0, 0,1, 1, 1},
> {0, MMU_FTR_CI_LARGE_PAGE, 0, 1, 2, 0},
> - {CPU_FTR_REAL_LE, PPC_FEATURE_TRUE_LE, 5, 0, 0},
> + {CPU_FTR_REAL_LE, PPC_FEATURE_TRUE_LE, 0, 5, 0, 0},
> /*
>* If the kernel doesn't support TM (ie.
> CONFIG_PPC_TRANSACTIONAL_MEM=n),
>* we don't want to turn on CPU_FTR_TM here, so we use CPU_FTR_TM_COMP
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [1/3] powerpc: scan_features() updates incorrect bits
On Fri, 2016-15-04 at 02:06:13 UTC, Unknown sender due to SPF wrote:
> The real LE feature entry in the ibm_pa_feature struct has the
> wrong number of elements. Instead of checking for byte 5, bit 0,
> we check for byte 0, bit 0, and we also incorrectly update cpu user
> feature bit 5.
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 7030b03..9a3a7c6 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -158,7 +158,7 @@ static struct ibm_pa_feature {
> {CPU_FTR_NOEXECUTE, 0, 0, 0, 6, 0},
> {CPU_FTR_NODSISRALIGN, 0, 0,1, 1, 1},
> {0, MMU_FTR_CI_LARGE_PAGE, 0, 1, 2, 0},
> - {CPU_FTR_REAL_LE, PPC_FEATURE_TRUE_LE, 5, 0, 0},
> + {CPU_FTR_REAL_LE, PPC_FEATURE_TRUE_LE, 0, 5, 0, 0},
This should be:
> + {CPU_FTR_REAL_LE, 0, PPC_FEATURE_TRUE_LE, 5, 0, 0},
Because the struct layout is:
static struct ibm_pa_feature {
unsigned long cpu_features; /* CPU_FTR_xxx bit */
unsigned long mmu_features; /* MMU_FTR_xxx bit */
unsigned intcpu_user_ftrs; /* PPC_FEATURE_xxx bit */
unsigned intcpu_user_ftrs2; /* PPC_FEATURE2_xxx bit */
unsigned char pabyte; /* byte number in ibm,pa-features */
unsigned char pabit; /* bit number (big-endian) */
unsigned char invert; /* if 1, pa bit set => clear feature */
}
I'll fix it up locally.
cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [1/3] powerpc: scan_features() updates incorrect bits
On Sat, 2016-04-16 at 00:27 +1000, Michael Ellerman wrote: > On Fri, 2016-15-04 at 02:06:13 UTC, Unknown sender due to SPF wrote: > > The real LE feature entry in the ibm_pa_feature struct has the > > wrong number of elements. Instead of checking for byte 5, bit 0, > > we check for byte 0, bit 0, and we also incorrectly update cpu user > > feature bit 5. > > Finally the user feature, we're setting 5, ie. 0x4 | 0x1, which is: > > #define PPC_FEATURE_PPC_LE0x0001 > > And nothing else, 0x4 is free. Bt, we should reserve 0x4 for the foreseeable future. Because it is erroneously set on older kernels. cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 1/3] powerpc: scan_features() updates incorrect bits for REAL_LE
From: Anton Blanchard
The REAL_LE feature entry in the ibm_pa_feature struct is missing an MMU
feature value, meaning all the remaining elements initialise the wrong
values.
This means instead of checking for byte 5, bit 0, we check for byte 0,
bit 0, and then we incorrectly set the CPU feature bit as well as MMU
feature bit 1 and CPU user feature bits 0 and 2 (5).
Checking byte 0 bit 0 (IBM numbering), means we're looking at the
"Memory Management Unit (MMU)" feature - ie. does the CPU have an MMU.
In practice that bit is set on all platforms which have the property.
This means we set CPU_FTR_REAL_LE always. In practice that seems not to
matter because all the modern cpus which have this property also
implement REAL_LE, and we've never needed to disable it.
We're also incorrectly setting MMU feature bit 1, which is:
#define MMU_FTR_TYPE_8xx 0x0002
Luckily the only place that looks for MMU_FTR_TYPE_8xx is in Book3E
code, which can't run on the same cpus as scan_features(). So this also
doesn't matter in practice.
Finally in the CPU user feature mask, we're setting bits 0 and 2. Bit 2
is not currently used, and bit 0 is:
#define PPC_FEATURE_PPC_LE0x0001
Which says the CPU supports the old style "PPC Little Endian" mode.
Again this should be harmless in practice as no 64-bit CPUs implement
that mode.
Fix the code by adding the missing initialisation of the MMU feature.
Also add a comment marking CPU user feature bit 2 (0x4) as reserved. It
would be unsafe to start using it as old kernels incorrectly set it.
Fixes: 44ae3ab3358e ("powerpc: Free up some CPU feature bits by moving out
MMU-related features")
Signed-off-by: Anton Blanchard
Cc: sta...@vger.kernel.org
[mpe: Flesh out changelog, add comment reserving 0x4]
Signed-off-by: Michael Ellerman
---
arch/powerpc/include/uapi/asm/cputable.h | 1 +
arch/powerpc/kernel/prom.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/uapi/asm/cputable.h
b/arch/powerpc/include/uapi/asm/cputable.h
index 8dde19962a5b..f63c96cd3608 100644
--- a/arch/powerpc/include/uapi/asm/cputable.h
+++ b/arch/powerpc/include/uapi/asm/cputable.h
@@ -31,6 +31,7 @@
#define PPC_FEATURE_PSERIES_PERFMON_COMPAT \
0x0040
+/* Reserved - do not use 0x0004 */
#define PPC_FEATURE_TRUE_LE0x0002
#define PPC_FEATURE_PPC_LE 0x0001
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 7030b035905d..080c96b44a7f 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -158,7 +158,7 @@ static struct ibm_pa_feature {
{CPU_FTR_NOEXECUTE, 0, 0, 0, 6, 0},
{CPU_FTR_NODSISRALIGN, 0, 0,1, 1, 1},
{0, MMU_FTR_CI_LARGE_PAGE, 0, 1, 2, 0},
- {CPU_FTR_REAL_LE, PPC_FEATURE_TRUE_LE, 5, 0, 0},
+ {CPU_FTR_REAL_LE, 0, PPC_FEATURE_TRUE_LE, 5, 0, 0},
/*
* If the kernel doesn't support TM (ie.
CONFIG_PPC_TRANSACTIONAL_MEM=n),
* we don't want to turn on CPU_FTR_TM here, so we use CPU_FTR_TM_COMP
--
2.5.0
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [v2, 1/3] powerpc: scan_features() updates incorrect bits for REAL_LE
On Mon, 2016-18-04 at 10:36:07 UTC, Michael Ellerman wrote:
> From: Anton Blanchard
>
> The REAL_LE feature entry in the ibm_pa_feature struct is missing an MMU
> feature value, meaning all the remaining elements initialise the wrong
> values.
...
>
> Fix the code by adding the missing initialisation of the MMU feature.
>
> Also add a comment marking CPU user feature bit 2 (0x4) as reserved. It
> would be unsafe to start using it as old kernels incorrectly set it.
>
> Fixes: 44ae3ab3358e ("powerpc: Free up some CPU feature bits by moving out
> MMU-related features")
> Signed-off-by: Anton Blanchard
> Cc: sta...@vger.kernel.org
> [mpe: Flesh out changelog, add comment reserving 0x4]
> Signed-off-by: Michael Ellerman
Applied to powerpc fixes.
https://git.kernel.org/powerpc/c/6997e57d693b07289694239e52
cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
