[PATCH 00/14] powerpc/rtas: various cleanups and improvements

[PATCH 00/14] powerpc/rtas: various cleanups and improvements

[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

-- 
2.23.0

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

[Nicholas Piggin]

This makes working on the code a bit easier.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 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

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 4d7829399570..13602b0f0472 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -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
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 7748c278d13c..1d599df6f169 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -555,52 +555,3 @@ ret_from_mcheck_exc:
 _ASM_NOKPROBE_SYMBOL(ret_from_mcheck_exc)
 #endif /* CONFIG_BOOKE */
 #endif /* !(CONFIG_4xx || CONFIG_BOOKE) */
-
-/*
- * PROM code for specific machines follows.  Put it
- * here so it's easy to add arch-specific sections later.
- * -- Cort
- */
-#ifdef CONFIG_PPC_RTAS
-/*
- * On CHRP, the Run-Time Abstraction Services (RTAS) have to be
- * called with the MMU off.
- */
-_GLOBAL(enter_rtas)
-	stwu	r1,-INT_FRAME_SIZE(r1)
-	mflr	r0
-	stw	r0,INT_FRAME_SIZE+4(r1)
-	LOAD_REG_ADDR(r4, rtas)
-	lis	r6,1f@ha	/* physical return address for rtas */
-	addi	r6,r6,1f@l
-	tophys(r6,r6)
-	lwz	r8,RTASENTRY(r4)
-	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)
-	mtspr	SPRN_SRR0,r8
-	mtspr	SPRN_SRR1,r9
-	rfi
-1:
-	lis	r8, 1f@h
-	ori	r8, r8, 1f@l
-	LOAD_REG_IMMEDIATE(r9,MSR_KERNEL)
-	mtspr	SPRN_SRR0,r8
-	mtspr	SPRN_SRR1,r9
-	rfi			/* Reactivate MMU translation */
-1:
-	lwz	r8,INT_FRAME_SIZE+4(r1)	/* get return address */
-	lwz	r9,8(r1)	/* original msr value */
-	addi	r1,r1,INT_FRAME_SIZE
-	li	r0,0
-	stw	r0, THREAD + RTAS_SP(r2)
-	mtlr	r8
-	mtmsr	r9
-	blr			/* return to caller */
-_ASM_NOKPROBE_SYMBOL(enter_rtas)
-#endif /* CONFIG_PPC_RTAS */
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 9581906b5ee9..01ace4c56104 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -264,156 +264,6 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
 	addi	r1,r1,SWITCH_FRAME_SIZE
 	blr
 
-#ifdef CONFIG_PPC_RTAS
-/*
- * On CHRP, the Run-Time Abstraction Services (RTAS) have to be
- * called with the MMU off.
- *
- * In addition, we need to be in 32b mode, at least for now.
- * 
- * Note: r3 is an input parameter to rtas, so don't trash it...
- */
-_GLOBAL(enter_rtas)
-	mflr	r0
-	std	r0,16(r1)
-        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 */
-
-	mfcr	r4
-	std	r4,_CCR(r1)
-	mfctr	r5
-	std	r5,_CTR(r1)
-	mfspr	r6,SPRN_XER
-	std	r6,_XER(r1)
-	mfdar	r7
-	std	r7,_DAR(r1)
-	mfdsisr	r8
-	std	r8,_DSISR(r1)
-
-	/* Temporary workaround to clear CR until RTAS can be modified to
-	 * ignore all bits.
-	 */
-	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
-	 * 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)
-	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
-
-__enter_rtas:
-	sync				/* disable interrupts 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 */
-	
-	mtspr	SPRN_SRR0,r5
-	mtspr	SPRN_SRR1,r6
-	RFI_TO_KERNEL
-	b	.	/* prevent speculative execution */
-
-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
-	mtmsrd  r6
-
-	/* relocation is off at this point */
-	GET_PACA(r4)
-	clrldi	r4,r4,2			/* convert to realmode address */
-
-	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 */
-
-	mtspr	SPRN_SRR0,r3
-	mtspr	SPRN_SRR1,r4
-	RFI_TO_KERNEL
-	b	.	/* prevent speculative execution */
-_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_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)
-	mtctr	r5
-	ld	r6,_XER(r1)
-	mtspr	SPRN_XER,r6
-	ld	r7,_DAR(r1)
-	mtdar	r7
-	ld	r8,_DSISR(r1)
-	mtdsisr	r8
-
-        addi	r1,r1,SWITCH_FRAME_SIZE	/* Unstack our frame */
-	ld	r0,16(r1)		/* get return address */
-
-	mtlr    r0
-        blr				/* return to caller */
-
-#endif /* CONFIG_PPC_RTAS */
-
 _GLOBAL(enter_prom)
 	mflr	r0
 	std	r0,16(r1)
diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
new file mode 100644
index 000000000000..192fea342744
--- /dev/null
+++ b/arch/powerpc/kernel/rtas_entry.S
@@ -0,0 +1,199 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include <asm/asm-offsets.h>
+#include <asm/bug.h>
+#include <asm/page.h>
+#include <asm/ppc_asm.h>
+
+/*
+ * RTAS is called with MSR IR, DR, EE disabled, and LR in the return address.
+ *
+ * Note: r3 is an input parameter to rtas, so don't trash it...
+ */
+
+#ifdef CONFIG_PPC32
+_GLOBAL(enter_rtas)
+	stwu	r1,-INT_FRAME_SIZE(r1)
+	mflr	r0
+	stw	r0,INT_FRAME_SIZE+4(r1)
+	LOAD_REG_ADDR(r4, rtas)
+	lis	r6,1f@ha	/* physical return address for rtas */
+	addi	r6,r6,1f@l
+	tophys(r6,r6)
+	lwz	r8,RTASENTRY(r4)
+	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)
+	mtspr	SPRN_SRR0,r8
+	mtspr	SPRN_SRR1,r9
+	rfi
+1:
+	lis	r8, 1f@h
+	ori	r8, r8, 1f@l
+	LOAD_REG_IMMEDIATE(r9,MSR_KERNEL)
+	mtspr	SPRN_SRR0,r8
+	mtspr	SPRN_SRR1,r9
+	rfi			/* Reactivate MMU translation */
+1:
+	lwz	r8,INT_FRAME_SIZE+4(r1)	/* get return address */
+	lwz	r9,8(r1)	/* original msr value */
+	addi	r1,r1,INT_FRAME_SIZE
+	li	r0,0
+	stw	r0, THREAD + RTAS_SP(r2)
+	mtlr	r8
+	mtmsr	r9
+	blr			/* return to caller */
+_ASM_NOKPROBE_SYMBOL(enter_rtas)
+
+#else /* CONFIG_PPC32 */
+#include <asm/exception-64s.h>
+
+/*
+ * 32-bit rtas on 64-bit machines has the additional problem that RTAS may
+ * not preserve the upper parts of registers it uses.
+ */
+_GLOBAL(enter_rtas)
+	mflr	r0
+	std	r0,16(r1)
+        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 */
+
+	mfcr	r4
+	std	r4,_CCR(r1)
+	mfctr	r5
+	std	r5,_CTR(r1)
+	mfspr	r6,SPRN_XER
+	std	r6,_XER(r1)
+	mfdar	r7
+	std	r7,_DAR(r1)
+	mfdsisr	r8
+	std	r8,_DSISR(r1)
+
+	/* Temporary workaround to clear CR until RTAS can be modified to
+	 * ignore all bits.
+	 */
+	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
+	 * 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)
+	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
+
+__enter_rtas:
+	sync				/* disable interrupts 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 */
+	
+	mtspr	SPRN_SRR0,r5
+	mtspr	SPRN_SRR1,r6
+	RFI_TO_KERNEL
+	b	.	/* prevent speculative execution */
+
+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
+	mtmsrd  r6
+
+	/* relocation is off at this point */
+	GET_PACA(r4)
+	clrldi	r4,r4,2			/* convert to realmode address */
+
+	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 */
+
+	mtspr	SPRN_SRR0,r3
+	mtspr	SPRN_SRR1,r4
+	RFI_TO_KERNEL
+	b	.	/* prevent speculative execution */
+_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_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)
+	mtctr	r5
+	ld	r6,_XER(r1)
+	mtspr	SPRN_XER,r6
+	ld	r7,_DAR(r1)
+	mtdar	r7
+	ld	r8,_DSISR(r1)
+	mtdsisr	r8
+
+        addi	r1,r1,SWITCH_FRAME_SIZE	/* Unstack our frame */
+	ld	r0,16(r1)		/* get return address */
+
+	mtlr    r0
+        blr				/* return to caller */
+
+#endif /* CONFIG_PPC32 */
-- 
2.23.0

