| Message ID | 20220601054850.250287-3-rmclure@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/6] powerpc: Add ZERO_GPRS macros for register clears | expand |
Le 01/06/2022 à 07:48, Rohan McLure a écrit : > [Vous ne recevez pas souvent de courriers de la part de rmclure@linux.ibm.com. Découvrez pourquoi cela peut être important à l'adresse https://aka.ms/LearnAboutSenderIdentification.] > > Clears user state in gprs to reduce the influence of user registers on > speculation within kernel syscall handlers. > > Remove conditional branches on result of `syscall_exit_prepare` to > restore non-volatile gprs, as these registers are always cleared and > hence always must be restored. Did you measure the implied performance loss ? That must be heavy. In patch 2 it is said that clearing registers is optional. I can't see it as an option here. Some PPC32 are not impacted by speculative problems, could we activate that big hammer only when required ? > > Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> > --- > arch/powerpc/kernel/interrupt_64.S | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S > index b11c2bd84827..e601ed999798 100644 > --- a/arch/powerpc/kernel/interrupt_64.S > +++ b/arch/powerpc/kernel/interrupt_64.S > @@ -108,6 +108,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > * but this is the best we can do. > */ > > + ZERO_GPRS(5, 12) > + ZERO_NVGPRS() > + > /* Calling convention has r3 = orig r0, r4 = regs */ > mr r3,r0 > bl system_call_exception > @@ -138,6 +141,7 @@ BEGIN_FTR_SECTION > HMT_MEDIUM_LOW > END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > > + REST_NVGPRS(r1) > cmpdi r3,0 > bne .Lsyscall_vectored_\name\()_restore_regs > > @@ -180,7 +184,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > ld r4,_LINK(r1) > ld r5,_XER(r1) > > - REST_NVGPRS(r1) > ld r0,GPR0(r1) > mtcr r2 > mtctr r3 > @@ -308,6 +311,9 @@ END_BTB_FLUSH_SECTION > wrteei 1 > #endif > > + ZERO_GPRS(5, 12) > + ZERO_NVGPRS() > + > /* Calling convention has r3 = orig r0, r4 = regs */ > mr r3,r0 > bl system_call_exception > @@ -350,6 +356,7 @@ BEGIN_FTR_SECTION > stdcx. r0,0,r1 /* to clear the reservation */ > END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS) > > + REST_NVGPRS(r1) > cmpdi r3,0 > bne .Lsyscall_restore_regs > /* Zero volatile regs that may contain sensitive kernel data */ > @@ -377,7 +384,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > .Lsyscall_restore_regs: > ld r3,_CTR(r1) > ld r4,_XER(r1) > - REST_NVGPRS(r1) > mtctr r3 > mtspr SPRN_XER,r4 > REST_GPR(0, r1) > @@ -445,7 +451,7 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user) > bl interrupt_exit_user_prepare > cmpdi r3,0 > bne- .Lrestore_nvgprs_\srr > -.Lrestore_nvgprs_\srr\()_cont: > + .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: > -- > 2.34.1 >
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S index b11c2bd84827..e601ed999798 100644 --- a/arch/powerpc/kernel/interrupt_64.S +++ b/arch/powerpc/kernel/interrupt_64.S @@ -108,6 +108,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) * but this is the best we can do. */ + ZERO_GPRS(5, 12) + ZERO_NVGPRS() + /* Calling convention has r3 = orig r0, r4 = regs */ mr r3,r0 bl system_call_exception @@ -138,6 +141,7 @@ BEGIN_FTR_SECTION HMT_MEDIUM_LOW END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) + REST_NVGPRS(r1) cmpdi r3,0 bne .Lsyscall_vectored_\name\()_restore_regs @@ -180,7 +184,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) ld r4,_LINK(r1) ld r5,_XER(r1) - REST_NVGPRS(r1) ld r0,GPR0(r1) mtcr r2 mtctr r3 @@ -308,6 +311,9 @@ END_BTB_FLUSH_SECTION wrteei 1 #endif + ZERO_GPRS(5, 12) + ZERO_NVGPRS() + /* Calling convention has r3 = orig r0, r4 = regs */ mr r3,r0 bl system_call_exception @@ -350,6 +356,7 @@ BEGIN_FTR_SECTION stdcx. r0,0,r1 /* to clear the reservation */ END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS) + REST_NVGPRS(r1) cmpdi r3,0 bne .Lsyscall_restore_regs /* Zero volatile regs that may contain sensitive kernel data */ @@ -377,7 +384,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) .Lsyscall_restore_regs: ld r3,_CTR(r1) ld r4,_XER(r1) - REST_NVGPRS(r1) mtctr r3 mtspr SPRN_XER,r4 REST_GPR(0, r1) @@ -445,7 +451,7 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\()_user) bl interrupt_exit_user_prepare cmpdi r3,0 bne- .Lrestore_nvgprs_\srr -.Lrestore_nvgprs_\srr\()_cont: + .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:
Clears user state in gprs to reduce the influence of user registers on speculation within kernel syscall handlers. Remove conditional branches on result of `syscall_exit_prepare` to restore non-volatile gprs, as these registers are always cleared and hence always must be restored. Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> --- arch/powerpc/kernel/interrupt_64.S | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)