Re: [PATCH] powerpc/64s: Clear PCR on boot

4 messages, 3 authors, 2018-05-28 · open the first message on its own page

Re: [PATCH] powerpc/64s: Clear PCR on boot

From: Guenter Roeck <linux@roeck-us.net>
Date: 2018-05-25 13:33:13

On Fri, May 18, 2018 at 11:37:42AM +1000, Michael Neuling wrote:

Clear the PCR (Processor Compatibility Register) on boot to ensure we
are not running in a compatibility mode.

We've seen this cause problems when a crash (and kdump) occurs while
running compat mode guests. The kdump kernel then runs with the PCR
set and causes problems. The symptom in the kdump kernel (also seen in
petitboot after fast-reboot) is early userspace programs taking
sigills on newer instructions (seen in libc).

Hi folks,

this patch causes qemu to bail out with

Trying to write privileged spr 338 (0x152) at c000000000033454

when running it with "-M powernv -cpu POWER8" and powernv_defconfig.

Can you confirm that this is a bug in qemu ?

Thanks,
Guenter

quoted hunk

Signed-off-by: Michael Neuling <redacted>
Cc: stable@vger.kernel.org
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/cpu_setup_power.S | 6 ++++++
 arch/powerpc/kernel/dt_cpu_ftrs.c     | 1 +
 2 files changed, 7 insertions(+)

diff --git a/arch/powerpc/kernel/cpu_setup_power.S b/arch/powerpc/kernel/cpu_setup_power.S
index 3f30c994e931..458b928dbd84 100644
--- a/arch/powerpc/kernel/cpu_setup_power.S
+++ b/arch/powerpc/kernel/cpu_setup_power.S

@@ -28,6 +28,7 @@ _GLOBAL(__setup_cpu_power7)
 	beqlr
 	li	r0,0
 	mtspr	SPRN_LPID,r0
+	mtspr	SPRN_PCR,r0
 	mfspr	r3,SPRN_LPCR
 	li	r4,(LPCR_LPES1 >> LPCR_LPES_SH)
 	bl	__init_LPCR_ISA206

@@ -41,6 +42,7 @@ _GLOBAL(__restore_cpu_power7)
 	beqlr
 	li	r0,0
 	mtspr	SPRN_LPID,r0
+	mtspr	SPRN_PCR,r0
 	mfspr	r3,SPRN_LPCR
 	li	r4,(LPCR_LPES1 >> LPCR_LPES_SH)
 	bl	__init_LPCR_ISA206

@@ -57,6 +59,7 @@ _GLOBAL(__setup_cpu_power8)
 	beqlr
 	li	r0,0
 	mtspr	SPRN_LPID,r0
+	mtspr	SPRN_PCR,r0
 	mfspr	r3,SPRN_LPCR
 	ori	r3, r3, LPCR_PECEDH
 	li	r4,0 /* LPES = 0 */

@@ -78,6 +81,7 @@ _GLOBAL(__restore_cpu_power8)
 	beqlr
 	li	r0,0
 	mtspr	SPRN_LPID,r0
+	mtspr	SPRN_PCR,r0
 	mfspr   r3,SPRN_LPCR
 	ori	r3, r3, LPCR_PECEDH
 	li	r4,0 /* LPES = 0 */

@@ -99,6 +103,7 @@ _GLOBAL(__setup_cpu_power9)
 	mtspr	SPRN_PSSCR,r0
 	mtspr	SPRN_LPID,r0
 	mtspr	SPRN_PID,r0
+	mtspr	SPRN_PCR,r0
 	mfspr	r3,SPRN_LPCR
 	LOAD_REG_IMMEDIATE(r4, LPCR_PECEDH | LPCR_PECE_HVEE | LPCR_HVICE  | LPCR_HEIC)
 	or	r3, r3, r4

@@ -123,6 +128,7 @@ _GLOBAL(__restore_cpu_power9)
 	mtspr	SPRN_PSSCR,r0
 	mtspr	SPRN_LPID,r0
 	mtspr	SPRN_PID,r0
+	mtspr	SPRN_PCR,r0
 	mfspr   r3,SPRN_LPCR
 	LOAD_REG_IMMEDIATE(r4, LPCR_PECEDH | LPCR_PECE_HVEE | LPCR_HVICE | LPCR_HEIC)
 	or	r3, r3, r4

diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 8ab51f6ca03a..c904477abaf3 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c

@@ -101,6 +101,7 @@ static void __restore_cpu_cpufeatures(void)
 	if (hv_mode) {
 		mtspr(SPRN_LPID, 0);
 		mtspr(SPRN_HFSCR, system_registers.hfscr);
+		mtspr(SPRN_PCR, 0);
 	}
 	mtspr(SPRN_FSCR, system_registers.fscr);

-- 
2.7.4

Re: [PATCH] powerpc/64s: Clear PCR on boot

From: Michael Ellerman <hidden>
Date: 2018-05-27 02:22:33


On 25 May 2018 11:33:08 pm AEST, Guenter Roeck <linux@roeck-us=2Enet> wrot=
e:

On Fri, May 18, 2018 at 11:37:42AM +1000, Michael Neuling wrote:

quoted

