Le 01/06/2022 à 07:48, 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 on interrupt entry and simplify exit logic. > > Signed-off-by: Rohan McLure <rmcl...@linux.ibm.com> > --- > arch/powerpc/kernel/exceptions-64s.S | 19 +++++++------------ > arch/powerpc/kernel/interrupt_64.S | 9 ++------- > 2 files changed, 9 insertions(+), 19 deletions(-) > > diff --git a/arch/powerpc/kernel/exceptions-64s.S > b/arch/powerpc/kernel/exceptions-64s.S > index 102896fc6a86..8e2c1c924a4d 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 */ > + ZERO_GPR(0) > > /* Mark our [H]SRRs valid for return */ > li r10,1 > @@ -538,14 +539,17 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > ld r10,IAREA+EX_R10(r13) > std r9,GPR9(r1) > std r10,GPR10(r1) > + ZERO_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) > + ZERO_GPR(11) /* keep r12 ([H]SRR1/MSR), r13 (PACA) for interrupt > routine */ > > SAVE_NVGPRS(r1) > + ZERO_NVGPRS() > > .if IDAR > .if IISIDE > @@ -577,8 +581,9 @@ 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_GPR(2, r1) /* save r2 in stackframe */ > SAVE_GPRS(3, 8, r1) /* save r3 - r8 in stackframe */ > + ZERO_GPRS(2, 8)
What's the point in clearing r2 whereas it will be overwritten just below ? > 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 92740d9889a3..3c742c07f4b6 100644 > --- a/arch/powerpc/kernel/interrupt_64.S > +++ b/arch/powerpc/kernel/interrupt_64.S > @@ -442,9 +442,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 Then should we make interrupt_exit_user_prepare() a void function if we don't use the returned value anymore ? > - 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: > @@ -458,6 +455,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) > @@ -527,10 +525,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) > @@ -571,6 +565,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
