Skip to contentSkip to conversation

powerpc/rtas: various cleanups and improvements

PATCH In review Nicholas Piggin 4 participants 29 messages Mar 8, 2022 – May 24, 2022 PowerPC

lore
Share & tools
lore

Nicholas Piggin

author reviewer
#0 lore raw

cover powerpc/rtas: various cleanups and improvements

I had a bunch of random little fixes and cleanups around and was prompted to put them together and make a change to call RTAS with MSR[RI] enabled because of a report of the hard lockup watchdog NMI IPI hitting in an rtas call which then crashed because it's unrecoverable.

Could possibly move patch 9 earlier if it would help with backporting.

Thanks,
Nick

Nicholas Piggin (14):
powerpc/rtas: Move rtas entry assembly into its own file powerpc/rtas: Make enter_rtas a nokprobe symbol on 64-bit powerpc/rtas: Fix whitespace in rtas_entry.S powerpc/rtas: Call enter_rtas with MSR[EE] disabled powerpc/rtas: Modernise RI clearing on 64-bit powerpc/rtas: Load rtas entry MSR explicitly powerpc/rtas: PACA can be restored directly from SPRG powerpc/rtas: call enter_rtas in real-mode on 64-bit powerpc/rtas: Leave MSR[RI] enabled over RTAS call powerpc/rtas: replace rtas_call_unlocked with raw_rtas_call powerpc/rtas: tidy __fetch_rtas_last_error powerpc/rtas: Close theoretical memory leak powerpc/rtas: enture rtas_call is called with MMU enabled powerpc/rtas: Consolidate and improve checking for rtas callers

 arch/powerpc/include/asm/rtas.h              |   4 +-
 arch/powerpc/kernel/Makefile                 |   2 +-
 arch/powerpc/kernel/entry_32.S               |  49 ------
 arch/powerpc/kernel/entry_64.S               | 150 -------------------
 arch/powerpc/kernel/rtas.c                   | 132 +++++++++-------
 arch/powerpc/kernel/rtas_entry.S             | 144 ++++++++++++++++++
 arch/powerpc/platforms/pseries/hotplug-cpu.c |   2 +-
 arch/powerpc/platforms/pseries/ras.c         |   7 +-
 arch/powerpc/xmon/xmon.c                     |   2 +-
 9 files changed, 227 insertions(+), 265 deletions(-)
 create mode 100644 arch/powerpc/kernel/rtas_entry.S
Signature · 1 line
2.23.0
author reviewer
#1 lore raw

PATCH 1/14 powerpc/rtas: Move rtas entry assembly into its own file

This makes working on the code a bit easier.

---
 arch/powerpc/kernel/Makefile     |   2 +-
 arch/powerpc/kernel/entry_32.S   |  49 --------
 arch/powerpc/kernel/entry_64.S   | 150 -----------------------
 arch/powerpc/kernel/rtas_entry.S | 199 +++++++++++++++++++++++++++++++
 4 files changed, 200 insertions(+), 200 deletions(-)
 create mode 100644 arch/powerpc/kernel/rtas_entry.S
arch/powerpc/kernel/Makefile +1 -1
index 4d7829399570..13602b0f0472 100644
@@ -68,7 +68,7 @@ obj-$(CONFIG_PPC_BOOK3S_IDLE)	+= idle_book3s.o
 procfs-y			:= proc_powerpc.o
 obj-$(CONFIG_PROC_FS)		+= $(procfs-y)
 rtaspci-$(CONFIG_PPC64)-$(CONFIG_PCI)	:= rtas_pci.o
-obj-$(CONFIG_PPC_RTAS)		+= rtas.o rtas-rtc.o $(rtaspci-y-y)
+obj-$(CONFIG_PPC_RTAS)		+= rtas_entry.o rtas.o rtas-rtc.o $(rtaspci-y-y)
 obj-$(CONFIG_PPC_RTAS_DAEMON)	+= rtasd.o
 obj-$(CONFIG_RTAS_FLASH)	+= rtas_flash.o
 obj-$(CONFIG_RTAS_PROC)		+= rtas-proc.o
arch/powerpc/kernel/entry_32.S +0 -49

52 lines not on this page · raw message

arch/powerpc/kernel/entry_64.S +0 -150

156 lines not on this page · raw message

arch/powerpc/kernel/rtas_entry.S new file +199 -0

199 lines not on this page · raw message

Signature · 1 line
2.23.0
author reviewer
#2 lore raw

PATCH 2/14 powerpc/rtas: Make enter_rtas a nokprobe symbol on 64-bit

In reply to Nicholas Piggin

This symbol is marked nokprobe on 32-bit but not 64-bit, add it.

---
 arch/powerpc/kernel/rtas_entry.S | 1 +
 1 file changed, 1 insertion(+)
arch/powerpc/kernel/rtas_entry.S +1 -0
index 192fea342744..afe3b789bc36 100644
@@ -165,6 +165,7 @@ rtas_return_loc:
 	mtspr	SPRN_SRR1,r4
 	RFI_TO_KERNEL
 	b	.	/* prevent speculative execution */
+_ASM_NOKPROBE_SYMBOL(enter_rtas)
 _ASM_NOKPROBE_SYMBOL(__enter_rtas)
 _ASM_NOKPROBE_SYMBOL(rtas_return_loc)
 
Signature · 1 line
2.23.0
author reviewer
#3 lore raw

PATCH 3/14 powerpc/rtas: Fix whitespace in rtas_entry.S

In reply to Nicholas Piggin

The code was moved verbatim including whitespace cruft. Fix that.

---
 arch/powerpc/kernel/rtas_entry.S | 34 ++++++++++++++++----------------
 1 file changed, 17 insertions(+), 17 deletions(-)
arch/powerpc/kernel/rtas_entry.S +17 -17
index afe3b789bc36..6fa10eb49a9c 100644
@@ -60,12 +60,12 @@ _ASM_NOKPROBE_SYMBOL(enter_rtas)
 _GLOBAL(enter_rtas)
 	mflr	r0
 	std	r0,16(r1)
-        stdu	r1,-SWITCH_FRAME_SIZE(r1) /* Save SP and create stack space. */
+	stdu	r1,-SWITCH_FRAME_SIZE(r1) /* Save SP and create stack space. */
 
 	/* Because RTAS is running in 32b mode, it clobbers the high order half
 	 * of all registers that it saves.  We therefore save those registers
 	 * RTAS might touch to the stack.  (r0, r3-r13 are caller saved)
-   	 */
+	 */
 	SAVE_GPR(2, r1)			/* Save the TOC */
 	SAVE_GPR(13, r1)		/* Save paca */
 	SAVE_NVGPRS(r1)			/* Save the non-volatiles */
@@ -105,21 +105,21 @@ _GLOBAL(enter_rtas)
 	/* Unfortunately, the stack pointer and the MSR are also clobbered,
 	 * so they are saved in the PACA which allows us to restore
 	 * our original state after RTAS returns.
-         */
+	 */
 	std	r1,PACAR1(r13)
-        std	r6,PACASAVEDMSR(r13)
+	std	r6,PACASAVEDMSR(r13)
 
-	/* Setup our real return addr */	
+	/* Setup our real return addr */
 	LOAD_REG_ADDR(r4,rtas_return_loc)
 	clrldi	r4,r4,2			/* convert to realmode address */
-       	mtlr	r4
+	mtlr	r4
 
 	li	r0,0
 	ori	r0,r0,MSR_EE|MSR_SE|MSR_BE|MSR_RI
 	andc	r0,r6,r0
-	
-        li      r9,1
-        rldicr  r9,r9,MSR_SF_LG,(63-MSR_SF_LG)
+
+	li	r9,1
+	rldicr	r9,r9,MSR_SF_LG,(63-MSR_SF_LG)
 	ori	r9,r9,MSR_IR|MSR_DR|MSR_FE0|MSR_FE1|MSR_FP|MSR_RI|MSR_LE
 	andc	r6,r0,r9
 
@@ -130,7 +130,7 @@ __enter_rtas:
 	LOAD_REG_ADDR(r4, rtas)
 	ld	r5,RTASENTRY(r4)	/* get the rtas->entry value */
 	ld	r4,RTASBASE(r4)		/* get the rtas->base value */
-	
+
 	mtspr	SPRN_SRR0,r5
 	mtspr	SPRN_SRR1,r6
 	RFI_TO_KERNEL
@@ -142,13 +142,13 @@ rtas_return_loc:
 	/*
 	 * Clear RI and set SF before anything.
 	 */
-	mfmsr   r6
+	mfmsr	r6
 	li	r0,MSR_RI
 	andc	r6,r6,r0
 	sldi	r0,r0,(MSR_SF_LG - MSR_RI_LG)
 	or	r6,r6,r0
 	sync
-	mtmsrd  r6
+	mtmsrd	r6
 
 	/* relocation is off at this point */
 	GET_PACA(r4)
@@ -158,8 +158,8 @@ rtas_return_loc:
 0:	mflr	r3
 	ld	r3,(1f-0b)(r3)		/* get &rtas_restore_regs */
 
-        ld	r1,PACAR1(r4)           /* Restore our SP */
-        ld	r4,PACASAVEDMSR(r4)     /* Restore our MSR */
+	ld	r1,PACAR1(r4)		/* Restore our SP */
+	ld	r4,PACASAVEDMSR(r4)	/* Restore our MSR */
 
 	mtspr	SPRN_SRR0,r3
 	mtspr	SPRN_SRR1,r4
@@ -191,10 +191,10 @@ rtas_restore_regs:
 	ld	r8,_DSISR(r1)
 	mtdsisr	r8
 
