[PATCH v3 18/18] powerpc/64s: Clear gprs on interrupt routine entry

Christophe Leroy christophe.leroy at csgroup.eu
Fri Aug 19 17:41:43 AEST 2022



Le 19/08/2022 à 05:38, Rohan McLure a écrit :
> Zero GPRS r0, r2-r11, r14-r31, on entry into the kernel for all
> other interrupt sources to limit influence of user-space values
> in potential speculation gadgets. The remaining gprs are overwritten by
> entry macros to interrupt handlers, irrespective of whether or not a
> given handler consumes these register values.
> 
> Prior to this commit, r14-r31 are restored on a per-interrupt basis at
> exit, but now they are always restored. Remove explicit REST_NVGPRS
> invocations as non-volatiles must now always be restored. 32-bit systems
> do not clear user registers on interrupt, and continue to depend on the
> return value of interrupt_exit_user_prepare to determine whether or not
> to restore non-volatiles.
> 
> The mmap_bench benchmark in selftests should rapidly invoke pagefaults.
> See ~0.8% performance regression with this mitigation, but this
> indicates the worst-case performance due to heavier-weight interrupt
> handlers.
> 
> Signed-off-by: Rohan McLure <rmclure at linux.ibm.com>
> ---
> V1 -> V2: Add benchmark data
> V2 -> V3: Use ZEROIZE_GPR{,S} macro renames, clarify
> interrupt_exit_user_prepare changes in summary.
> ---
>   arch/powerpc/kernel/exceptions-64s.S | 21 ++++++++-------------
>   arch/powerpc/kernel/interru_64.S   |  9 ++-------pt
What about arch/powerpc/kernel/exceptions-64e.S, no change required 
inside it ? As interru_64.S applies to both 64s and 64e, I would have 
expected changes in exceptions_64e too.

