diff mbox series

[v3,16/18] powerpc/32: Clarify interrupt restores with REST_GPR macro in entry_32.S

Message ID 20220819033806.162054-17-rmclure@linux.ibm.com (mailing list archive)
State Superseded
Headers show
Series powerpc: Syscall wrapper and register clearing | expand

Commit Message

Rohan McLure Aug. 19, 2022, 3:38 a.m. UTC 
Restoring the register state of the interrupted thread involves issuing
a large number of predictable loads to the kernel stack frame. Issue the
REST_GPR{,S} macros to clearly signal when this is happening, and bunch
together restores at the end of the interrupt handler where the saved
value is not consumed earlier in the handler code.

Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
Reported-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
V2 -> V3: New patch.
---
 arch/powerpc/kernel/entry_32.S | 35 ++++++++++++--------------------
 1 file changed, 13 insertions(+), 22 deletions(-)

Comments

Christophe Leroy Aug. 19, 2022, 6:29 a.m. UTC | #1 
Le 19/08/2022 à 05:38, Rohan McLure a écrit :
> Restoring the register state of the interrupted thread involves issuing
> a large number of predictable loads to the kernel stack frame. Issue the
> REST_GPR{,S} macros to clearly signal when this is happening, and bunch
> together restores at the end of the interrupt handler where the saved
> value is not consumed earlier in the handler code.

Keep all possible restores before restoring SRR0/SRR1, see details below.

> 
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> Reported-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> V2 -> V3: New patch.
> ---
>   arch/powerpc/kernel/entry_32.S | 35 ++++++++++++--------------------
>   1 file changed, 13 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 8d6e02ef5dc0..03a105a5806a 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -68,7 +68,7 @@ prepare_transfer_to_handler:
>   	lwz	r9,_MSR(r11)		/* if sleeping, clear MSR.EE */
>   	rlwinm	r9,r9,0,~MSR_EE
>   	lwz	r12,_LINK(r11)		/* and return to address in LR */
> -	lwz	r2, GPR2(r11)
> +	REST_GPR(2, r11)
>   	b	fast_exception_return
>   _ASM_NOKPROBE_SYMBOL(prepare_transfer_to_handler)
>   #endif /* CONFIG_PPC_BOOK3S_32 || CONFIG_E500 */
> @@ -144,7 +144,7 @@ ret_from_syscall:
>   	lwz	r7,_NIP(r1)
>   	lwz	r8,_MSR(r1)
>   	cmpwi	r3,0
> -	lwz	r3,GPR3(r1)
> +	REST_GPR(3, r1)
>   syscall_exit_finish:
>   	mtspr	SPRN_SRR0,r7
>   	mtspr	SPRN_SRR1,r8
> @@ -152,8 +152,8 @@ syscall_exit_finish:
>   	bne	3f
>   	mtcr	r5
>   
> -1:	lwz	r2,GPR2(r1)
> -	lwz	r1,GPR1(r1)
> +1:	REST_GPR(2, r1)
> +	REST_GPR(1, r1)
>   	rfi
>   #ifdef CONFIG_40x
>   	b .	/* Prevent prefetch past rfi */
> @@ -165,10 +165,8 @@ syscall_exit_finish:
>   	REST_NVGPRS(r1)
>   	mtctr	r4
>   	mtxer	r5
> -	lwz	r0,GPR0(r1)
> -	lwz	r3,GPR3(r1)
> -	REST_GPRS(4, 11, r1)
> -	lwz	r12,GPR12(r1)
> +	REST_GPR(0, r1)
> +	REST_GPRS(3, 12, r1)
>   	b	1b
>   
>   #ifdef CONFIG_44x
> @@ -260,24 +258,22 @@ fast_exception_return:
>   	beq	3f			/* if not, we've got problems */
>   #endif
>   
> -2:	REST_GPRS(3, 6, r11)
> -	lwz	r10,_CCR(r11)
> -	REST_GPRS(1, 2, r11)
> +2:	lwz	r10,_CCR(r11)
>   	mtcr	r10
>   	lwz	r10,_LINK(r11)
>   	mtlr	r10
>   	/* Clear the exception_marker on the stack to avoid confusing stacktrace */
>   	li	r10, 0
>   	stw	r10, 8(r11)
> -	REST_GPR(10, r11)
>   #if defined(CONFIG_PPC_8xx) && defined(CONFIG_PERF_EVENTS)
>   	mtspr	SPRN_NRI, r0
>   #endif
>   	mtspr	SPRN_SRR1,r9
>   	mtspr	SPRN_SRR0,r12
> -	REST_GPR(9, r11)
> +	REST_GPRS(1, 6, r11)
> +	REST_GPRS(9, 10, r11)

Please keep this before modification of SRR0/SRR1. Once SRR0/SRR1 are 
modified, interrupts become unrecoverable. We want to keep that window 
as small as possible.