-        addi	r1,r1,SWITCH_FRAME_SIZE	/* Unstack our frame */
+	addi	r1,r1,SWITCH_FRAME_SIZE	/* Unstack our frame */
 	ld	r0,16(r1)		/* get return address */
 
-	mtlr    r0
-        blr				/* return to caller */
+	mtlr	r0
+	blr				/* return to caller */
 
 #endif /* CONFIG_PPC32 */
Signature · 1 line
2.23.0
author reviewer
#4 lore raw

PATCH 4/14 powerpc/rtas: Call enter_rtas with MSR[EE] disabled

In reply to Nicholas Piggin

Disable MSR[EE] in C code rather than asm.

---
 arch/powerpc/kernel/rtas.c       |  4 ++++
 arch/powerpc/kernel/rtas_entry.S | 17 +----------------
 2 files changed, 5 insertions(+), 16 deletions(-)
arch/powerpc/kernel/rtas.c +4 -0
index 733e6ef36758..6b5892d6a56b 100644
@@ -49,6 +49,10 @@ void enter_rtas(unsigned long);
 
 static inline void do_enter_rtas(unsigned long args)
 {
+	BUG_ON(!irqs_disabled());
+
+	hard_irq_disable(); /* Ensure MSR[EE] is disabled on PPC64 */
+
 	enter_rtas(args);
 
 	srr_regs_clobbered(); /* rtas uses SRRs, invalidate */
arch/powerpc/kernel/rtas_entry.S +1 -16
index 6fa10eb49a9c..45fa661c2ff6 100644
@@ -24,8 +24,6 @@ _GLOBAL(enter_rtas)
 	lwz	r4,RTASBASE(r4)
 	mfmsr	r9
 	stw	r9,8(r1)
-	LOAD_REG_IMMEDIATE(r0,MSR_KERNEL)
-	mtmsr	r0	/* disable interrupts so SRR0/1 don't get trashed */
 	li	r9,MSR_KERNEL & ~(MSR_IR|MSR_DR)
 	mtlr	r6
 	stw	r1, THREAD + RTAS_SP(r2)
@@ -87,20 +85,7 @@ _GLOBAL(enter_rtas)
 	li	r0,0
 	mtcr	r0
 
-#ifdef CONFIG_BUG
-	/* There is no way it is acceptable to get here with interrupts enabled,
-	 * check it with the asm equivalent of WARN_ON
-	 */
-	lbz	r0,PACAIRQSOFTMASK(r13)
-1:	tdeqi	r0,IRQS_ENABLED
-	EMIT_WARN_ENTRY 1b,__FILE__,__LINE__,BUGFLAG_WARNING
-#endif
-
-	/* Hard-disable interrupts */
 	mfmsr	r6
-	rldicl	r7,r6,48,1
-	rotldi	r7,r7,16
-	mtmsrd	r7,1
 
 	/* Unfortunately, the stack pointer and the MSR are also clobbered,
 	 * so they are saved in the PACA which allows us to restore
@@ -124,7 +109,7 @@ _GLOBAL(enter_rtas)
 	andc	r6,r0,r9
 
 __enter_rtas:
-	sync				/* disable interrupts so SRR0/1 */
+	sync				/* disable RI so SRR0/1 */
 	mtmsrd	r0			/* don't get trashed */
 
 	LOAD_REG_ADDR(r4, rtas)
Signature · 1 line
2.23.0
Reviewed-by on v1 4/14
#5 lore raw

Re: [PATCH 04/14] powerpc/rtas: Call enter_rtas with MSR[EE] disabled

Quoted 3 lines · Nicholas Piggin

Disable MSR[EE] in C code rather than asm.

FWIW,

Quoted 63 lines
---
 arch/powerpc/kernel/rtas.c       |  4 ++++
 arch/powerpc/kernel/rtas_entry.S | 17 +----------------
 2 files changed, 5 insertions(+), 16 deletions(-)
arch/powerpc/kernel/rtas.c +4 -0
index 733e6ef36758..6b5892d6a56b 100644
@@ -49,6 +49,10 @@ void enter_rtas(unsigned long);
 
 static inline void do_enter_rtas(unsigned long args)
 {
+	BUG_ON(!irqs_disabled());
+
+	hard_irq_disable(); /* Ensure MSR[EE] is disabled on PPC64 */
+
 	enter_rtas(args);
 
 	srr_regs_clobbered(); /* rtas uses SRRs, invalidate */
arch/powerpc/kernel/rtas_entry.S +1 -16
index 6fa10eb49a9c..45fa661c2ff6 100644
@@ -24,8 +24,6 @@ _GLOBAL(enter_rtas)
 	lwz	r4,RTASBASE(r4)
 	mfmsr	r9
 	stw	r9,8(r1)
-	LOAD_REG_IMMEDIATE(r0,MSR_KERNEL)
-	mtmsr	r0	/* disable interrupts so SRR0/1 don't get trashed */
 	li	r9,MSR_KERNEL & ~(MSR_IR|MSR_DR)
 	mtlr	r6
 	stw	r1, THREAD + RTAS_SP(r2)
@@ -87,20 +85,7 @@ _GLOBAL(enter_rtas)
 	li	r0,0
 	mtcr	r0
 
-#ifdef CONFIG_BUG
-	/* There is no way it is acceptable to get here with interrupts enabled,
-	 * check it with the asm equivalent of WARN_ON
-	 */
-	lbz	r0,PACAIRQSOFTMASK(r13)
-1:	tdeqi	r0,IRQS_ENABLED
-	EMIT_WARN_ENTRY 1b,__FILE__,__LINE__,BUGFLAG_WARNING
-#endif
-
-	/* Hard-disable interrupts */
 	mfmsr	r6
-	rldicl	r7,r6,48,1
-	rotldi	r7,r7,16
-	mtmsrd	r7,1
 
 	/* Unfortunately, the stack pointer and the MSR are also clobbered,
 	 * so they are saved in the PACA which allows us to restore
@@ -124,7 +109,7 @@ _GLOBAL(enter_rtas)
 	andc	r6,r0,r9
 
 __enter_rtas:
-	sync				/* disable interrupts so SRR0/1 */
+	sync				/* disable RI so SRR0/1 */
 	mtmsrd	r0			/* don't get trashed */
 
 	LOAD_REG_ADDR(r4, rtas)
author reviewer
#6 lore raw

PATCH 5/14 powerpc/rtas: Modernise RI clearing on 64-bit

In reply to Nicholas Piggin

mtmsrd L=1 can clear MSR[RI] without the previous MSR value; it does not require sync; it can be moved later to before SRRs are live.

---
 arch/powerpc/kernel/rtas_entry.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/powerpc/kernel/rtas_entry.S +3 -3
index 45fa661c2ff6..7b93687b9a10 100644
@@ -109,13 +109,13 @@ _GLOBAL(enter_rtas)
 	andc	r6,r0,r9
 
 __enter_rtas:
-	sync				/* disable RI so SRR0/1 */
-	mtmsrd	r0			/* don't get trashed */
-
 	LOAD_REG_ADDR(r4, rtas)
 	ld	r5,RTASENTRY(r4)	/* get the rtas->entry value */
 	ld	r4,RTASBASE(r4)		/* get the rtas->base value */
 
+	li	r0,0
+	mtmsrd	r0,1			/* disable RI before using SRR0/1 */
+
 	mtspr	SPRN_SRR0,r5
 	mtspr	SPRN_SRR1,r6
 	RFI_TO_KERNEL
Signature · 1 line
2.23.0
Reviewed-by on v1 5/14
#7 lore raw

Re: [PATCH 05/14] powerpc/rtas: Modernise RI clearing on 64-bit

Quoted 4 lines · Nicholas Piggin

mtmsrd L=1 can clear MSR[RI] without the previous MSR value; it does not require sync; it can be moved later to before SRRs are live.

Quoted 25 lines
---
 arch/powerpc/kernel/rtas_entry.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
arch/powerpc/kernel/rtas_entry.S +3 -3
index 45fa661c2ff6..7b93687b9a10 100644
@@ -109,13 +109,13 @@ _GLOBAL(enter_rtas)
 	andc	r6,r0,r9
 
 __enter_rtas:
-	sync				/* disable RI so SRR0/1 */
-	mtmsrd	r0			/* don't get trashed */
-
 	LOAD_REG_ADDR(r4, rtas)
 	ld	r5,RTASENTRY(r4)	/* get the rtas->entry value */
 	ld	r4,RTASBASE(r4)		/* get the rtas->base value */
 
+	li	r0,0
+	mtmsrd	r0,1			/* disable RI before using SRR0/1 */
+
 	mtspr	SPRN_SRR0,r5
 	mtspr	SPRN_SRR1,r6
 	RFI_TO_KERNEL
author reviewer
#8 lore raw

PATCH 6/14 powerpc/rtas: Load rtas entry MSR explicitly

In reply to Nicholas Piggin

Rather than adjust the current MSR value to find the rtas entry MSR on 64-bit, load the explicit value we want as 32-bit does.

This prevents some facilities (e.g., VEC and VSX) from being left enabled which doesn't seem to cause a problem but it's more consistent to always use the same MSR and minimise facilities enabled.

---
 arch/powerpc/kernel/rtas_entry.S | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)
arch/powerpc/kernel/rtas_entry.S +1 -8
index 7b93687b9a10..08eb731f08b8 100644
@@ -99,14 +99,7 @@ _GLOBAL(enter_rtas)
 	clrldi	r4,r4,2			/* convert to realmode address */
 	mtlr	r4
 
-	li	r0,0
-	ori	r0,r0,MSR_EE|MSR_SE|MSR_BE|MSR_RI
-	andc	r0,r6,r0
-
-	li	r9,1
-	rldicr	r9,r9,MSR_SF_LG,(63-MSR_SF_LG)
-	ori	r9,r9,MSR_IR|MSR_DR|MSR_FE0|MSR_FE1|MSR_FP|MSR_RI|MSR_LE
-	andc	r6,r0,r9
+	LOAD_REG_IMMEDIATE(r6, MSR_ME)
 
 __enter_rtas:
 	LOAD_REG_ADDR(r4, rtas)
Signature · 1 line
2.23.0
Reviewed-by on v1 6/14
#9 lore raw

Re: [PATCH 06/14] powerpc/rtas: Load rtas entry MSR explicitly

Quoted 9 lines · Nicholas Piggin

Rather than adjust the current MSR value to find the rtas entry MSR on 64-bit, load the explicit value we want as 32-bit does.

This prevents some facilities (e.g., VEC and VSX) from being left enabled which doesn't seem to cause a problem but it's more consistent to always use the same MSR and minimise facilities enabled.

Quoted 24 lines
---
 arch/powerpc/kernel/rtas_entry.S | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)
