Skip to contentSkip to conversation

powerpc/powernv: Fix opal entry/exit MSR_RI coverage

PATCH In review Nicholas Piggin 2 participants 5 messages Mar 30, 2017 – Apr 24, 2017 PowerPC

lore
Share & tools
lore

Nicholas Piggin

author reviewer
#0 lore raw

PATCH 1/1 powerpc/powernv: Fix opal entry/exit MSR_RI coverage

There are some windows in opal entry/exit that can not recover from a re-entrant interrupt (e.g., machine check) due to using SRR registers, but they currently do not have MSR_RI clear.

These were found by machine check injection coverage tests using the powerpc system simulator (Mambo).

Signed-off-by Nicholas Piggin <[email protected]>
---
 arch/powerpc/boot/ppc_asm.h                    | 12 +++++++-----
 arch/powerpc/include/asm/ppc_asm.h             | 10 ++++++----
 arch/powerpc/platforms/powernv/opal-wrappers.S | 15 +++++++++------
 3 files changed, 22 insertions(+), 15 deletions(-)
arch/powerpc/boot/ppc_asm.h +7 -5
index b03373d8b386..68e388ee94fe 100644
@@ -67,13 +67,15 @@
 #define MSR_LE		0x0000000000000001
 
 #define FIXUP_ENDIAN						   \
-	tdi   0, 0, 0x48; /* Reverse endian of b . + 8		*/ \
-	b     $+36;	  /* Skip trampoline if endian is good	*/ \
-	.long 0x05009f42; /* bcl 20,31,$+4			*/ \
-	.long 0xa602487d; /* mflr r10				*/ \
-	.long 0x1c004a39; /* addi r10,r10,28			*/ \
+	tdi   0,0,0x48;	  /* Reverse endian of b . + 8		*/ \
+	b     $+44;	  /* Skip trampoline if endian is good	*/ \
 	.long 0xa600607d; /* mfmsr r11				*/ \
 	.long 0x01006b69; /* xori r11,r11,1			*/ \
+	.long 0x00004039; /* li r10,0				*/ \
+	.long 0x6401417d; /* mtmsrd r10,1			*/ \
+	.long 0x05009f42; /* bcl 20,31,$+4			*/ \
+	.long 0xa602487d; /* mflr r10				*/ \
+	.long 0x14004a39; /* addi r10,r10,20			*/ \
 	.long 0xa6035a7d; /* mtsrr0 r10				*/ \
 	.long 0xa6037b7d; /* mtsrr1 r11				*/ \
 	.long 0x2400004c  /* rfid				*/
arch/powerpc/include/asm/ppc_asm.h +6 -4
index 359c44341761..9d47cba1da3f 100644
@@ -770,12 +770,14 @@ END_FTR_SECTION_IFCLR(CPU_FTR_601)
 #else
 #define FIXUP_ENDIAN						   \
 	tdi   0,0,0x48;	  /* Reverse endian of b . + 8		*/ \
-	b     $+36;	  /* Skip trampoline if endian is good	*/ \
-	.long 0x05009f42; /* bcl 20,31,$+4			*/ \
-	.long 0xa602487d; /* mflr r10				*/ \
-	.long 0x1c004a39; /* addi r10,r10,28			*/ \
+	b     $+44;	  /* Skip trampoline if endian is good	*/ \
 	.long 0xa600607d; /* mfmsr r11				*/ \
 	.long 0x01006b69; /* xori r11,r11,1			*/ \
+	.long 0x00004039; /* li r10,0				*/ \
+	.long 0x6401417d; /* mtmsrd r10,1			*/ \
+	.long 0x05009f42; /* bcl 20,31,$+4			*/ \
+	.long 0xa602487d; /* mflr r10				*/ \
+	.long 0x14004a39; /* addi r10,r10,20			*/ \
 	.long 0xa6035a7d; /* mtsrr0 r10				*/ \
 	.long 0xa6037b7d; /* mtsrr1 r11				*/ \
 	.long 0x2400004c  /* rfid				*/