>   	REST_GPR(12, r11)
> -	lwz	r11,GPR11(r11)
> +	REST_GPR(11, r11)
>   	rfi
>   #ifdef CONFIG_40x
>   	b .	/* Prevent prefetch past rfi */
> @@ -454,9 +450,7 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return)
>   	lwz	r3,_MSR(r1);						\
>   	andi.	r3,r3,MSR_PR;						\
>   	bne	interrupt_return;					\
> -	lwz	r0,GPR0(r1);						\
> -	lwz	r2,GPR2(r1);						\
> -	REST_GPRS(3, 8, r1);						\
> +	REST_GPR(0, r1);						\
>   	lwz	r10,_XER(r1);						\
>   	lwz	r11,_CTR(r1);						\
>   	mtspr	SPRN_XER,r10;						\
> @@ -475,11 +469,8 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return)
>   	lwz	r12,_MSR(r1);						\
>   	mtspr	exc_lvl_srr0,r11;					\
>   	mtspr	exc_lvl_srr1,r12;					\
> -	lwz	r9,GPR9(r1);						\
> -	lwz	r12,GPR12(r1);						\
> -	lwz	r10,GPR10(r1);						\
> -	lwz	r11,GPR11(r1);						\
> -	lwz	r1,GPR1(r1);						\
> +	REST_GPRS(2, 12, r1);						\

Same, please keep things minimal between restore of SRR0/SRR1 and RFI to 
minimise the unrecoverable window.

> +	REST_GPR(1, r1);						\
>   	exc_lvl_rfi;							\
>   	b	.;		/* prevent prefetch past exc_lvl_rfi */
>
diff mbox series

Patch

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 8d6e02ef5dc0..03a105a5806a 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -68,7 +68,7 @@  prepare_transfer_to_handler:
 	lwz	r9,_MSR(r11)		/* if sleeping, clear MSR.EE */
 	rlwinm	r9,r9,0,~MSR_EE
 	lwz	r12,_LINK(r11)		/* and return to address in LR */
-	lwz	r2, GPR2(r11)
+	REST_GPR(2, r11)
 	b	fast_exception_return
 _ASM_NOKPROBE_SYMBOL(prepare_transfer_to_handler)
 #endif /* CONFIG_PPC_BOOK3S_32 || CONFIG_E500 */
@@ -144,7 +144,7 @@  ret_from_syscall:
 	lwz	r7,_NIP(r1)
 	lwz	r8,_MSR(r1)
 	cmpwi	r3,0
-	lwz	r3,GPR3(r1)
+	REST_GPR(3, r1)
 syscall_exit_finish:
 	mtspr	SPRN_SRR0,r7
 	mtspr	SPRN_SRR1,r8
@@ -152,8 +152,8 @@  syscall_exit_finish:
 	bne	3f
 	mtcr	r5
 
-1:	lwz	r2,GPR2(r1)
-	lwz	r1,GPR1(r1)
+1:	REST_GPR(2, r1)
+	REST_GPR(1, r1)
 	rfi
 #ifdef CONFIG_40x
 	b .	/* Prevent prefetch past rfi */
@@ -165,10 +165,8 @@  syscall_exit_finish:
 	REST_NVGPRS(r1)
 	mtctr	r4
 	mtxer	r5
-	lwz	r0,GPR0(r1)
-	lwz	r3,GPR3(r1)
-	REST_GPRS(4, 11, r1)
-	lwz	r12,GPR12(r1)
+	REST_GPR(0, r1)
+	REST_GPRS(3, 12, r1)
 	b	1b
 
 #ifdef CONFIG_44x
@@ -260,24 +258,22 @@  fast_exception_return:
 	beq	3f			/* if not, we've got problems */
 #endif
 
-2:	REST_GPRS(3, 6, r11)
-	lwz	r10,_CCR(r11)
-	REST_GPRS(1, 2, r11)
+2:	lwz	r10,_CCR(r11)
 	mtcr	r10
 	lwz	r10,_LINK(r11)
 	mtlr	r10
 	/* Clear the exception_marker on the stack to avoid confusing stacktrace */
 	li	r10, 0
 	stw	r10, 8(r11)
-	REST_GPR(10, r11)
 #if defined(CONFIG_PPC_8xx) && defined(CONFIG_PERF_EVENTS)
 	mtspr	SPRN_NRI, r0
 #endif
 	mtspr	SPRN_SRR1,r9
 	mtspr	SPRN_SRR0,r12
-	REST_GPR(9, r11)
+	REST_GPRS(1, 6, r11)
+	REST_GPRS(9, 10, r11)
 	REST_GPR(12, r11)
-	lwz	r11,GPR11(r11)
+	REST_GPR(11, r11)
 	rfi
 #ifdef CONFIG_40x
 	b .	/* Prevent prefetch past rfi */
@@ -454,9 +450,7 @@  _ASM_NOKPROBE_SYMBOL(interrupt_return)
 	lwz	r3,_MSR(r1);						\
 	andi.	r3,r3,MSR_PR;						\
 	bne	interrupt_return;					\
-	lwz	r0,GPR0(r1);						\
-	lwz	r2,GPR2(r1);						\
-	REST_GPRS(3, 8, r1);						\
+	REST_GPR(0, r1);						\
 	lwz	r10,_XER(r1);						\
 	lwz	r11,_CTR(r1);						\
 	mtspr	SPRN_XER,r10;						\
@@ -475,11 +469,8 @@  _ASM_NOKPROBE_SYMBOL(interrupt_return)
 	lwz	r12,_MSR(r1);						\
 	mtspr	exc_lvl_srr0,r11;					\
 	mtspr	exc_lvl_srr1,r12;					\
-	lwz	r9,GPR9(r1);						\
-	lwz	r12,GPR12(r1);						\
-	lwz	r10,GPR10(r1);						\
-	lwz	r11,GPR11(r1);						\
-	lwz	r1,GPR1(r1);						\
+	REST_GPRS(2, 12, r1);						\
+	REST_GPR(1, r1);						\
 	exc_lvl_rfi;							\
 	b	.;		/* prevent prefetch past exc_lvl_rfi */