arch/powerpc/kernel/rtas_entry.S +1 -8
index 7b93687b9a10..08eb731f08b8 100644
@@ -99,14 +99,7 @@ _GLOBAL(enter_rtas)
 	clrldi	r4,r4,2			/* convert to realmode address */
 	mtlr	r4
 
-	li	r0,0
-	ori	r0,r0,MSR_EE|MSR_SE|MSR_BE|MSR_RI
-	andc	r0,r6,r0
-
-	li	r9,1
-	rldicr	r9,r9,MSR_SF_LG,(63-MSR_SF_LG)
-	ori	r9,r9,MSR_IR|MSR_DR|MSR_FE0|MSR_FE1|MSR_FP|MSR_RI|MSR_LE
-	andc	r6,r0,r9
+	LOAD_REG_IMMEDIATE(r6, MSR_ME)
 
 __enter_rtas:
 	LOAD_REG_ADDR(r4, rtas)
author reviewer
#10 lore raw

PATCH 7/14 powerpc/rtas: PACA can be restored directly from SPRG

In reply to Nicholas Piggin

On 64-bit, PACA is saved in a SPRG so it does not need to be saved on stack. We also don't need to mask off the top bits for real mode addresses because the architecture does this for us.

---
 arch/powerpc/kernel/rtas_entry.S | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)
arch/powerpc/kernel/rtas_entry.S +4 -9
index 08eb731f08b8..5f65ea4436c6 100644
@@ -62,10 +62,9 @@ _GLOBAL(enter_rtas)
 
 	/* Because RTAS is running in 32b mode, it clobbers the high order half
 	 * of all registers that it saves.  We therefore save those registers
-	 * RTAS might touch to the stack.  (r0, r3-r13 are caller saved)
+	 * RTAS might touch to the stack.  (r0, r3-r12 are caller saved)
 	 */
 	SAVE_GPR(2, r1)			/* Save the TOC */
-	SAVE_GPR(13, r1)		/* Save paca */
 	SAVE_NVGPRS(r1)			/* Save the non-volatiles */
 
 	mfcr	r4
@@ -129,15 +128,14 @@ rtas_return_loc:
 	mtmsrd	r6
 
 	/* relocation is off at this point */
-	GET_PACA(r4)
-	clrldi	r4,r4,2			/* convert to realmode address */
+	GET_PACA(r13)
 
 	bcl	20,31,$+4
 0:	mflr	r3
 	ld	r3,(1f-0b)(r3)		/* get &rtas_restore_regs */
 
-	ld	r1,PACAR1(r4)		/* Restore our SP */
-	ld	r4,PACASAVEDMSR(r4)	/* Restore our MSR */
+	ld	r1,PACAR1(r13)		/* Restore our SP */
+	ld	r4,PACASAVEDMSR(r13)	/* Restore our MSR */
 
 	mtspr	SPRN_SRR0,r3
 	mtspr	SPRN_SRR1,r4
@@ -153,11 +151,8 @@ _ASM_NOKPROBE_SYMBOL(rtas_return_loc)
 rtas_restore_regs:
 	/* relocation is on at this point */
 	REST_GPR(2, r1)			/* Restore the TOC */
-	REST_GPR(13, r1)		/* Restore paca */
 	REST_NVGPRS(r1)			/* Restore the non-volatiles */
 
-	GET_PACA(r13)
-
 	ld	r4,_CCR(r1)
 	mtcr	r4
 	ld	r5,_CTR(r1)
Signature · 1 line
2.23.0
Reviewed-by on v1 7/14
#11 lore raw

Re: [PATCH 07/14] powerpc/rtas: PACA can be restored directly from SPRG

Quoted 5 lines · Nicholas Piggin

On 64-bit, PACA is saved in a SPRG so it does not need to be saved on stack. We also don't need to mask off the top bits for real mode addresses because the architecture does this for us.

Quoted 51 lines
---
 arch/powerpc/kernel/rtas_entry.S | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)
arch/powerpc/kernel/rtas_entry.S +4 -9
index 08eb731f08b8..5f65ea4436c6 100644
@@ -62,10 +62,9 @@ _GLOBAL(enter_rtas)
 
 	/* Because RTAS is running in 32b mode, it clobbers the high order half
 	 * of all registers that it saves.  We therefore save those registers
-	 * RTAS might touch to the stack.  (r0, r3-r13 are caller saved)
+	 * RTAS might touch to the stack.  (r0, r3-r12 are caller saved)
 	 */
 	SAVE_GPR(2, r1)			/* Save the TOC */
-	SAVE_GPR(13, r1)		/* Save paca */
 	SAVE_NVGPRS(r1)			/* Save the non-volatiles */
 
 	mfcr	r4
@@ -129,15 +128,14 @@ rtas_return_loc:
 	mtmsrd	r6
 
 	/* relocation is off at this point */
-	GET_PACA(r4)
-	clrldi	r4,r4,2			/* convert to realmode address */
+	GET_PACA(r13)
 
 	bcl	20,31,$+4
 0:	mflr	r3
 	ld	r3,(1f-0b)(r3)		/* get &rtas_restore_regs */
 
-	ld	r1,PACAR1(r4)		/* Restore our SP */
-	ld	r4,PACASAVEDMSR(r4)	/* Restore our MSR */
+	ld	r1,PACAR1(r13)		/* Restore our SP */
+	ld	r4,PACASAVEDMSR(r13)	/* Restore our MSR */
 
 	mtspr	SPRN_SRR0,r3
 	mtspr	SPRN_SRR1,r4
@@ -153,11 +151,8 @@ _ASM_NOKPROBE_SYMBOL(rtas_return_loc)
 rtas_restore_regs:
 	/* relocation is on at this point */
 	REST_GPR(2, r1)			/* Restore the TOC */
-	REST_GPR(13, r1)		/* Restore paca */
 	REST_NVGPRS(r1)			/* Restore the non-volatiles */
 
-	GET_PACA(r13)
-
 	ld	r4,_CCR(r1)
 	mtcr	r4
 	ld	r5,_CTR(r1)
author reviewer
#12 lore raw

PATCH 8/14 powerpc/rtas: call enter_rtas in real-mode on 64-bit

In reply to Nicholas Piggin

This moves MSR save/restore and some real-mode juggling out of asm and into C code, simplifying things.

---
 arch/powerpc/kernel/rtas.c       | 15 ++++++++++++---
 arch/powerpc/kernel/rtas_entry.S | 32 +++++---------------------------
 2 files changed, 17 insertions(+), 30 deletions(-)
arch/powerpc/kernel/rtas.c +12 -3
index 6b5892d6a56b..87ede1877816 100644
@@ -47,13 +47,22 @@
 /* This is here deliberately so it's only used in this file */
 void enter_rtas(unsigned long);
 
-static inline void do_enter_rtas(unsigned long args)
+static noinline void do_enter_rtas(unsigned long args)
 {
 	BUG_ON(!irqs_disabled());
 
-	hard_irq_disable(); /* Ensure MSR[EE] is disabled on PPC64 */
+	if (IS_ENABLED(CONFIG_PPC64)) {
+		unsigned long msr;
 
-	enter_rtas(args);
+		hard_irq_disable();
+
+		msr = mfmsr();
+		mtmsr(msr & ~(MSR_IR|MSR_DR));
+		enter_rtas(args);
+		mtmsr(msr);
+	} else {
+		enter_rtas(args);
+	}
 
 	srr_regs_clobbered(); /* rtas uses SRRs, invalidate */
 }
arch/powerpc/kernel/rtas_entry.S +5 -27
index 5f65ea4436c6..292551684bbd 100644
@@ -84,14 +84,11 @@ _GLOBAL(enter_rtas)
 	li	r0,0
 	mtcr	r0
 
-	mfmsr	r6
-
-	/* Unfortunately, the stack pointer and the MSR are also clobbered,
-	 * so they are saved in the PACA which allows us to restore
-	 * our original state after RTAS returns.
+	/*
+	 * The stack pointer is clobbered, so it is saved in the PACA which
+	 * allows us to restore our original state after RTAS returns.
 	 */
 	std	r1,PACAR1(r13)
-	std	r6,PACASAVEDMSR(r13)
 
 	/* Setup our real return addr */
 	LOAD_REG_ADDR(r4,rtas_return_loc)
@@ -100,7 +97,6 @@ _GLOBAL(enter_rtas)
 
 	LOAD_REG_IMMEDIATE(r6, MSR_ME)
 