>   2 files changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index a3b51441b039..038e42fb2182 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -502,6 +502,7 @@ DEFINE_FIXED_SYMBOL(\name\()_common_real, text)
>   	std	r10,0(r1)		/* make stack chain pointer	*/
>   	std	r0,GPR0(r1)		/* save r0 in stackframe	*/
>   	std	r10,GPR1(r1)		/* save r1 in stackframe	*/
> +	ZEROIZE_GPR(0)
>   
>   	/* Mark our [H]SRRs valid for return */
>   	li	r10,1
> @@ -538,14 +539,18 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>   	ld	r10,IAREA+EX_R10(r13)
>   	std	r9,GPR9(r1)
>   	std	r10,GPR10(r1)
> +	ZEROIZE_GPRS(9, 10)
>   	ld	r9,IAREA+EX_R11(r13)	/* move r11 - r13 to stackframe	*/
>   	ld	r10,IAREA+EX_R12(r13)
>   	ld	r11,IAREA+EX_R13(r13)
>   	std	r9,GPR11(r1)
>   	std	r10,GPR12(r1)
>   	std	r11,GPR13(r1)
> +	/* keep r12 ([H]SRR1/MSR), r13 (PACA) for interrupt routine */
> +	ZEROIZE_GPR(11)
>   
>   	SAVE_NVGPRS(r1)
> +	ZEROIZE_NVGPRS()
>   
>   	.if IDAR
>   	.if IISIDE
> @@ -577,8 +582,8 @@ BEGIN_FTR_SECTION
>   END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
>   	ld	r10,IAREA+EX_CTR(r13)
>   	std	r10,_CTR(r1)
> -	std	r2,GPR2(r1)		/* save r2 in stackframe	*/
> -	SAVE_GPRS(3, 8, r1)		/* save r3 - r8 in stackframe   */
> +	SAVE_GPRS(2, 8, r1)		/* save r2 - r8 in stackframe   */
> +	ZEROIZE_GPRS(2, 8)
>   	mflr	r9			/* Get LR, later save to stack	*/
>   	ld	r2,PACATOC(r13)		/* get kernel TOC into r2	*/
>   	std	r9,_LINK(r1)
> @@ -696,6 +701,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
>   	mtlr	r9
>   	ld	r9,_CCR(r1)
>   	mtcr	r9
> +	REST_NVGPRS(r1)
>   	REST_GPRS(2, 13, r1)
>   	REST_GPR(0, r1)
>   	/* restore original r1. */
> @@ -1368,11 +1374,6 @@ ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
>   	b	interrupt_return_srr
>   
>   1:	bl	do_break
> -	/*
> -	 * do_break() may have changed the NV GPRS while handling a breakpoint.
> -	 * If so, we need to restore them with their updated values.
> -	 */
> -	REST_NVGPRS(r1)
>   	b	interrupt_return_srr
>   
>   
> @@ -1598,7 +1599,6 @@ EXC_COMMON_BEGIN(alignment_common)
>   	GEN_COMMON alignment
>   	addi	r3,r1,STACK_FRAME_OVERHEAD
>   	bl	alignment_exception
> -	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
>   	b	interrupt_return_srr
>   
>   
> @@ -1708,7 +1708,6 @@ EXC_COMMON_BEGIN(program_check_common)
>   .Ldo_program_check:
>   	addi	r3,r1,STACK_FRAME_OVERHEAD
>   	bl	program_check_exception
> -	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
>   	b	interrupt_return_srr
>   
>   
> @@ -2139,7 +2138,6 @@ EXC_COMMON_BEGIN(emulation_assist_common)
>   	GEN_COMMON emulation_assist
>   	addi	r3,r1,STACK_FRAME_OVERHEAD
>   	bl	emulation_assist_interrupt
> -	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
>   	b	interrupt_return_hsrr
>   
>   
> @@ -2457,7 +2455,6 @@ EXC_COMMON_BEGIN(facility_unavailable_common)
>   	GEN_COMMON facility_unavailable
>   	addi	r3,r1,STACK_FRAME_OVERHEAD
>   	bl	facility_unavailable_exception
> -	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
>   	b	interrupt_return_srr
>   
>   
> @@ -2485,7 +2482,6 @@ EXC_COMMON_BEGIN(h_facility_unavailable_common)
>   	GEN_COMMON h_facility_unavailable
>   	addi	r3,r1,STACK_FRAME_OVERHEAD
>   	bl	facility_unavailable_exception
> -	REST_NVGPRS(r1) /* XXX Shouldn't be necessary in practice */
>   	b	interrupt_return_hsrr
>   
>   
> @@ -2711,7 +2707,6 @@ EXC_COMMON_BEGIN(altivec_assist_common)
>   	addi	r3,r1,STACK_FRAME_OVERHEAD
>   #ifdef CONFIG_ALTIVEC
>   	bl	altivec_assist_exception
> -	REST_NVGPRS(r1) /* instruction emulation may change GPRs */
>   #else
>   	bl	unknown_exception
>   #endif
> diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
> index ad302ad93433..f9ee93e3a0d3 100644
> --- a/arch/powerpc/kernel/interrupt_64.S
> +++ b/arch/powerpc/kernel/interrupt_64.S
> @@ -432,9 +432,6 @@ interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
>   _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user)
>   	addi	r3,r1,STACK_FRAME_OVERHEAD
>   	bl	interrupt_exit_user_prepare
> -	cmpdi	r3,0
> -	bne-	.Lrestore_nvgprs_\srr
> -.Lrestore_nvgprs_\srr\()_cont:
>   	std	r1,PACA_EXIT_SAVE_R1(r13) /* save r1 for restart */
>   #ifdef CONFIG_PPC_BOOK3S
>   .Linterrupt_return_\srr\()_user_rst_start:
> @@ -448,6 +445,7 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user)
>   	stb	r11,PACAIRQHAPPENED(r13) # clear out possible HARD_DIS
>   
>   .Lfast_user_interrupt_return_\srr\():
> +	REST_NVGPRS(r1)
>   #ifdef CONFIG_PPC_BOOK3S
>   	.ifc \srr,srr
>   	lbz	r4,PACASRR_VALID(r13)
> @@ -517,10 +515,6 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
>   	b	.	/* prevent speculative execution */
>   .Linterrupt_return_\srr\()_user_rst_end:
>   
> -.Lrestore_nvgprs_\srr\():
> -	REST_NVGPRS(r1)
> -	b	.Lrestore_nvgprs_\srr\()_cont
> -
>   #ifdef CONFIG_PPC_BOOK3S
>   interrupt_return_\srr\()_user_restart:
>   _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user_restart)
> @@ -561,6 +555,7 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_kernel)
>   1:
>   
>   .Lfast_kernel_interrupt_return_\srr\():
> +	REST_NVGPRS(r1)
>   	cmpdi	cr1,r3,0
>   #ifdef CONFIG_PPC_BOOK3S
>   	.ifc \srr,srr


More information about the Linuxppc-dev mailing list