[PATCH 02/14] powerpc/rtas: Make enter_rtas a nokprobe symbol on 64-bit

[Nicholas Piggin]

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

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/rtas_entry.S | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
index 192fea342744..afe3b789bc36 100644
--- a/arch/powerpc/kernel/rtas_entry.S
+++ b/arch/powerpc/kernel/rtas_entry.S
@@ -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)
 
-- 
2.23.0

[PATCH 03/14] powerpc/rtas: Fix whitespace in rtas_entry.S

[Nicholas Piggin]

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

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/rtas_entry.S | 34 ++++++++++++++++----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
index afe3b789bc36..6fa10eb49a9c 100644
--- a/arch/powerpc/kernel/rtas_entry.S
+++ b/arch/powerpc/kernel/rtas_entry.S
@@ -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 */
-- 
2.23.0

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

[Nicholas Piggin]

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

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/rtas.c       |  4 ++++
 arch/powerpc/kernel/rtas_entry.S | 17 +----------------
 2 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 733e6ef36758..6b5892d6a56b 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -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 */
diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
index 6fa10eb49a9c..45fa661c2ff6 100644
--- a/arch/powerpc/kernel/rtas_entry.S
+++ b/arch/powerpc/kernel/rtas_entry.S
@@ -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)
-- 
2.23.0

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

[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.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/rtas_entry.S | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
index 45fa661c2ff6..7b93687b9a10 100644
--- a/arch/powerpc/kernel/rtas_entry.S
+++ b/arch/powerpc/kernel/rtas_entry.S
@@ -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
-- 
2.23.0

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

[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.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/rtas_entry.S | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
index 7b93687b9a10..08eb731f08b8 100644
--- a/arch/powerpc/kernel/rtas_entry.S
+++ b/arch/powerpc/kernel/rtas_entry.S
@@ -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)
-- 
2.23.0

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

[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.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/rtas_entry.S | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
index 08eb731f08b8..5f65ea4436c6 100644
--- a/arch/powerpc/kernel/rtas_entry.S
+++ b/arch/powerpc/kernel/rtas_entry.S
@@ -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)
-- 
2.23.0

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

[Nicholas Piggin]

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

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/rtas.c       | 15 ++++++++++++---
 arch/powerpc/kernel/rtas_entry.S | 32 +++++---------------------------
 2 files changed, 17 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 6b5892d6a56b..87ede1877816 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -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 */
 }
diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
index 5f65ea4436c6..292551684bbd 100644
--- a/arch/powerpc/kernel/rtas_entry.S
+++ b/arch/powerpc/kernel/rtas_entry.S
@@ -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 */
-- 
2.23.0

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

[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.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/rtas.c       |  1 +
 arch/powerpc/kernel/rtas_entry.S | 13 +++----------
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 87ede1877816..fece066115f0 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -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);
diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
index 292551684bbd..72b27b14ccc9 100644
--- a/arch/powerpc/kernel/rtas_entry.S
+++ b/arch/powerpc/kernel/rtas_entry.S
@@ -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)
-- 
2.23.0

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