-__enter_rtas:
 	LOAD_REG_ADDR(r4, rtas)
 	ld	r5,RTASENTRY(r4)	/* get the rtas->entry value */
 	ld	r4,RTASBASE(r4)		/* get the rtas->base value */
@@ -112,6 +108,7 @@ __enter_rtas:
 	mtspr	SPRN_SRR1,r6
 	RFI_TO_KERNEL
 	b	.	/* prevent speculative execution */
+_ASM_NOKPROBE_SYMBOL(enter_rtas)
 
 rtas_return_loc:
 	FIXUP_ENDIAN
@@ -127,29 +124,10 @@ rtas_return_loc:
 	sync
 	mtmsrd	r6
 
-	/* relocation is off at this point */
 	GET_PACA(r13)
 
-	bcl	20,31,$+4
-0:	mflr	r3
-	ld	r3,(1f-0b)(r3)		/* get &rtas_restore_regs */
-
 	ld	r1,PACAR1(r13)		/* Restore our SP */
-	ld	r4,PACASAVEDMSR(r13)	/* Restore our MSR */
 
-	mtspr	SPRN_SRR0,r3
-	mtspr	SPRN_SRR1,r4
-	RFI_TO_KERNEL
-	b	.	/* prevent speculative execution */
-_ASM_NOKPROBE_SYMBOL(enter_rtas)
-_ASM_NOKPROBE_SYMBOL(__enter_rtas)
-_ASM_NOKPROBE_SYMBOL(rtas_return_loc)
-
-	.align	3
-1:	.8byte	rtas_restore_regs
-
-rtas_restore_regs:
-	/* relocation is on at this point */
 	REST_GPR(2, r1)			/* Restore the TOC */
 	REST_NVGPRS(r1)			/* Restore the non-volatiles */
 
@@ -169,5 +147,5 @@ rtas_restore_regs:
 
 	mtlr	r0
 	blr				/* return to caller */
-
+_ASM_NOKPROBE_SYMBOL(rtas_return_loc)
 #endif /* CONFIG_PPC32 */
Signature · 1 line
2.23.0
#13 lore raw

Re: [PATCH 08/14] powerpc/rtas: call enter_rtas in real-mode on 64-bit

Quoted 78 lines · Nicholas Piggin

This moves MSR save/restore and some real-mode juggling out of asm and into C code, simplifying things.

---
 arch/powerpc/kernel/rtas.c       | 15 ++++++++++++---
 arch/powerpc/kernel/rtas_entry.S | 32 +++++---------------------------
 2 files changed, 17 insertions(+), 30 deletions(-)
arch/powerpc/kernel/rtas.c +12 -3
index 6b5892d6a56b..87ede1877816 100644
@@ -47,13 +47,22 @@
 /* This is here deliberately so it's only used in this file */
 void enter_rtas(unsigned long);
 
-static inline void do_enter_rtas(unsigned long args)
+static noinline void do_enter_rtas(unsigned long args)
 {
 	BUG_ON(!irqs_disabled());
 
-	hard_irq_disable(); /* Ensure MSR[EE] is disabled on PPC64 */
+	if (IS_ENABLED(CONFIG_PPC64)) {
+		unsigned long msr;
 
-	enter_rtas(args);
+		hard_irq_disable();
+
+		msr = mfmsr();
+		mtmsr(msr & ~(MSR_IR|MSR_DR));
+		enter_rtas(args);
+		mtmsr(msr);
+	} else {
+		enter_rtas(args);
+	}
 
 	srr_regs_clobbered(); /* rtas uses SRRs, invalidate */
 }
arch/powerpc/kernel/rtas_entry.S +4 -7
index 5f65ea4436c6..292551684bbd 100644
@@ -84,14 +84,11 @@ _GLOBAL(enter_rtas)
 	li	r0,0
 	mtcr	r0
 
-	mfmsr	r6
-
-	/* Unfortunately, the stack pointer and the MSR are also clobbered,
-	 * so they are saved in the PACA which allows us to restore
-	 * our original state after RTAS returns.
+	/*
+	 * The stack pointer is clobbered, so it is saved in the PACA which
+	 * allows us to restore our original state after RTAS returns.
 	 */
 	std	r1,PACAR1(r13)
-	std	r6,PACASAVEDMSR(r13)
 
 	/* Setup our real return addr */
 	LOAD_REG_ADDR(r4,rtas_return_loc)
@@ -100,7 +97,6 @@ _GLOBAL(enter_rtas)
 
 	LOAD_REG_IMMEDIATE(r6, MSR_ME)
 
-__enter_rtas:
 	LOAD_REG_ADDR(r4, rtas)
 	ld	r5,RTASENTRY(r4)	/* get the rtas->entry value */
 	ld	r4,RTASBASE(r4)		/* get the rtas->base value */
@@ -112,6 +108,7 @@ __enter_rtas:
 	mtspr	SPRN_SRR1,r6
 	RFI_TO_KERNEL
 	b	.	/* prevent speculative execution */
+_ASM_NOKPROBE_SYMBOL(enter_rtas)
 
 rtas_return_loc:
 	FIXUP_ENDIAN

arch/powerpc/kernel/rtas_entry.S rtas_return_loc: · line 125

Quoted 2 lines
arch/powerpc/kernel/rtas_entry.S +0 -0
@@ -127,29 +124,10 @@ rtas_return_loc:
 	sync
 	mtmsrd	r6

Since MSR plumbing is still needed in the asm, what is the benefit of doing the real mode switching in the C code?

What if the MSR is saved in the PACA before switching to real mode, and restored in rtas_return_loc?

Quoted 4 lines
arch/powerpc/kernel/rtas_entry.S +0 -1
-	/* relocation is off at this point */
 	GET_PACA(r13)
 

arch/powerpc/kernel/rtas_entry.S rtas_return_loc: · line 142

Quoted 10 lines
arch/powerpc/kernel/rtas_entry.S +0 -8
-	bcl	20,31,$+4
-0:	mflr	r3
-	ld	r3,(1f-0b)(r3)		/* get &rtas_restore_regs */
-
 	ld	r1,PACAR1(r13)		/* Restore our SP */
-	ld	r4,PACASAVEDMSR(r13)	/* Restore our MSR */
 
-	mtspr	SPRN_SRR0,r3
-	mtspr	SPRN_SRR1,r4
-	RFI_TO_KERNEL

rfid is not more called to restore MSR.
Noob question, is there any impact of using mtmsrd instead of rfid to restore the MSR?

Quoted 20 lines
arch/powerpc/kernel/rtas_entry.S +1 -11
-	b	.	/* prevent speculative execution */
-_ASM_NOKPROBE_SYMBOL(enter_rtas)
-_ASM_NOKPROBE_SYMBOL(__enter_rtas)
-_ASM_NOKPROBE_SYMBOL(rtas_return_loc)
-
-	.align	3
-1:	.8byte	rtas_restore_regs
-
-rtas_restore_regs:
-	/* relocation is on at this point */
 	REST_GPR(2, r1)			/* Restore the TOC */
 	REST_NVGPRS(r1)			/* Restore the non-volatiles */
 
@@ -169,5 +147,5 @@ rtas_restore_regs:
 
 	mtlr	r0
 	blr				/* return to caller */
-
+_ASM_NOKPROBE_SYMBOL(rtas_return_loc)
 #endif /* CONFIG_PPC32 */
#14 lore raw

Re: [PATCH 08/14] powerpc/rtas: call enter_rtas in real-mode on 64-bit

In reply to Nicholas Piggin
Quoted 21 lines · Nicholas Piggin

This moves MSR save/restore and some real-mode juggling out of asm and into C code, simplifying things.

---
 arch/powerpc/kernel/rtas.c       | 15 ++++++++++++---
 arch/powerpc/kernel/rtas_entry.S | 32 +++++---------------------------
 2 files changed, 17 insertions(+), 30 deletions(-)
arch/powerpc/kernel/rtas.c +1 -1
index 6b5892d6a56b..87ede1877816 100644
@@ -47,13 +47,22 @@
 /* This is here deliberately so it's only used in this file */
 void enter_rtas(unsigned long);
 
-static inline void do_enter_rtas(unsigned long args)
+static noinline void do_enter_rtas(unsigned long args)
 {
 	BUG_ON(!irqs_disabled());

arch/powerpc/kernel/rtas.c line 60

Quoted 10 lines
arch/powerpc/kernel/rtas.c +6 -2
 
-	hard_irq_disable(); /* Ensure MSR[EE] is disabled on PPC64 */
+	if (IS_ENABLED(CONFIG_PPC64)) {
+		unsigned long msr;
 
-	enter_rtas(args);
+		hard_irq_disable();
+
+		msr = mfmsr();
+		mtmsr(msr & ~(MSR_IR|MSR_DR));

Further test done on top this series shows that switching MSR_DR off before entering enter_rtas() is generating DSI when accessing the stack in enter_rtas(). This may happen if the task stack is mapped beyond the VRMA.

Furthermore, there is no real need to run enter_rtas() in real mode (IR and DR unset) because the MSR will be set to real mode when doing rfid, see below.

Quoted 39 lines
arch/powerpc/kernel/rtas.c +5 -0
+		enter_rtas(args);
+		mtmsr(msr);
+	} else {
+		enter_rtas(args);
+	}
 
 	srr_regs_clobbered(); /* rtas uses SRRs, invalidate */
 }
arch/powerpc/kernel/rtas_entry.S +3 -7
index 5f65ea4436c6..292551684bbd 100644
@@ -84,14 +84,11 @@ _GLOBAL(enter_rtas)
 	li	r0,0
 	mtcr	r0
 