arch/powerpc/platforms/powernv/opal-wrappers.S +9 -6
index da8a0f7a035c..458109b1c291 100644
@@ -67,15 +67,14 @@ END_FTR_SECTION(0, 1);						\
 	OPAL_BRANCH(opal_tracepoint_entry) \
 	mfcr	r11;			\
 	stw	r11,8(r1);		\
-	li	r11,0;			\
-	ori	r11,r11,MSR_EE;		\
 	std	r12,PACASAVEDMSR(r13);	\
-	andc	r12,r12,r11;		\
-	mtmsrd	r12,1;			\
 	LOAD_REG_ADDR(r11,opal_return);	\
 	mtlr	r11;			\
-	li	r11,MSR_DR|MSR_IR|MSR_LE;\
+	lis	r11,MSR_EE@h;		\
+	ori	r11,r11,MSR_DR|MSR_IR|MSR_LE;\
 	andc	r12,r12,r11;		\
+	li	r11,0;			\
+	mtmsrd	r11,1; /* EE=0 RI=0 */	\
 	mtspr	SPRN_HSRR1,r12;		\
 	LOAD_REG_ADDR(r11,opal);	\
 	ld	r12,8(r11);		\

@@ -95,9 +94,11 @@ opal_return:
 	lwz	r4,8(r1);
 	ld	r5,PPC_LR_STKOFF(r1);
 	ld	r6,PACASAVEDMSR(r13);
+	mtcr	r4;
+	li	r4,0;
+	mtmsrd	r4,1; /* EE=0 RI=0 */
 	mtspr	SPRN_SRR0,r5;
 	mtspr	SPRN_SRR1,r6;
-	mtcr	r4;
 	rfid
 
 opal_real_call:

@@ -108,6 +109,8 @@ opal_real_call:
 	mtlr	r11
 	li	r11,MSR_LE
 	andc	r12,r12,r11
+	li	r11,0;
+	mtmsrd	r11,1; /* EE=0 RI=0 */
 	mtspr	SPRN_HSRR1,r12
 	LOAD_REG_ADDR(r11,opal)
 	ld	r12,8(r11)
Signature · 1 line
2.11.0
#1 lore raw
Quoted 6 lines · Nicholas Piggin

There are some windows in opal entry/exit that can not recover from a re-entrant interrupt (e.g., machine check) due to using SRR registers, but they currently do not have MSR_RI clear.

These were found by machine check injection coverage tests using the powerpc system simulator (Mambo).

So you make us enter/exit OPAL with RI off with your patch.

OPAL currently won't re-enable it (which is a problem... we need to deal with MCEs happening while in OPAL, especially the ones with the special recovery table).

So if we do that we need to make OPAL turn it back on. In that case I would suggest making OPAL also turn it back off, thus avoiding touching FIXUP_ENDIAN...

Ben.

Quoted 101 lines
Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/boot/ppc_asm.h                    | 12 +++++++----- arch/powerpc/include/asm/ppc_asm.h             | 10 ++++++---- arch/powerpc/platforms/powernv/opal-wrappers.S | 15 +++++++++------

 3 files changed, 22 insertions(+), 15 deletions(-)
arch/powerpc/boot/ppc_asm.h +0 -0
index b03373d8b386..68e388ee94fe 100644
@@ -67,13 +67,15 @@
Quoted 1 line

 #define MSR_LE 0x0000000000000001

Quoted 18 lines

 #define FIXUP_ENDIAN    \

Quoted 17 lines
arch/powerpc/boot/ppc_asm.h +0 -1
-	tdi   0, 0, 0x48; /* Reverse endian of b . + 8		*/ \
Quoted 1 line
(quoted diff) +0 -1
-	b     $+36;	  /* Skip trampoline if endian is good	*/ \
arch/powerpc/boot/ppc_asm.h +0 -3
-	.long 0x05009f42; /* bcl 20,31,$+4			*/ \
-	.long 0xa602487d; /* mflr r10				*/ \
-	.long 0x1c004a39; /* addi r10,r10,28			*/ \

arch/powerpc/boot/ppc_asm.h line 71

Quoted 2 lines
arch/powerpc/boot/ppc_asm.h +2 -0
+	tdi   0,0,0x48;	  /* Reverse endian of b . + 8		*/ \
+	b     $+44;	  /* Skip trampoline if endian is good	*/ \

  .long 0xa600607d; /* mfmsr r11 */ \
  .long 0x01006b69; /* xori r11,r11,1 */ \