[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.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 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(-)

diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 82e5b055fa2a..1014ff140cf8 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -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);
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index fece066115f0..751a20669669 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -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();
 
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index b81fc846d99c..17c05650b6b9 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -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");
 }
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 74c9b1b5bc66..b009ed7de1ee 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -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);
 }
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index fd72753e8ad5..6f53e8bccc33 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -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 */
-- 
2.23.0

[PATCH 11/14] powerpc/rtas: tidy __fetch_rtas_last_error

[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.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/rtas.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 751a20669669..e047793cbb80 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -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);
 
-- 
2.23.0

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

[Nicholas Piggin]

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

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/rtas.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index e047793cbb80..1fc22138e3ab 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -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);
 	}
 
-- 
2.23.0

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

[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.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/rtas.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 1fc22138e3ab..adf4892aeecd 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -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 */
-- 
2.23.0

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

[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.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/rtas.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index adf4892aeecd..7f8a3fd685f9 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -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();
 
-- 
2.23.0

Re: [PATCH 00/14] powerpc/rtas: various cleanups and improvements

[Christophe Leroy]

Hi Nick,

Le 08/03/2022 à 14:50, Nicholas Piggin a écrit :
> 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/20210714122753.76021-1-zhuangyi1@huawei.com/

https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210526092020.554341-1-chenhuang5@huawei.com/

Christophe

> 
> 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
> 

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

[Laurent Dufour]

On 08/03/2022, 14:50:37, Nicholas Piggin wrote:
> Disable MSR[EE] in C code rather than asm.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

FWIW,
Reviewed-by: Laurent Dufour <ldufour@linux.ibm.com>

> ---
>  arch/powerpc/kernel/rtas.c       |  4 ++++
>  arch/powerpc/kernel/rtas_entry.S | 17 +----------------
>  2 files changed, 5 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 733e6ef36758..6b5892d6a56b 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -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 */
> diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
> index 6fa10eb49a9c..45fa661c2ff6 100644
> --- a/arch/powerpc/kernel/rtas_entry.S
> +++ b/arch/powerpc/kernel/rtas_entry.S
> @@ -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)

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

[Laurent Dufour]

On 08/03/2022, 14:50:38, Nicholas Piggin wrote:
> 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.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Laurent Dufour <ldufour@linux.ibm.com>

> ---
>  arch/powerpc/kernel/rtas_entry.S | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
> index 45fa661c2ff6..7b93687b9a10 100644
> --- a/arch/powerpc/kernel/rtas_entry.S
> +++ b/arch/powerpc/kernel/rtas_entry.S
> @@ -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

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

[Laurent Dufour]

On 08/03/2022, 14:50:39, Nicholas Piggin wrote:
> 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.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Laurent Dufour <ldufour@linux.ibm.com>

> ---
>  arch/powerpc/kernel/rtas_entry.S | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
> index 7b93687b9a10..08eb731f08b8 100644
> --- a/arch/powerpc/kernel/rtas_entry.S
> +++ b/arch/powerpc/kernel/rtas_entry.S
> @@ -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)

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

[Laurent Dufour]

On 08/03/2022, 14:50:40, Nicholas Piggin wrote:
> 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.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Laurent Dufour <ldufour@linux.ibm.com>

> ---
>  arch/powerpc/kernel/rtas_entry.S | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
> index 08eb731f08b8..5f65ea4436c6 100644
> --- a/arch/powerpc/kernel/rtas_entry.S
> +++ b/arch/powerpc/kernel/rtas_entry.S
> @@ -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)

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

[Laurent Dufour]

On 08/03/2022, 14:50:41, Nicholas Piggin wrote:
> This moves MSR save/restore and some real-mode juggling out of asm and
> into C code, simplifying things.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/kernel/rtas.c       | 15 ++++++++++++---
>  arch/powerpc/kernel/rtas_entry.S | 32 +++++---------------------------
>  2 files changed, 17 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 6b5892d6a56b..87ede1877816 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -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 */
>  }
> diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
> index 5f65ea4436c6..292551684bbd 100644
> --- a/arch/powerpc/kernel/rtas_entry.S
> +++ b/arch/powerpc/kernel/rtas_entry.S
> @@ -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

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?