-	mfmsr	r6
-
-	/* Unfortunately, the stack pointer and the MSR are also clobbered,
-	 * so they are saved in the PACA which allows us to restore
-	 * our original state after RTAS returns.
+	/*
+	 * The stack pointer is clobbered, so it is saved in the PACA which
+	 * allows us to restore our original state after RTAS returns.
 	 */
 	std	r1,PACAR1(r13)
-	std	r6,PACASAVEDMSR(r13)
 
 	/* Setup our real return addr */
 	LOAD_REG_ADDR(r4,rtas_return_loc)
@@ -100,7 +97,6 @@ _GLOBAL(enter_rtas)
 
 	LOAD_REG_IMMEDIATE(r6, MSR_ME)
 
-__enter_rtas:
 	LOAD_REG_ADDR(r4, rtas)
 	ld	r5,RTASENTRY(r4)	/* get the rtas->entry value */
 	ld	r4,RTASBASE(r4)		/* get the rtas->base value */

arch/powerpc/kernel/rtas_entry.S __enter_rtas: · line 109

Quoted 2 lines
arch/powerpc/kernel/rtas_entry.S +0 -0
@@ -112,6 +108,7 @@ __enter_rtas:
 	mtspr	SPRN_SRR1,r6
 	RFI_TO_KERNEL

rfid will load the MSR with the value stored in SRR1 (formely r6) and so switch to the real mode. This why there is no need to switch earlier in real mode.

Quoted 11 lines
arch/powerpc/kernel/rtas_entry.S +1 -1
 	b	.	/* prevent speculative execution */
+_ASM_NOKPROBE_SYMBOL(enter_rtas)
 
 rtas_return_loc:
 	FIXUP_ENDIAN
@@ -127,29 +124,10 @@ rtas_return_loc:
 	sync
 	mtmsrd	r6
 
-	/* relocation is off at this point */>  	GET_PACA(r13)
 

arch/powerpc/kernel/rtas_entry.S rtas_return_loc: · line 141

Quoted 10 lines
arch/powerpc/kernel/rtas_entry.S +0 -8
-	bcl	20,31,$+4
-0:	mflr	r3
-	ld	r3,(1f-0b)(r3)		/* get &rtas_restore_regs */
-
 	ld	r1,PACAR1(r13)		/* Restore our SP */
-	ld	r4,PACASAVEDMSR(r13)	/* Restore our MSR */
 
-	mtspr	SPRN_SRR0,r3
-	mtspr	SPRN_SRR1,r4
-	RFI_TO_KERNEL

This was turning on MSR_DR and MSR_IR so rtas_restore() could access the stack even if it is beyond the VRMA.

That patch is breaking this and generating panic when task's stack are below VRMA.

Quoted 20 lines
arch/powerpc/kernel/rtas_entry.S +1 -11
-	b	.	/* prevent speculative execution */
-_ASM_NOKPROBE_SYMBOL(enter_rtas)
-_ASM_NOKPROBE_SYMBOL(__enter_rtas)
-_ASM_NOKPROBE_SYMBOL(rtas_return_loc)
-
-	.align	3
-1:	.8byte	rtas_restore_regs
-
-rtas_restore_regs:
-	/* relocation is on at this point */
 	REST_GPR(2, r1)			/* Restore the TOC */
 	REST_NVGPRS(r1)			/* Restore the non-volatiles */
 
@@ -169,5 +147,5 @@ rtas_restore_regs:
 
 	mtlr	r0
 	blr				/* return to caller */
-
+_ASM_NOKPROBE_SYMBOL(rtas_return_loc)
 #endif /* CONFIG_PPC32 */
author reviewer
#15 lore raw

PATCH 9/14 powerpc/rtas: Leave MSR[RI] enabled over RTAS call

In reply to Nicholas Piggin

PAPR specifies that RTAS may be called with MSR[RI] enabled if the calling context is recoverable, and RTAS will manage RI as necessary. Call the rtas entry point with RI enabled, and add a check to ensure the caller has RI enabled.

---
 arch/powerpc/kernel/rtas.c       |  1 +
 arch/powerpc/kernel/rtas_entry.S | 13 +++----------
 2 files changed, 4 insertions(+), 10 deletions(-)
arch/powerpc/kernel/rtas.c +1 -0
index 87ede1877816..fece066115f0 100644
@@ -57,6 +57,7 @@ static noinline void do_enter_rtas(unsigned long args)
 		hard_irq_disable();
 
 		msr = mfmsr();
+		BUG_ON(!(msr & MSR_RI));
 		mtmsr(msr & ~(MSR_IR|MSR_DR));
 		enter_rtas(args);
 		mtmsr(msr);
arch/powerpc/kernel/rtas_entry.S +3 -10
index 292551684bbd..72b27b14ccc9 100644
@@ -95,7 +95,7 @@ _GLOBAL(enter_rtas)
 	clrldi	r4,r4,2			/* convert to realmode address */
 	mtlr	r4
 
-	LOAD_REG_IMMEDIATE(r6, MSR_ME)
+	LOAD_REG_IMMEDIATE(r6, MSR_ME|MSR_RI)
 
 	LOAD_REG_ADDR(r4, rtas)
 	ld	r5,RTASENTRY(r4)	/* get the rtas->entry value */
@@ -113,15 +113,8 @@ _ASM_NOKPROBE_SYMBOL(enter_rtas)
 rtas_return_loc:
 	FIXUP_ENDIAN
 
-	/*
-	 * Clear RI and set SF before anything.
-	 */
-	mfmsr	r6
-	li	r0,MSR_RI
-	andc	r6,r6,r0
-	sldi	r0,r0,(MSR_SF_LG - MSR_RI_LG)
-	or	r6,r6,r0
-	sync
+	/* Set SF before anything. */
+	LOAD_REG_IMMEDIATE(r6, MSR_KERNEL & ~(MSR_IR|MSR_DR))
 	mtmsrd	r6
 
 	GET_PACA(r13)
Signature · 1 line
2.23.0
#16 lore raw

Re: [PATCH 09/14] powerpc/rtas: Leave MSR[RI] enabled over RTAS call

Quoted 41 lines · Nicholas Piggin

PAPR specifies that RTAS may be called with MSR[RI] enabled if the calling context is recoverable, and RTAS will manage RI as necessary. Call the rtas entry point with RI enabled, and add a check to ensure the caller has RI enabled.

---
 arch/powerpc/kernel/rtas.c       |  1 +
 arch/powerpc/kernel/rtas_entry.S | 13 +++----------
 2 files changed, 4 insertions(+), 10 deletions(-)
arch/powerpc/kernel/rtas.c +1 -0
index 87ede1877816..fece066115f0 100644
@@ -57,6 +57,7 @@ static noinline void do_enter_rtas(unsigned long args)
 		hard_irq_disable();
 
 		msr = mfmsr();
+		BUG_ON(!(msr & MSR_RI));
 		mtmsr(msr & ~(MSR_IR|MSR_DR));
 		enter_rtas(args);
 		mtmsr(msr);
arch/powerpc/kernel/rtas_entry.S +1 -2
index 292551684bbd..72b27b14ccc9 100644
@@ -95,7 +95,7 @@ _GLOBAL(enter_rtas)
 	clrldi	r4,r4,2			/* convert to realmode address */
 	mtlr	r4
 
-	LOAD_REG_IMMEDIATE(r6, MSR_ME)
+	LOAD_REG_IMMEDIATE(r6, MSR_ME|MSR_RI)
 
 	LOAD_REG_ADDR(r4, rtas)
 	ld	r5,RTASENTRY(r4)	/* get the rtas->entry value */
@@ -113,15 +113,8 @@ _ASM_NOKPROBE_SYMBOL(enter_rtas)
 rtas_return_loc:
 	FIXUP_ENDIAN
 
-	/*

arch/powerpc/kernel/rtas_entry.S _ASM_NOKPROBE_SYMBOL(enter_rtas) · line 117

Quoted 10 lines
arch/powerpc/kernel/rtas_entry.S +2 -8
-	 * Clear RI and set SF before anything.
-	 */
-	mfmsr	r6
-	li	r0,MSR_RI
-	andc	r6,r6,r0
-	sldi	r0,r0,(MSR_SF_LG - MSR_RI_LG)
-	or	r6,r6,r0
-	sync
+	/* Set SF before anything. */
+	LOAD_REG_IMMEDIATE(r6, MSR_KERNEL & ~(MSR_IR|MSR_DR))

I guess we cannot directly load a MSR value stored in the PACA at that time, isn't it?

Quoted 3 lines
arch/powerpc/kernel/rtas_entry.S +0 -0
 	mtmsrd	r6
 
 	GET_PACA(r13)
author reviewer
#17 lore raw

PATCH 10/14 powerpc/rtas: replace rtas_call_unlocked with raw_rtas_call

In reply to Nicholas Piggin

Use the same calling and rets return convention with the raw rtas call rather than requiring callers to load and byteswap return values out of rtas_args.

---
 arch/powerpc/include/asm/rtas.h              |  4 +-
 arch/powerpc/kernel/rtas.c                   | 53 +++++++++++---------
 arch/powerpc/platforms/pseries/hotplug-cpu.c |  2 +-
 arch/powerpc/platforms/pseries/ras.c         |  7 +--
 arch/powerpc/xmon/xmon.c                     |  2 +-
 5 files changed, 33 insertions(+), 35 deletions(-)
arch/powerpc/include/asm/rtas.h +2 -2
index 82e5b055fa2a..1014ff140cf8 100644
@@ -241,8 +241,8 @@ extern int rtas_token(const char *service);
 extern int rtas_service_present(const char *service);
 extern int rtas_call(int token, int, int, int *, ...);
 int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...);