arch/powerpc/boot/ppc_asm.h +5 -0
+	.long 0x00004039; /* li r10,0				*/ \
+	.long 0x6401417d; /* mtmsrd r10,1			*/ \
+	.long 0x05009f42; /* bcl 20,31,$+4			*/ \
+	.long 0xa602487d; /* mflr r10				*/ \
+	.long 0x14004a39; /* addi r10,r10,20			*/ \

  .long 0xa6035a7d; /* mtsrr0 r10 */ \
  .long 0xa6037b7d; /* mtsrr1 r11 */ \
  .long 0x2400004c  /* rfid */

arch/powerpc/include/asm/ppc_asm.h +0 -0
index 359c44341761..9d47cba1da3f 100644
@@ -770,12 +770,14 @@ END_FTR_SECTION_IFCLR(CPU_FTR_601)
  #else
 >  #define FIXUP_ENDIAN						   \
 > > >  	tdi   0,0,0x48;	  /* Reverse endian of b . + 8		*/ \
 > > > -	b     $+36;	  /* Skip trampoline if endian is good	*/ \
 > > -	.long 0x05009f42; /* bcl 20,31,$+4			*/ \
 > > -	.long 0xa602487d; /* mflr r10				*/ \
 > > -	.long 0x1c004a39; /* addi r10,r10,28			*/ \
 > > > +	b     $+44;	  /* Skip trampoline if endian is good	*/ \
 > >  	.long 0xa600607d; /* mfmsr r11				*/ \
 > >  	.long 0x01006b69; /* xori r11,r11,1			*/ \
 > > +	.long 0x00004039; /* li r10,0				*/ \
 > > +	.long 0x6401417d; /* mtmsrd r10,1			*/ \
Quoted 6 lines
Quoted 6 lines
arch/powerpc/boot/ppc_asm.h +3 -0
+	.long 0x05009f42; /* bcl 20,31,$+4			*/ \
+	.long 0xa602487d; /* mflr r10				*/ \
+	.long 0x14004a39; /* addi r10,r10,20			*/ \

  .long 0xa6035a7d; /* mtsrr0 r10 */ \
  .long 0xa6037b7d; /* mtsrr1 r11 */ \
  .long 0x2400004c  /* rfid */

arch/powerpc/platforms/powernv/opal-wrappers.S +0 -0
index da8a0f7a035c..458109b1c291 100644
Quoted 20 lines
(quoted diff) +0 -0
@@ -67,15 +67,14 @@ END_FTR_SECTION(0, 1);						\
  	OPAL_BRANCH(opal_tracepoint_entry) \
 > >  	mfcr	r11;			\
 > >  	stw	r11,8(r1);		\
 > > -	li	r11,0;			\
 > > -	ori	r11,r11,MSR_EE;		\
 > >  	std	r12,PACASAVEDMSR(r13);	\
 > > -	andc	r12,r12,r11;		\
 > > -	mtmsrd	r12,1;			\
 >  	LOAD_REG_ADDR(r11,opal_return);	\
 > >  	mtlr	r11;			\
 > -	li	r11,MSR_DR|MSR_IR|MSR_LE;\
 > > > +	lis	r11,MSR_EE@h;		\
 > +	ori	r11,r11,MSR_DR|MSR_IR|MSR_LE;\
 > >  	andc	r12,r12,r11;		\
Quoted 5 lines
Quoted 3 lines
arch/powerpc/platforms/powernv/opal-wrappers.S +2 -0
+	li	r11,0;			\
+	mtmsrd	r11,1; /* EE=0 RI=0 */	\

  mtspr SPRN_HSRR1,r12; \

  LOAD_REG_ADDR(r11,opal); \

Quoted 1 line

  ld r12,8(r11); \

arch/powerpc/platforms/powernv/opal-wrappers.S +0 -0
@@ -95,9 +94,11 @@ opal_return:
 > >  	lwz	r4,8(r1);
 > >  	ld	r5,PPC_LR_STKOFF(r1);
 > >  	ld	r6,PACASAVEDMSR(r13);
 > > +	mtcr	r4;
 > > +	li	r4,0;
