* [PATCH 01/14] powerpc/rtas: Move rtas entry assembly into its own file
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
@ 2022-03-08 13:50 ` Nicholas Piggin
2022-03-08 13:50 ` [PATCH 02/14] powerpc/rtas: Make enter_rtas a nokprobe symbol on 64-bit Nicholas Piggin
` (15 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Nicholas Piggin @ 2022-03-08 13:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Dufour, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 02/14] powerpc/rtas: Make enter_rtas a nokprobe symbol on 64-bit
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
2022-03-08 13:50 ` [PATCH 01/14] powerpc/rtas: Move rtas entry assembly into its own file Nicholas Piggin
@ 2022-03-08 13:50 ` Nicholas Piggin
2022-03-08 13:50 ` [PATCH 03/14] powerpc/rtas: Fix whitespace in rtas_entry.S Nicholas Piggin
` (14 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Nicholas Piggin @ 2022-03-08 13:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Dufour, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 03/14] powerpc/rtas: Fix whitespace in rtas_entry.S
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
2022-03-08 13:50 ` [PATCH 01/14] powerpc/rtas: Move rtas entry assembly into its own file Nicholas Piggin
2022-03-08 13:50 ` [PATCH 02/14] powerpc/rtas: Make enter_rtas a nokprobe symbol on 64-bit Nicholas Piggin
@ 2022-03-08 13:50 ` Nicholas Piggin
2022-03-08 13:50 ` [PATCH 04/14] powerpc/rtas: Call enter_rtas with MSR[EE] disabled Nicholas Piggin
` (13 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Nicholas Piggin @ 2022-03-08 13:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Dufour, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 04/14] powerpc/rtas: Call enter_rtas with MSR[EE] disabled
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
` (2 preceding siblings ...)
2022-03-08 13:50 ` [PATCH 03/14] powerpc/rtas: Fix whitespace in rtas_entry.S Nicholas Piggin
@ 2022-03-08 13:50 ` Nicholas Piggin
2022-03-14 15:12 ` Laurent Dufour
2022-03-08 13:50 ` [PATCH 05/14] powerpc/rtas: Modernise RI clearing on 64-bit Nicholas Piggin
` (12 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Nicholas Piggin @ 2022-03-08 13:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Dufour, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 04/14] powerpc/rtas: Call enter_rtas with MSR[EE] disabled
2022-03-08 13:50 ` [PATCH 04/14] powerpc/rtas: Call enter_rtas with MSR[EE] disabled Nicholas Piggin
@ 2022-03-14 15:12 ` Laurent Dufour
0 siblings, 0 replies; 29+ messages in thread
From: Laurent Dufour @ 2022-03-14 15:12 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
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)
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 05/14] powerpc/rtas: Modernise RI clearing on 64-bit
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
` (3 preceding siblings ...)
2022-03-08 13:50 ` [PATCH 04/14] powerpc/rtas: Call enter_rtas with MSR[EE] disabled Nicholas Piggin
@ 2022-03-08 13:50 ` Nicholas Piggin
2022-03-14 15:15 ` Laurent Dufour
2022-03-08 13:50 ` [PATCH 06/14] powerpc/rtas: Load rtas entry MSR explicitly Nicholas Piggin
` (11 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Nicholas Piggin @ 2022-03-08 13:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Dufour, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 05/14] powerpc/rtas: Modernise RI clearing on 64-bit
2022-03-08 13:50 ` [PATCH 05/14] powerpc/rtas: Modernise RI clearing on 64-bit Nicholas Piggin
@ 2022-03-14 15:15 ` Laurent Dufour
0 siblings, 0 replies; 29+ messages in thread
From: Laurent Dufour @ 2022-03-14 15:15 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
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
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 06/14] powerpc/rtas: Load rtas entry MSR explicitly
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
` (4 preceding siblings ...)
2022-03-08 13:50 ` [PATCH 05/14] powerpc/rtas: Modernise RI clearing on 64-bit Nicholas Piggin
@ 2022-03-08 13:50 ` Nicholas Piggin
2022-03-14 15:17 ` Laurent Dufour
2022-03-08 13:50 ` [PATCH 07/14] powerpc/rtas: PACA can be restored directly from SPRG Nicholas Piggin
` (10 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Nicholas Piggin @ 2022-03-08 13:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Dufour, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 06/14] powerpc/rtas: Load rtas entry MSR explicitly
2022-03-08 13:50 ` [PATCH 06/14] powerpc/rtas: Load rtas entry MSR explicitly Nicholas Piggin
@ 2022-03-14 15:17 ` Laurent Dufour
0 siblings, 0 replies; 29+ messages in thread
From: Laurent Dufour @ 2022-03-14 15:17 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
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)
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 07/14] powerpc/rtas: PACA can be restored directly from SPRG
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
` (5 preceding siblings ...)
2022-03-08 13:50 ` [PATCH 06/14] powerpc/rtas: Load rtas entry MSR explicitly Nicholas Piggin
@ 2022-03-08 13:50 ` Nicholas Piggin
2022-03-14 15:32 ` Laurent Dufour
2022-03-08 13:50 ` [PATCH 08/14] powerpc/rtas: call enter_rtas in real-mode on 64-bit Nicholas Piggin
` (9 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Nicholas Piggin @ 2022-03-08 13:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Dufour, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 07/14] powerpc/rtas: PACA can be restored directly from SPRG
2022-03-08 13:50 ` [PATCH 07/14] powerpc/rtas: PACA can be restored directly from SPRG Nicholas Piggin
@ 2022-03-14 15:32 ` Laurent Dufour
0 siblings, 0 replies; 29+ messages in thread
From: Laurent Dufour @ 2022-03-14 15:32 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
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)
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 08/14] powerpc/rtas: call enter_rtas in real-mode on 64-bit
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
` (6 preceding siblings ...)
2022-03-08 13:50 ` [PATCH 07/14] powerpc/rtas: PACA can be restored directly from SPRG Nicholas Piggin
@ 2022-03-08 13:50 ` Nicholas Piggin
2022-03-14 17:09 ` Laurent Dufour
2022-03-17 9:36 ` Laurent Dufour
2022-03-08 13:50 ` [PATCH 09/14] powerpc/rtas: Leave MSR[RI] enabled over RTAS call Nicholas Piggin
` (8 subsequent siblings)
16 siblings, 2 replies; 29+ messages in thread
From: Nicholas Piggin @ 2022-03-08 13:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Dufour, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 08/14] powerpc/rtas: call enter_rtas in real-mode on 64-bit
2022-03-08 13:50 ` [PATCH 08/14] powerpc/rtas: call enter_rtas in real-mode on 64-bit Nicholas Piggin
@ 2022-03-14 17:09 ` Laurent Dufour
2022-03-17 9:36 ` Laurent Dufour
1 sibling, 0 replies; 29+ messages in thread
From: Laurent Dufour @ 2022-03-14 17:09 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
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 */
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 08/14] powerpc/rtas: call enter_rtas in real-mode on 64-bit
2022-03-08 13:50 ` [PATCH 08/14] powerpc/rtas: call enter_rtas in real-mode on 64-bit Nicholas Piggin
2022-03-14 17:09 ` Laurent Dufour
@ 2022-03-17 9:36 ` Laurent Dufour
1 sibling, 0 replies; 29+ messages in thread
From: Laurent Dufour @ 2022-03-17 9:36 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Aneesh Kumar K.V
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 */
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 09/14] powerpc/rtas: Leave MSR[RI] enabled over RTAS call
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
` (7 preceding siblings ...)
2022-03-08 13:50 ` [PATCH 08/14] powerpc/rtas: call enter_rtas in real-mode on 64-bit Nicholas Piggin
@ 2022-03-08 13:50 ` Nicholas Piggin
2022-03-14 17:17 ` Laurent Dufour
2022-03-08 13:50 ` [PATCH 10/14] powerpc/rtas: replace rtas_call_unlocked with raw_rtas_call Nicholas Piggin
` (7 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Nicholas Piggin @ 2022-03-08 13:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Dufour, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 09/14] powerpc/rtas: Leave MSR[RI] enabled over RTAS call
2022-03-08 13:50 ` [PATCH 09/14] powerpc/rtas: Leave MSR[RI] enabled over RTAS call Nicholas Piggin
@ 2022-03-14 17:17 ` Laurent Dufour
0 siblings, 0 replies; 29+ messages in thread
From: Laurent Dufour @ 2022-03-14 17:17 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
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)
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 10/14] powerpc/rtas: replace rtas_call_unlocked with raw_rtas_call
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
` (8 preceding siblings ...)
2022-03-08 13:50 ` [PATCH 09/14] powerpc/rtas: Leave MSR[RI] enabled over RTAS call Nicholas Piggin
@ 2022-03-08 13:50 ` Nicholas Piggin
2022-03-14 17:30 ` Laurent Dufour
2022-03-08 13:50 ` [PATCH 11/14] powerpc/rtas: tidy __fetch_rtas_last_error Nicholas Piggin
` (6 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Nicholas Piggin @ 2022-03-08 13:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Dufour, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 10/14] powerpc/rtas: replace rtas_call_unlocked with raw_rtas_call
2022-03-08 13:50 ` [PATCH 10/14] powerpc/rtas: replace rtas_call_unlocked with raw_rtas_call Nicholas Piggin
@ 2022-03-14 17:30 ` Laurent Dufour
0 siblings, 0 replies; 29+ messages in thread
From: Laurent Dufour @ 2022-03-14 17:30 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
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 */
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 11/14] powerpc/rtas: tidy __fetch_rtas_last_error
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
` (9 preceding siblings ...)
2022-03-08 13:50 ` [PATCH 10/14] powerpc/rtas: replace rtas_call_unlocked with raw_rtas_call Nicholas Piggin
@ 2022-03-08 13:50 ` Nicholas Piggin
2022-03-08 13:50 ` [PATCH 12/14] powerpc/rtas: Close theoretical memory leak Nicholas Piggin
` (5 subsequent siblings)
16 siblings, 0 replies; 29+ messages in thread
From: Nicholas Piggin @ 2022-03-08 13:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Dufour, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 12/14] powerpc/rtas: Close theoretical memory leak
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
` (10 preceding siblings ...)
2022-03-08 13:50 ` [PATCH 11/14] powerpc/rtas: tidy __fetch_rtas_last_error Nicholas Piggin
@ 2022-03-08 13:50 ` Nicholas Piggin
2022-03-15 17:17 ` Laurent Dufour
2022-03-08 13:50 ` [PATCH 13/14] powerpc/rtas: enture rtas_call is called with MMU enabled Nicholas Piggin
` (4 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Nicholas Piggin @ 2022-03-08 13:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Dufour, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 12/14] powerpc/rtas: Close theoretical memory leak
2022-03-08 13:50 ` [PATCH 12/14] powerpc/rtas: Close theoretical memory leak Nicholas Piggin
@ 2022-03-15 17:17 ` Laurent Dufour
0 siblings, 0 replies; 29+ messages in thread
From: Laurent Dufour @ 2022-03-15 17:17 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
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);
> }
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 13/14] powerpc/rtas: enture rtas_call is called with MMU enabled
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
` (11 preceding siblings ...)
2022-03-08 13:50 ` [PATCH 12/14] powerpc/rtas: Close theoretical memory leak Nicholas Piggin
@ 2022-03-08 13:50 ` Nicholas Piggin
2022-03-15 17:18 ` Laurent Dufour
2022-03-08 13:50 ` [PATCH 14/14] powerpc/rtas: Consolidate and improve checking for rtas callers Nicholas Piggin
` (3 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Nicholas Piggin @ 2022-03-08 13:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Dufour, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 13/14] powerpc/rtas: enture rtas_call is called with MMU enabled
2022-03-08 13:50 ` [PATCH 13/14] powerpc/rtas: enture rtas_call is called with MMU enabled Nicholas Piggin
@ 2022-03-15 17:18 ` Laurent Dufour
0 siblings, 0 replies; 29+ messages in thread
From: Laurent Dufour @ 2022-03-15 17:18 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
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 */
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 14/14] powerpc/rtas: Consolidate and improve checking for rtas callers
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
` (12 preceding siblings ...)
2022-03-08 13:50 ` [PATCH 13/14] powerpc/rtas: enture rtas_call is called with MMU enabled Nicholas Piggin
@ 2022-03-08 13:50 ` Nicholas Piggin
2022-03-15 17:26 ` Laurent Dufour
2022-03-08 14:51 ` [PATCH 00/14] powerpc/rtas: various cleanups and improvements Christophe Leroy
` (2 subsequent siblings)
16 siblings, 1 reply; 29+ messages in thread
From: Nicholas Piggin @ 2022-03-08 13:50 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Dufour, 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
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 14/14] powerpc/rtas: Consolidate and improve checking for rtas callers
2022-03-08 13:50 ` [PATCH 14/14] powerpc/rtas: Consolidate and improve checking for rtas callers Nicholas Piggin
@ 2022-03-15 17:26 ` Laurent Dufour
0 siblings, 0 replies; 29+ messages in thread
From: Laurent Dufour @ 2022-03-15 17:26 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
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();
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 00/14] powerpc/rtas: various cleanups and improvements
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
` (13 preceding siblings ...)
2022-03-08 13:50 ` [PATCH 14/14] powerpc/rtas: Consolidate and improve checking for rtas callers Nicholas Piggin
@ 2022-03-08 14:51 ` Christophe Leroy
2022-03-17 11:15 ` Laurent Dufour
2022-05-24 11:08 ` Michael Ellerman
16 siblings, 0 replies; 29+ messages in thread
From: Christophe Leroy @ 2022-03-08 14:51 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Laurent Dufour
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
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 00/14] powerpc/rtas: various cleanups and improvements
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
` (14 preceding siblings ...)
2022-03-08 14:51 ` [PATCH 00/14] powerpc/rtas: various cleanups and improvements Christophe Leroy
@ 2022-03-17 11:15 ` Laurent Dufour
2022-05-24 11:08 ` Michael Ellerman
16 siblings, 0 replies; 29+ messages in thread
From: Laurent Dufour @ 2022-03-17 11:15 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
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
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 00/14] powerpc/rtas: various cleanups and improvements
2022-03-08 13:50 [PATCH 00/14] powerpc/rtas: various cleanups and improvements Nicholas Piggin
` (15 preceding siblings ...)
2022-03-17 11:15 ` Laurent Dufour
@ 2022-05-24 11:08 ` Michael Ellerman
16 siblings, 0 replies; 29+ messages in thread
From: Michael Ellerman @ 2022-05-24 11:08 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Laurent Dufour
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
^ permalink raw reply [flat|nested] 29+ messages in thread