-void rtas_call_unlocked(struct rtas_args *args, int token, int nargs,
-			int nret, ...);
+int raw_rtas_call(struct rtas_args *args, int token,
+			int nargs, int nret, int *outputs, ...);
 extern void __noreturn rtas_restart(char *cmd);
 extern void rtas_power_off(void);
 extern void __noreturn rtas_halt(void);
arch/powerpc/kernel/rtas.c +28 -25

108 lines not on this page · raw message

arch/powerpc/platforms/pseries/hotplug-cpu.c +1 -1
index b81fc846d99c..17c05650b6b9 100644
@@ -53,7 +53,7 @@ static void rtas_stop_self(void)
 
 	BUG_ON(rtas_stop_self_token == RTAS_UNKNOWN_SERVICE);
 
-	rtas_call_unlocked(&args, rtas_stop_self_token, 0, 1, NULL);
+	raw_rtas_call(&args, rtas_stop_self_token, 0, 1, NULL);
 
 	panic("Alas, I survived.\n");
 }
arch/powerpc/platforms/pseries/ras.c +1 -6
index 74c9b1b5bc66..b009ed7de1ee 100644
@@ -465,12 +465,7 @@ static void fwnmi_release_errinfo(void)
 	struct rtas_args rtas_args;
 	int ret;
 
-	/*
-	 * On pseries, the machine check stack is limited to under 4GB, so
-	 * args can be on-stack.
-	 */
-	rtas_call_unlocked(&rtas_args, ibm_nmi_interlock_token, 0, 1, NULL);
-	ret = be32_to_cpu(rtas_args.rets[0]);
+	ret = raw_rtas_call(&rtas_args, ibm_nmi_interlock_token, 0, 1, NULL);
 	if (ret != 0)
 		printk(KERN_ERR "FWNMI: nmi-interlock failed: %d\n", ret);
 }
arch/powerpc/xmon/xmon.c +1 -1
index fd72753e8ad5..6f53e8bccc33 100644
@@ -410,7 +410,7 @@ static inline void disable_surveillance(void)
 	if (set_indicator_token == RTAS_UNKNOWN_SERVICE)
 		return;
 
-	rtas_call_unlocked(&args, set_indicator_token, 3, 1, NULL,
+	raw_rtas_call(&args, set_indicator_token, 3, 1, NULL,
 			   SURVEILLANCE_TOKEN, 0, 0);
 
 #endif /* CONFIG_PPC_PSERIES */
Signature · 1 line
2.23.0
Reviewed-by on v1 10/14
#18 lore raw

Re: [PATCH 10/14] powerpc/rtas: replace rtas_call_unlocked with raw_rtas_call

Quoted 5 lines · Nicholas Piggin

Use the same calling and rets return convention with the raw rtas call rather than requiring callers to load and byteswap return values out of rtas_args.

Despite a minor comment below

Quoted 13 lines
---
 arch/powerpc/include/asm/rtas.h              |  4 +-
 arch/powerpc/kernel/rtas.c                   | 53 +++++++++++---------
 arch/powerpc/platforms/pseries/hotplug-cpu.c |  2 +-
 arch/powerpc/platforms/pseries/ras.c         |  7 +--
 arch/powerpc/xmon/xmon.c                     |  2 +-
 5 files changed, 33 insertions(+), 35 deletions(-)
arch/powerpc/include/asm/rtas.h +0 -0
index 82e5b055fa2a..1014ff140cf8 100644

arch/powerpc/include/asm/rtas.h extern int rtas_token(const char *service); · line 245

Quoted 7 lines
arch/powerpc/include/asm/rtas.h +2 -2
@@ -241,8 +241,8 @@ extern int rtas_token(const char *service);
 extern int rtas_service_present(const char *service);
 extern int rtas_call(int token, int, int, int *, ...);
 int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...);
-void rtas_call_unlocked(struct rtas_args *args, int token, int nargs,
-			int nret, ...);
+int raw_rtas_call(struct rtas_args *args, int token,
+			int nargs, int nret, int *outputs, ...);

Minor, would it be better to keep the "unlocked" suffix to advise that the rtas lock is not held here?

Quoted 165 lines
arch/powerpc/include/asm/rtas.h +0 -0
 extern void __noreturn rtas_restart(char *cmd);
 extern void rtas_power_off(void);
 extern void __noreturn rtas_halt(void);
arch/powerpc/kernel/rtas.c +28 -25
index fece066115f0..751a20669669 100644
@@ -123,7 +123,7 @@ static void call_rtas_display_status(unsigned char c)
 		return;
 
 	s = lock_rtas();
-	rtas_call_unlocked(&rtas.args, 10, 1, 1, NULL, c);
+	raw_rtas_call(&rtas.args, 10, 1, 1, NULL, c);
 	unlock_rtas(s);
 }
 
@@ -434,10 +434,9 @@ static char *__fetch_rtas_last_error(char *altbuf)
 #define get_errorlog_buffer()		NULL
 #endif
 
-
-static void
-va_rtas_call_unlocked(struct rtas_args *args, int token, int nargs, int nret,
-		      va_list list)
+static int notrace va_raw_rtas_call(struct rtas_args *args, int token,
+				int nargs, int nret, int *outputs,
+				va_list list)
 {
 	int i;
 
@@ -453,21 +452,37 @@ va_rtas_call_unlocked(struct rtas_args *args, int token, int nargs, int nret,
 		args->rets[i] = 0;
 
 	do_enter_rtas(__pa(args));
+
+	if (nret > 1 && outputs != NULL) {
+		for (i = 0; i < nret-1; ++i)
+			outputs[i] = be32_to_cpu(args->rets[i+1]);
+	}
+
+	return (nret > 0) ? be32_to_cpu(args->rets[0]) : 0;
 }
 
-void rtas_call_unlocked(struct rtas_args *args, int token, int nargs, int nret, ...)
+/*
+ * Like rtas_call but no kmalloc or printk etc in error handling, so
+ * error won't go through log_error. No tracing, may be called in real mode.
+ * rtas_args must be supplied, and appropriate synchronization for the rtas
+ * call being made has to be performed by the caller.
+ */
+int notrace raw_rtas_call(struct rtas_args *args, int token,
+			int nargs, int nret, int *outputs, ...)
 {
 	va_list list;
+	int ret;
 
-	va_start(list, nret);
-	va_rtas_call_unlocked(args, token, nargs, nret, list);
+	va_start(list, outputs);
+	ret = va_raw_rtas_call(args, token, nargs, nret, outputs, list);
 	va_end(list);
+
+	return ret;
 }
 
 int rtas_call(int token, int nargs, int nret, int *outputs, ...)
 {
 	va_list list;
-	int i;
 	unsigned long s;
 	struct rtas_args *rtas_args;
 	char *buff_copy = NULL;
@@ -482,19 +497,14 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...)
 	rtas_args = &rtas.args;
 
 	va_start(list, outputs);
-	va_rtas_call_unlocked(rtas_args, token, nargs, nret, list);
+	ret = va_raw_rtas_call(rtas_args, token, nargs, nret, outputs, list);
 	va_end(list);
 
 	/* A -1 return code indicates that the last command couldn't
 	   be completed due to a hardware error. */
-	if (be32_to_cpu(rtas_args->rets[0]) == -1)
+	if (ret == -1)
 		buff_copy = __fetch_rtas_last_error(NULL);
 
-	if (nret > 1 && outputs != NULL)
-		for (i = 0; i < nret-1; ++i)
-			outputs[i] = be32_to_cpu(rtas_args->rets[i+1]);
-	ret = (nret > 0)? be32_to_cpu(rtas_args->rets[0]): 0;
-
 	unlock_rtas(s);
 
 	if (buff_copy) {
@@ -950,7 +960,7 @@ int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...)
 	va_list list;
 	struct rtas_args *args;
 	unsigned long flags;
-	int i, ret = 0;
+	int ret;
 
 	if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
 		return -1;
@@ -962,16 +972,9 @@ int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...)
 	args = local_paca->rtas_args_reentrant;
 
 	va_start(list, outputs);
-	va_rtas_call_unlocked(args, token, nargs, nret, list);
+	ret = va_raw_rtas_call(args, token, nargs, nret, outputs, list);
 	va_end(list);
 
-	if (nret > 1 && outputs)
-		for (i = 0; i < nret - 1; ++i)
-			outputs[i] = be32_to_cpu(args->rets[i + 1]);
-
-	if (nret > 0)
-		ret = be32_to_cpu(args->rets[0]);
-
 	local_irq_restore(flags);
 	preempt_enable();
 
arch/powerpc/platforms/pseries/hotplug-cpu.c +1 -1
index b81fc846d99c..17c05650b6b9 100644
@@ -53,7 +53,7 @@ static void rtas_stop_self(void)
 
 	BUG_ON(rtas_stop_self_token == RTAS_UNKNOWN_SERVICE);
 
-	rtas_call_unlocked(&args, rtas_stop_self_token, 0, 1, NULL);
+	raw_rtas_call(&args, rtas_stop_self_token, 0, 1, NULL);
 
 	panic("Alas, I survived.\n");
 }
arch/powerpc/platforms/pseries/ras.c +1 -6
index 74c9b1b5bc66..b009ed7de1ee 100644
@@ -465,12 +465,7 @@ static void fwnmi_release_errinfo(void)
 	struct rtas_args rtas_args;
 	int ret;
 