>  
> -	/* 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

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

> -	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 */

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

[Laurent Dufour]

On 08/03/2022, 14:50:42, Nicholas Piggin wrote:
> 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.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/kernel/rtas.c       |  1 +
>  arch/powerpc/kernel/rtas_entry.S | 13 +++----------
>  2 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 87ede1877816..fece066115f0 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -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);
> diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
> index 292551684bbd..72b27b14ccc9 100644
> --- a/arch/powerpc/kernel/rtas_entry.S
> +++ b/arch/powerpc/kernel/rtas_entry.S
> @@ -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))

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

>  	mtmsrd	r6
>  
>  	GET_PACA(r13)

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

[Laurent Dufour]

On 08/03/2022, 14:50:43, Nicholas Piggin wrote:
> 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.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Despite a minor comment below
Reviewed-by: Laurent Dufour <ldufour@linux.ibm.com>

> ---
>  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(-)
> 
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 82e5b055fa2a..1014ff140cf8 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -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?

>  extern void __noreturn rtas_restart(char *cmd);
>  extern void rtas_power_off(void);
>  extern void __noreturn rtas_halt(void);
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index fece066115f0..751a20669669 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -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();
>  
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index b81fc846d99c..17c05650b6b9 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -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");
>  }
> diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
> index 74c9b1b5bc66..b009ed7de1ee 100644
> --- a/arch/powerpc/platforms/pseries/ras.c
> +++ b/arch/powerpc/platforms/pseries/ras.c
> @@ -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);
>  }
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index fd72753e8ad5..6f53e8bccc33 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -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 */

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

[Laurent Dufour]

On 08/03/2022, 14:50:45, Nicholas Piggin wrote:
> 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.

> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/kernel/rtas.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index e047793cbb80..1fc22138e3ab 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -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);
>  	}
>  

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

[Laurent Dufour]

On 08/03/2022, 14:50:46, Nicholas Piggin wrote:
> 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.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Laurent Dufour <ldufour@linux.ibm.com>

> ---
>  arch/powerpc/kernel/rtas.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 1fc22138e3ab..adf4892aeecd 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -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 */

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

[Laurent Dufour]

On 08/03/2022, 14:50:47, Nicholas Piggin wrote:
> Add range checking from the rtas syscall, and other error checks
> and warnings to kernel callers, so problems can be found and
> fixed.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/kernel/rtas.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index adf4892aeecd..7f8a3fd685f9 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -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;
> +	}

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?


> +
>  	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();
>  

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

[Laurent Dufour]

On 08/03/2022, 14:50:41, Nicholas Piggin wrote:
> This moves MSR save/restore and some real-mode juggling out of asm and
> into C code, simplifying things.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/kernel/rtas.c       | 15 ++++++++++++---
>  arch/powerpc/kernel/rtas_entry.S | 32 +++++---------------------------
>  2 files changed, 17 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 6b5892d6a56b..87ede1877816 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -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));

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.

> +		enter_rtas(args);
> +		mtmsr(msr);
> +	} else {
> +		enter_rtas(args);
> +	}
>  
>  	srr_regs_clobbered(); /* rtas uses SRRs, invalidate */
>  }
> diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S
> index 5f65ea4436c6..292551684bbd 100644
> --- a/arch/powerpc/kernel/rtas_entry.S
> +++ b/arch/powerpc/kernel/rtas_entry.S
> @@ -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

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.

>  	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
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.

> -	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 */

Re: [PATCH 00/14] powerpc/rtas: various cleanups and improvements

[Laurent Dufour]

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/20220317110601.86917-1-ldufour@linux.ibm.com/

On 08/03/2022, 14:50:33, Nicholas Piggin wrote:
> 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
> 

Re: [PATCH 00/14] powerpc/rtas: various cleanups and improvements

[Michael Ellerman]

On Tue, 8 Mar 2022 23:50:33 +1000, Nicholas Piggin wrote:
> 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