[PATCH v2 14/14] powerpc/64s: Clear gprs on interrupt routine entry

Andrew Donnellan ajd at linux.ibm.com
Fri Aug 12 18:26:58 AEST 2022


On Mon, 2022-07-25 at 16:32 +1000, Rohan McLure wrote:
> 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 on interrupt entry and simplify exit logic. 32-bit

I wouldn't describe the existing uses of REST_NVGPRS as "on interrupt
entry" given they all happen right before branching to
interrupt_return_srr.

> systems
> will still conditionally need to restore NVGPRS so keep return value
> for
> interrupt_exit_user_prepare.

Could you clarify what you mean here?

> 
> 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
> ---
>  arch/powerpc/kernel/exceptions-64s.S | 21 ++++++++-------------
>  arch/powerpc/kernel/interrupt_64.S   |  9 ++-------
>  2 files changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/exceptions-64s.S
> b/arch/powerpc/kernel/exceptions-64s.S
> index 102896fc6a86..62dd6270852e 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        */
> +       NULLIFY_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)
> +       NULLIFY_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
> */
> +       NULLIFY_GPR(11)
>  
>         SAVE_NVGPRS(r1)
> +       NULLIFY_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   */
> +       NULLIFY_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 efaf162fa772..083a94604072 100644
> --- a/arch/powerpc/kernel/interrupt_64.S
> +++ b/arch/powerpc/kernel/interrupt_64.S
> @@ -430,9 +430,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:
> @@ -446,6 +443,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)
> @@ -515,10 +513,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)
> @@ -559,6 +553,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

-- 
Andrew Donnellan    OzLabs, ADL Canberra
ajd at linux.ibm.com   IBM Australia Limited



More information about the Linuxppc-dev mailing list