-	/*
-	 * On pseries, the machine check stack is limited to under 4GB, so
-	 * args can be on-stack.
-	 */
-	rtas_call_unlocked(&rtas_args, ibm_nmi_interlock_token, 0, 1, NULL);
-	ret = be32_to_cpu(rtas_args.rets[0]);
+	ret = raw_rtas_call(&rtas_args, ibm_nmi_interlock_token, 0, 1, NULL);
 	if (ret != 0)
 		printk(KERN_ERR "FWNMI: nmi-interlock failed: %d\n", ret);
 }
arch/powerpc/xmon/xmon.c +1 -1
index fd72753e8ad5..6f53e8bccc33 100644
@@ -410,7 +410,7 @@ static inline void disable_surveillance(void)
 	if (set_indicator_token == RTAS_UNKNOWN_SERVICE)
 		return;
 
-	rtas_call_unlocked(&args, set_indicator_token, 3, 1, NULL,
+	raw_rtas_call(&args, set_indicator_token, 3, 1, NULL,
 			   SURVEILLANCE_TOKEN, 0, 0);
 
 #endif /* CONFIG_PPC_PSERIES */
author reviewer
#19 lore raw

PATCH 11/14 powerpc/rtas: tidy __fetch_rtas_last_error

In reply to Nicholas Piggin

__fetch_rtas_last_error can use the same rtas_args as the caller used for the failed rtas call. It can also use a higher-level helper to assemble the rtas_args.

---
 arch/powerpc/kernel/rtas.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)
arch/powerpc/kernel/rtas.c +9 -21
index 751a20669669..e047793cbb80 100644
@@ -385,34 +385,22 @@ static int rtas_last_error_token;
  *  this routine must be called atomically with whatever produced
  *  the error (i.e. with rtas.lock still held from the previous call).
  */