Clear the PCR (Processor Compatibility Register) on boot to ensure we
are not running in a compatibility mode=2E
=20
We've seen this cause problems when a crash (and kdump) occurs while
running compat mode guests=2E The kdump kernel then runs with the PCR
set and causes problems=2E The symptom in the kdump kernel (also seen

in

quoted

petitboot after fast-reboot) is early userspace programs taking
sigills on newer instructions (seen in libc)=2E
=20

Hi folks,

this patch causes qemu to bail out with

Trying to write privileged spr 338 (0x152) at c000000000033454

when running it with "-M powernv -cpu POWER8" and powernv_defconfig=2E

Can you confirm that this is a bug in qemu ?

Hi Guenter,

Thanks for the report=2E As far as I'm concerned this is a qemu bug, that =
register should exist on a bare metal Power8=2E

Will try and get you a qemu patch to fix it=2E

cheers

--=20
Sent from my Android phone with K-9 Mail=2E Please excuse my brevity=2E

Re: [PATCH] powerpc/64s: Clear PCR on boot

From: Guenter Roeck <linux@roeck-us.net>
Date: 2018-05-27 03:45:08

Hi Michael,

On Sun, May 27, 2018 at 12:22:23PM +1000, Michael Ellerman wrote:


On 25 May 2018 11:33:08 pm AEST, Guenter Roeck [off-list ref] wrote:

quoted

On Fri, May 18, 2018 at 11:37:42AM +1000, Michael Neuling wrote:

quoted

Clear the PCR (Processor Compatibility Register) on boot to ensure we
are not running in a compatibility mode.

We've seen this cause problems when a crash (and kdump) occurs while
running compat mode guests. The kdump kernel then runs with the PCR
set and causes problems. The symptom in the kdump kernel (also seen

in

quoted

petitboot after fast-reboot) is early userspace programs taking
sigills on newer instructions (seen in libc).

Hi folks,

this patch causes qemu to bail out with

Trying to write privileged spr 338 (0x152) at c000000000033454

when running it with "-M powernv -cpu POWER8" and powernv_defconfig.

Can you confirm that this is a bug in qemu ?

Hi Guenter,

Thanks for the report. As far as I'm concerned this is a qemu bug, that register should exist on a bare metal Power8.

Will try and get you a qemu patch to fix it.

I already have a patch, or at least one that does the trick for me.
Getting qemu patched was not the problem. I just want to be sure that
the problem is indeed a qemu problem.

Thanks,
Guenter

---

From 1617bac264b4c49d817b6947611affa9b73318f6 Mon Sep 17 00:00:00 2001

From: Guenter Roeck <linux@roeck-us.net>
Date: Fri, 25 May 2018 06:38:40 -0700
Subject: [PATCH] PowerPC: Permit privileged access to SPR_PCR for POWER7+

Without this access, Linux mainline bails out.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 target/ppc/translate_init.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 391b94b..3b09c49 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c

@@ -7953,11 +7953,12 @@ static void gen_spr_power6_common(CPUPPCState *env)
 #endif
     /*
      * Register PCR to report POWERPC_EXCP_PRIV_REG instead of
-     * POWERPC_EXCP_INVAL_SPR.
+     * POWERPC_EXCP_INVAL_SPR in userspace. Permit privileged
+     * access.
      */
     spr_register(env, SPR_PCR, "PCR",
                  SPR_NOACCESS, SPR_NOACCESS,
-                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
                  0x00000000);
 }

-- 
2.7.4

Re: [PATCH] powerpc/64s: Clear PCR on boot

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2018-05-28 13:29:43

On Sat, 2018-05-26 at 20:45 -0700, Guenter Roeck wrote:

I already have a patch, or at least one that does the trick for me.
Getting qemu patched was not the problem. I just want to be sure that
the problem is indeed a qemu problem.

Hey Guenter !

It's not quite the right patch though. The PCR is a hypervisor
priviledged register, your patch makes it supervisor accessible.

I don't have all my stuff at hand to provide a "proper" or tested patch
but it should look like

    spr_register_hv(env, SPR_PCR, "PCR",
                    SPR_NOACCESS, SPR_NOACCESS,
                    SPR_NOACCESS, SPR_NOACCESS,
                    &spr_read_generic, &spr_write_generic, 0);

Additionally the TCG ppc instruction decoder should be made to check
the PCR for varous instructions (that or use a specific write callback
that affects the CPU flags) but that's less urgent.

Cheers,
Ben.

quoted hunk

Thanks,
Guenter

---

quoted

From 1617bac264b4c49d817b6947611affa9b73318f6 Mon Sep 17 00:00:00 2001

From: Guenter Roeck <linux@roeck-us.net>
Date: Fri, 25 May 2018 06:38:40 -0700
Subject: [PATCH] PowerPC: Permit privileged access to SPR_PCR for POWER7+

Without this access, Linux mainline bails out.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 target/ppc/translate_init.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 391b94b..3b09c49 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c

@@ -7953,11 +7953,12 @@ static void gen_spr_power6_common(CPUPPCState *env)
 #endif
     /*
      * Register PCR to report POWERPC_EXCP_PRIV_REG instead of
-     * POWERPC_EXCP_INVAL_SPR.
+     * POWERPC_EXCP_INVAL_SPR in userspace. Permit privileged
+     * access.
      */
     spr_register(env, SPR_PCR, "PCR",
                  SPR_NOACCESS, SPR_NOACCESS,
-                 SPR_NOACCESS, SPR_NOACCESS,
+                 &spr_read_generic, &spr_write_generic,
                  0x00000000);
 }

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help