Quoted 5 lines

arch/powerpc/platforms/powernv/opal-wrappers.S opal_return: · line 100

Quoted 4 lines
arch/powerpc/platforms/powernv/opal-wrappers.S +1 -0
+	mtmsrd	r4,1; /* EE=0 RI=0 */

  mtspr SPRN_SRR0,r5;
  mtspr SPRN_SRR1,r6;

arch/powerpc/platforms/powernv/opal-wrappers.S +0 -1
-	mtcr	r4;

  rfid


 opal_real_call:

arch/powerpc/platforms/powernv/opal-wrappers.S +0 -0
@@ -108,6 +109,8 @@ opal_real_call:
Quoted 8 lines
Quoted 6 lines

  mtlr r11
  li r11,MSR_LE
  andc r12,r12,r11

arch/powerpc/platforms/powernv/opal-wrappers.S +2 -0
+	li	r11,0;
+	mtmsrd	r11,1; /* EE=0 RI=0 */

  mtspr SPRN_HSRR1,r12

  LOAD_REG_ADDR(r11,opal)

Quoted 1 line

  ld r12,8(r11)

author reviewer
#2 lore raw
Quoted 9 lines · On Mon, 24 Apr 2017 11:47:48 +1000 Benjamin Herrenschmidt…
Quoted 6 lines · Nicholas Piggin

There are some windows in opal entry/exit that can not recover from a re-entrant interrupt (e.g., machine check) due to using SRR registers, but they currently do not have MSR_RI clear.

These were found by machine check injection coverage tests using the powerpc system simulator (Mambo).

So you make us enter/exit OPAL with RI off with your patch.

It should hrfid to opal with MSR_RI set. It seems to be doing the right thing when stepping through it with the simulator.

Quoted 8 lines

OPAL currently won't re-enable it (which is a problem... we need to deal with MCEs happening while in OPAL, especially the ones with the special recovery table).

So if we do that we need to make OPAL turn it back on. In that case I would suggest making OPAL also turn it back off, thus avoiding touching FIXUP_ENDIAN...

#3 lore raw
Quoted 8 lines · Nicholas Piggin

On Mon, 24 Apr 2017 11:47:48 +1000

Quoted 7 lines

Benjamin Herrenschmidt <[email protected]> wrote:

Quoted 4 lines · Nicholas Piggin

There are some windows in opal entry/exit that can not recover from a re-entrant interrupt (e.g., machine check) due to using SRR registers, but they currently do not have MSR_RI clear.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

These were found by machine check injection coverage tests using the powerpc system simulator (Mambo).

So you make us enter/exit OPAL with RI off with your patch.

It should hrfid to opal with MSR_RI set. It seems to be doing the right thing when stepping through it with the simulator.

Ok, it's me mis-reading it...

I am not fan of changing FIXUP_ENDIAN but I suppose we don't have much choice. This will slow down OPAL entry/exit further...maybe we should use HSRR0/1 instead ? That way we don't have to touch RI ...

Cheers,
Ben.

author reviewer
#4 lore raw
Quoted 14 lines · On Mon, 24 Apr 2017 15:09:02 +1000 Benjamin Herrenschmidt…
Quoted 13 lines · Nicholas Piggin

On Mon, 24 Apr 2017 11:47:48 +1000

Quoted 11 lines

Benjamin Herrenschmidt <[email protected]> wrote:

Quoted 6 lines · Nicholas Piggin

There are some windows in opal entry/exit that can not recover from a re-entrant interrupt (e.g., machine check) due to using SRR registers, but they currently do not have MSR_RI clear.

These were found by machine check injection coverage tests using the powerpc system simulator (Mambo).

So you make us enter/exit OPAL with RI off with your patch.

Quoted 8 lines
Quoted 2 lines

It should hrfid to opal with MSR_RI set. It seems to be doing the right thing when stepping through it with the simulator.

Ok, it's me mis-reading it...

I am not fan of changing FIXUP_ENDIAN but I suppose we don't have much choice. This will slow down OPAL entry/exit further...maybe we should use HSRR0/1 instead ? That way we don't have to touch RI ...

I'll see if I can make that work.