-static char *__fetch_rtas_last_error(char *altbuf)
+static char *__fetch_rtas_last_error(struct rtas_args *args, char *altbuf)
 {
-	struct rtas_args err_args, save_args;
 	u32 bufsz;
 	char *buf = NULL;
+	int ret;
 
 	if (rtas_last_error_token == -1)
 		return NULL;
 
 	bufsz = rtas_get_error_log_max();
 
-	err_args.token = cpu_to_be32(rtas_last_error_token);
-	err_args.nargs = cpu_to_be32(2);
-	err_args.nret = cpu_to_be32(1);
-	err_args.args[0] = cpu_to_be32(__pa(rtas_err_buf));
-	err_args.args[1] = cpu_to_be32(bufsz);
-	err_args.args[2] = 0;
-
-	save_args = rtas.args;
-	rtas.args = err_args;
-
-	do_enter_rtas(__pa(&rtas.args));
-
-	err_args = rtas.args;
-	rtas.args = save_args;
+	ret = raw_rtas_call(args, rtas_last_error_token, 2, 1, NULL,
+				__pa(rtas_err_buf), bufsz);
 
 	/* Log the error in the unlikely case that there was one. */
-	if (unlikely(err_args.args[2] == 0)) {
+	if (unlikely(ret == 0)) {
 		if (altbuf) {
 			buf = altbuf;
 		} else {
@@ -430,8 +418,8 @@ static char *__fetch_rtas_last_error(char *altbuf)
 #define get_errorlog_buffer()	kmalloc(RTAS_ERROR_LOG_MAX, GFP_KERNEL)
 
 #else /* CONFIG_RTAS_ERROR_LOGGING */
-#define __fetch_rtas_last_error(x)	NULL
-#define get_errorlog_buffer()		NULL
+#define __fetch_rtas_last_error(args, x)	NULL
+#define get_errorlog_buffer()			NULL
 #endif
 
 static int notrace va_raw_rtas_call(struct rtas_args *args, int token,
@@ -503,7 +491,7 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...)
 	/* A -1 return code indicates that the last command couldn't
 	   be completed due to a hardware error. */
 	if (ret == -1)
-		buff_copy = __fetch_rtas_last_error(NULL);
+		buff_copy = __fetch_rtas_last_error(rtas_args, NULL);
 
 	unlock_rtas(s);
 
@@ -1247,7 +1235,7 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
 	/* A -1 return code indicates that the last command couldn't
 	   be completed due to a hardware error. */
 	if (be32_to_cpu(args.rets[0]) == -1)
-		errbuf = __fetch_rtas_last_error(buff_copy);
+		errbuf = __fetch_rtas_last_error(&rtas.args, buff_copy);
 
 	unlock_rtas(flags);
 
Signature · 1 line
2.23.0
author reviewer
#20 lore raw

PATCH 12/14 powerpc/rtas: Close theoretical memory leak

In reply to Nicholas Piggin

If buff_copy allocation failed then there was an error and the errbuf allocation succeeded, it will not be logged or freed.

---
 arch/powerpc/kernel/rtas.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
arch/powerpc/kernel/rtas.c +4 -3
index e047793cbb80..1fc22138e3ab 100644
@@ -1239,9 +1239,10 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
 
 	unlock_rtas(flags);
 
-	if (buff_copy) {
-		if (errbuf)
-			log_error(errbuf, ERR_TYPE_RTAS_LOG, 0);
+	if (errbuf) {
+		log_error(errbuf, ERR_TYPE_RTAS_LOG, 0);
+		kfree(errbuf);
+	} else if (buff_copy) {
 		kfree(buff_copy);
 	}
 
Signature · 1 line
2.23.0
#21 lore raw

Re: [PATCH 12/14] powerpc/rtas: Close theoretical memory leak

Quoted 2 lines · Nicholas Piggin

If buff_copy allocation failed then there was an error and the errbuf allocation succeeded, it will not be logged or freed.

Good catch!

Since you're dealing with the error log buffer allocation/free, I think it would be better to not make this allocation in __fetch_rtas_last_error() and to rely on the caller to allocate it before taking the rtas lock.

This way, the allocation is done without holding the rtas lock, as done in rtas().

This would simplify __fetch_rtas_last_error() and the caller logic to free the buffer too.

Laurent.

Quoted 23 lines
---
 arch/powerpc/kernel/rtas.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
arch/powerpc/kernel/rtas.c +4 -3
index e047793cbb80..1fc22138e3ab 100644
@@ -1239,9 +1239,10 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
 
 	unlock_rtas(flags);
 
-	if (buff_copy) {
-		if (errbuf)
-			log_error(errbuf, ERR_TYPE_RTAS_LOG, 0);
+	if (errbuf) {
+		log_error(errbuf, ERR_TYPE_RTAS_LOG, 0);
+		kfree(errbuf);
+	} else if (buff_copy) {
 		kfree(buff_copy);
 	}
 
author reviewer
#22 lore raw

PATCH 13/14 powerpc/rtas: enture rtas_call is called with MMU enabled

In reply to Nicholas Piggin

rtas_call must not be called with the MMU disabled because in case of rtas error, log_error is called which requires MMU enabled. Add a test and warning for this.

---
 arch/powerpc/kernel/rtas.c | 5 +++++
 1 file changed, 5 insertions(+)
arch/powerpc/kernel/rtas.c +5 -0
index 1fc22138e3ab..adf4892aeecd 100644
@@ -479,6 +479,11 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...)
 	if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
 		return -1;
 
+	if ((mfmsr() & (MSR_IR|MSR_DR)) != (MSR_IR|MSR_DR)) {
+		WARN_ON_ONCE(1);
+		return -1;
+	}
+
 	s = lock_rtas();
 
 	/* We use the global rtas args buffer */
Signature · 1 line
2.23.0
Reviewed-by on v1 13/14
#23 lore raw

Re: [PATCH 13/14] powerpc/rtas: enture rtas_call is called with MMU enabled

Quoted 5 lines · Nicholas Piggin

rtas_call must not be called with the MMU disabled because in case of rtas error, log_error is called which requires MMU enabled. Add a test and warning for this.

Quoted 20 lines
---
 arch/powerpc/kernel/rtas.c | 5 +++++
 1 file changed, 5 insertions(+)
arch/powerpc/kernel/rtas.c +5 -0
index 1fc22138e3ab..adf4892aeecd 100644
@@ -479,6 +479,11 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...)
 	if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
 		return -1;
 
+	if ((mfmsr() & (MSR_IR|MSR_DR)) != (MSR_IR|MSR_DR)) {
+		WARN_ON_ONCE(1);
+		return -1;
+	}
+
 	s = lock_rtas();
 
 	/* We use the global rtas args buffer */
author reviewer
#24 lore raw

PATCH 14/14 powerpc/rtas: Consolidate and improve checking for rtas callers

In reply to Nicholas Piggin

Add range checking from the rtas syscall, and other error checks and warnings to kernel callers, so problems can be found and fixed.

---
 arch/powerpc/kernel/rtas.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)
arch/powerpc/kernel/rtas.c +17 -6
index adf4892aeecd..7f8a3fd685f9 100644
@@ -428,6 +428,23 @@ static int notrace va_raw_rtas_call(struct rtas_args *args, int token,
 {
 	int i;
 
+	if (!irqs_disabled()) {
+		WARN_ON_ONCE(1);
+		return -1;
+	}
+
+	if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE) {
+		WARN_ON_ONCE(1);
+		return -1;
+	}
+
+	if (nargs >= ARRAY_SIZE(args->args)
+	    || nret > ARRAY_SIZE(args->args)
+	    || nargs + nret > ARRAY_SIZE(args->args)) {
+		WARN_ON_ONCE(1);
+		return -1;
+	}
+
 	args->token = cpu_to_be32(token);
 	args->nargs = cpu_to_be32(nargs);
 	args->nret  = cpu_to_be32(nret);
@@ -476,9 +493,6 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...)
 	char *buff_copy = NULL;
 	int ret;
 
-	if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
-		return -1;
-
 	if ((mfmsr() & (MSR_IR|MSR_DR)) != (MSR_IR|MSR_DR)) {
 		WARN_ON_ONCE(1);
 		return -1;
@@ -955,9 +969,6 @@ int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...)
 	unsigned long flags;
 	int ret;
 
-	if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
-		return -1;
-
 	local_irq_save(flags);
 	preempt_disable();
 
Signature · 1 line
2.23.0
#25 lore raw

Re: [PATCH 14/14] powerpc/rtas: Consolidate and improve checking for rtas callers

Quoted 23 lines · Nicholas Piggin

Add range checking from the rtas syscall, and other error checks and warnings to kernel callers, so problems can be found and fixed.

---
 arch/powerpc/kernel/rtas.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)
arch/powerpc/kernel/rtas.c +6 -0
index adf4892aeecd..7f8a3fd685f9 100644
@@ -428,6 +428,23 @@ static int notrace va_raw_rtas_call(struct rtas_args *args, int token,
 {
 	int i;
 
+	if (!irqs_disabled()) {
+		WARN_ON_ONCE(1);
+		return -1;
+	}
+
+	if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE) {

arch/powerpc/kernel/rtas.c static int notrace va_raw_rtas_call(struct rtas_args *args, int token, · line 446

Quoted 10 lines
arch/powerpc/kernel/rtas.c +10 -0
+		WARN_ON_ONCE(1);
+		return -1;
+	}
+
+	if (nargs >= ARRAY_SIZE(args->args)
+	    || nret > ARRAY_SIZE(args->args)
+	    || nargs + nret > ARRAY_SIZE(args->args)) {
+		WARN_ON_ONCE(1);
+		return -1;
+	}

These 3 tests are making the function returning -1, which is previously only returned in the case the call cannot be achieved because of a hardware error (as stated in rtas_call()).

Should a dedicated error code been returned here?

Quoted 24 lines
arch/powerpc/kernel/rtas.c +1 -6
+
 	args->token = cpu_to_be32(token);
 	args->nargs = cpu_to_be32(nargs);
 	args->nret  = cpu_to_be32(nret);
@@ -476,9 +493,6 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...)
 	char *buff_copy = NULL;
 	int ret;
 
-	if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
-		return -1;
-
 	if ((mfmsr() & (MSR_IR|MSR_DR)) != (MSR_IR|MSR_DR)) {
 		WARN_ON_ONCE(1);
 		return -1;
@@ -955,9 +969,6 @@ int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...)
 	unsigned long flags;
 	int ret;
 
-	if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
-		return -1;
-
 	local_irq_save(flags);
 	preempt_disable();
 
#26 lore raw
In reply to Nicholas Piggin

Hi Nick,

Le 08/03/2022 à 14:50, Nicholas Piggin a écrit :

Quoted 11 lines

I had a bunch of random little fixes and cleanups around and was prompted to put them together and make a change to call RTAS with MSR[RI] enabled because of a report of the hard lockup watchdog NMI IPI hitting in an rtas call which then crashed because it's unrecoverable.

Could possibly move patch 9 earlier if it would help with backporting.

Thanks,
Nick

While you are at cleaning RTAS, maybe you could add to your series the two following patches, unless they are not applicable anymore ?

https://patchwork.ozlabs.org/project/linuxppc-dev/patch/[email protected]/

https://patchwork.ozlabs.org/project/linuxppc-dev/patch/[email protected]/

Christophe

Quoted 29 lines

Nicholas Piggin (14):
powerpc/rtas: Move rtas entry assembly into its own file powerpc/rtas: Make enter_rtas a nokprobe symbol on 64-bit powerpc/rtas: Fix whitespace in rtas_entry.S powerpc/rtas: Call enter_rtas with MSR[EE] disabled powerpc/rtas: Modernise RI clearing on 64-bit powerpc/rtas: Load rtas entry MSR explicitly powerpc/rtas: PACA can be restored directly from SPRG powerpc/rtas: call enter_rtas in real-mode on 64-bit powerpc/rtas: Leave MSR[RI] enabled over RTAS call powerpc/rtas: replace rtas_call_unlocked with raw_rtas_call powerpc/rtas: tidy __fetch_rtas_last_error powerpc/rtas: Close theoretical memory leak powerpc/rtas: enture rtas_call is called with MMU enabled powerpc/rtas: Consolidate and improve checking for rtas callers

  arch/powerpc/include/asm/rtas.h              |   4 +-
  arch/powerpc/kernel/Makefile                 |   2 +-
  arch/powerpc/kernel/entry_32.S               |  49 ------
  arch/powerpc/kernel/entry_64.S               | 150 -------------------
  arch/powerpc/kernel/rtas.c                   | 132 +++++++++-------
  arch/powerpc/kernel/rtas_entry.S             | 144 ++++++++++++++++++
  arch/powerpc/platforms/pseries/hotplug-cpu.c |   2 +-
  arch/powerpc/platforms/pseries/ras.c         |   7 +-
  arch/powerpc/xmon/xmon.c                     |   2 +-
  9 files changed, 227 insertions(+), 265 deletions(-)

create mode 100644 arch/powerpc/kernel/rtas_entry.S

#27 lore raw
In reply to Nicholas Piggin

Hi Nick,

As this series needs additional work, I just sent a single patch [1] fixing the MSR[RI] issue addressed in the patch 9/14 of this series.

I did that because that fix is fixing a panic currently seen and this will ease backport to stable and distro kernel.

I suggest rebasing this series on top of this new patch.

Cheers,
Laurent.

1:
https://lore.kernel.org/linuxppc-dev/[email protected]/

Quoted 40 lines · Nicholas Piggin

I had a bunch of random little fixes and cleanups around and was prompted to put them together and make a change to call RTAS with MSR[RI] enabled because of a report of the hard lockup watchdog NMI IPI hitting in an rtas call which then crashed because it's unrecoverable.

Could possibly move patch 9 earlier if it would help with backporting.

Thanks,
Nick

Nicholas Piggin (14):
powerpc/rtas: Move rtas entry assembly into its own file powerpc/rtas: Make enter_rtas a nokprobe symbol on 64-bit powerpc/rtas: Fix whitespace in rtas_entry.S powerpc/rtas: Call enter_rtas with MSR[EE] disabled powerpc/rtas: Modernise RI clearing on 64-bit powerpc/rtas: Load rtas entry MSR explicitly powerpc/rtas: PACA can be restored directly from SPRG powerpc/rtas: call enter_rtas in real-mode on 64-bit powerpc/rtas: Leave MSR[RI] enabled over RTAS call powerpc/rtas: replace rtas_call_unlocked with raw_rtas_call powerpc/rtas: tidy __fetch_rtas_last_error powerpc/rtas: Close theoretical memory leak powerpc/rtas: enture rtas_call is called with MMU enabled powerpc/rtas: Consolidate and improve checking for rtas callers

 arch/powerpc/include/asm/rtas.h              |   4 +-
 arch/powerpc/kernel/Makefile                 |   2 +-
 arch/powerpc/kernel/entry_32.S               |  49 ------
 arch/powerpc/kernel/entry_64.S               | 150 -------------------
 arch/powerpc/kernel/rtas.c                   | 132 +++++++++-------
 arch/powerpc/kernel/rtas_entry.S             | 144 ++++++++++++++++++
 arch/powerpc/platforms/pseries/hotplug-cpu.c |   2 +-
 arch/powerpc/platforms/pseries/ras.c         |   7 +-
 arch/powerpc/xmon/xmon.c                     |   2 +-
 9 files changed, 227 insertions(+), 265 deletions(-)
 create mode 100644 arch/powerpc/kernel/rtas_entry.S
#28 lore raw
In reply to Nicholas Piggin
Quoted 10 lines · Nicholas Piggin

I had a bunch of random little fixes and cleanups around and was prompted to put them together and make a change to call RTAS with MSR[RI] enabled because of a report of the hard lockup watchdog NMI IPI hitting in an rtas call which then crashed because it's unrecoverable.

Could possibly move patch 9 earlier if it would help with backporting.

[...]

Patches 1-4, 7, 9 & 13 applied to powerpc/next.

[01/14] powerpc/rtas: Move rtas entry assembly into its own file

        https://git.kernel.org/powerpc/c/838ee286ecc9a3c76e6bd8f5aaad0c8c5c66b9ca
[02/14] powerpc/rtas: Make enter_rtas a nokprobe symbol on 64-bit
        https://git.kernel.org/powerpc/c/07940b4b61cf0cbcfb9e4226c07318f737157c42
[03/14] powerpc/rtas: Fix whitespace in rtas_entry.S
        https://git.kernel.org/powerpc/c/4e949faae2bd42783a2b2b732b7bf17557d94cfb
[04/14] powerpc/rtas: Call enter_rtas with MSR[EE] disabled
        https://git.kernel.org/powerpc/c/c5a65e0a420d50655bf692fc7386813683c0cd81
[07/14] powerpc/rtas: PACA can be restored directly from SPRG
        https://git.kernel.org/powerpc/c/5c86bd02b3c3ef68a109fa7e690ad62d3091f6d4
[09/14] powerpc/rtas: Leave MSR[RI] enabled over RTAS call
        https://git.kernel.org/powerpc/c/014b2e896cc8445fcc04636e69bf5f9e24281daa
[13/14] powerpc/rtas: enture rtas_call is called with MMU enabled
        https://git.kernel.org/powerpc/c/804c0a166ffea628eb7ef72b9fd710883cb1fa8f

cheers