| Message ID | 20220725063136.121108-1-rmclure@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc: Syscall wrapper and register clearing | expand |
On Mon, 2022-07-25 at 16:31 +1000, Rohan McLure wrote: > Use the convenience macros for saving/clearing/restoring gprs in > keeping > with syscall calling conventions. The plural variants of these macros > can store a range of registers for concision. > > This works well when the save-to-stack logic is simple (a gpr is > saved > to its corresponding offset in the struct pt_regs on the stack). > Wherever a different gpr is storing the initial value of another gpr > at > the interrupt location, care must be taken to issue the correct save > instruction and so the macros are not applied in neighbouring stores. This second paragraph is a bit unclear, I think I understand what you're saying but could you rephrase it for clarity? > > Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> > --- > V1 -> V2: Update summary > --- > arch/powerpc/kernel/interrupt_64.S | 29 ++++++---------------------- > 1 file changed, 6 insertions(+), 23 deletions(-) > > diff --git a/arch/powerpc/kernel/interrupt_64.S > b/arch/powerpc/kernel/interrupt_64.S > index 34167cfa5d60..efaf162fa772 100644 > --- a/arch/powerpc/kernel/interrupt_64.S > +++ b/arch/powerpc/kernel/interrupt_64.S > @@ -71,12 +71,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name) > mfcr r12 > li r11,0 > /* Save syscall parameters in r3-r8 */ > - std r3,GPR3(r1) > - std r4,GPR4(r1) > - std r5,GPR5(r1) > - std r6,GPR6(r1) > - std r7,GPR7(r1) > - std r8,GPR8(r1) > + SAVE_GPRS(3, 8, r1) > /* Zero r9-r12, this should only be required when restoring > all GPRs */ > std r11,GPR9(r1) > std r11,GPR10(r1) > @@ -156,17 +151,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > /* Could zero these as per ABI, but we may consider a > stricter ABI > * which preserves these if libc implementations can benefit, > so > * restore them for now until further measurement is done. */ > - ld r0,GPR0(r1) > - ld r4,GPR4(r1) > - ld r5,GPR5(r1) > - ld r6,GPR6(r1) > - ld r7,GPR7(r1) > - ld r8,GPR8(r1) > + REST_GPR(0, r1) > + REST_GPRS(4, 8, r1) > /* Zero volatile regs that may contain sensitive kernel data > */ > - li r9,0 > - li r10,0 > - li r11,0 > - li r12,0 > + NULLIFY_GPRS(9, 12) > mtspr SPRN_XER,r0 > > /* > @@ -188,7 +176,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > ld r4,_LINK(r1) > ld r5,_XER(r1) > > - ld r0,GPR0(r1) > + REST_GPR(0, r1) > mtcr r2 > mtctr r3 > mtlr r4 > @@ -256,12 +244,7 @@ END_BTB_FLUSH_SECTION > mfcr r12 > li r11,0 > /* Save syscall parameters in r3-r8 */ > - std r3,GPR3(r1) > - std r4,GPR4(r1) > - std r5,GPR5(r1) > - std r6,GPR6(r1) > - std r7,GPR7(r1) > - std r8,GPR8(r1) > + SAVE_GPRS(3, 8, r1) > /* Zero r9-r12, this should only be required when restoring > all GPRs */ > std r11,GPR9(r1) > std r11,GPR10(r1)
Le 25/07/2022 à 08:31, Rohan McLure a écrit : > Use the convenience macros for saving/clearing/restoring gprs in keeping > with syscall calling conventions. The plural variants of these macros > can store a range of registers for concision. > > This works well when the save-to-stack logic is simple (a gpr is saved > to its corresponding offset in the struct pt_regs on the stack). > Wherever a different gpr is storing the initial value of another gpr at > the interrupt location, care must be taken to issue the correct save > instruction and so the macros are not applied in neighbouring stores. Some cleaning could also be done in entry_32.S > > Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> > --- > V1 -> V2: Update summary > --- > arch/powerpc/kernel/interrupt_64.S | 29 ++++++---------------------- > 1 file changed, 6 insertions(+), 23 deletions(-) > > diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S > index 34167cfa5d60..efaf162fa772 100644 > --- a/arch/powerpc/kernel/interrupt_64.S > +++ b/arch/powerpc/kernel/interrupt_64.S > @@ -71,12 +71,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name) > mfcr r12 > li r11,0 > /* Save syscall parameters in r3-r8 */ > - std r3,GPR3(r1) > - std r4,GPR4(r1) > - std r5,GPR5(r1) > - std r6,GPR6(r1) > - std r7,GPR7(r1) > - std r8,GPR8(r1) > + SAVE_GPRS(3, 8, r1) > /* Zero r9-r12, this should only be required when restoring all GPRs */ > std r11,GPR9(r1) > std r11,GPR10(r1) > @@ -156,17 +151,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > /* Could zero these as per ABI, but we may consider a stricter ABI > * which preserves these if libc implementations can benefit, so > * restore them for now until further measurement is done. */ > - ld r0,GPR0(r1) > - ld r4,GPR4(r1) > - ld r5,GPR5(r1) > - ld r6,GPR6(r1) > - ld r7,GPR7(r1) > - ld r8,GPR8(r1) > + REST_GPR(0, r1) > + REST_GPRS(4, 8, r1) > /* Zero volatile regs that may contain sensitive kernel data */ > - li r9,0 > - li r10,0 > - li r11,0 > - li r12,0 > + NULLIFY_GPRS(9, 12) > mtspr SPRN_XER,r0 > > /* > @@ -188,7 +176,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) > ld r4,_LINK(r1) > ld r5,_XER(r1) > > - ld r0,GPR0(r1) > + REST_GPR(0, r1) > mtcr r2 > mtctr r3 > mtlr r4 > @@ -256,12 +244,7 @@ END_BTB_FLUSH_SECTION > mfcr r12 > li r11,0 > /* Save syscall parameters in r3-r8 */ > - std r3,GPR3(r1) > - std r4,GPR4(r1) > - std r5,GPR5(r1) > - std r6,GPR6(r1) > - std r7,GPR7(r1) > - std r8,GPR8(r1) > + SAVE_GPRS(3, 8, r1) > /* Zero r9-r12, this should only be required when restoring all GPRs */ > std r11,GPR9(r1) > std r11,GPR10(r1)
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S index 34167cfa5d60..efaf162fa772 100644 --- a/arch/powerpc/kernel/interrupt_64.S +++ b/arch/powerpc/kernel/interrupt_64.S @@ -71,12 +71,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name) mfcr r12 li r11,0 /* Save syscall parameters in r3-r8 */ - std r3,GPR3(r1) - std r4,GPR4(r1) - std r5,GPR5(r1) - std r6,GPR6(r1) - std r7,GPR7(r1) - std r8,GPR8(r1) + SAVE_GPRS(3, 8, r1) /* Zero r9-r12, this should only be required when restoring all GPRs */ std r11,GPR9(r1) std r11,GPR10(r1) @@ -156,17 +151,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) /* Could zero these as per ABI, but we may consider a stricter ABI * which preserves these if libc implementations can benefit, so * restore them for now until further measurement is done. */ - ld r0,GPR0(r1) - ld r4,GPR4(r1) - ld r5,GPR5(r1) - ld r6,GPR6(r1) - ld r7,GPR7(r1) - ld r8,GPR8(r1) + REST_GPR(0, r1) + REST_GPRS(4, 8, r1) /* Zero volatile regs that may contain sensitive kernel data */ - li r9,0 - li r10,0 - li r11,0 - li r12,0 + NULLIFY_GPRS(9, 12) mtspr SPRN_XER,r0 /* @@ -188,7 +176,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR) ld r4,_LINK(r1) ld r5,_XER(r1) - ld r0,GPR0(r1) + REST_GPR(0, r1) mtcr r2 mtctr r3 mtlr r4 @@ -256,12 +244,7 @@ END_BTB_FLUSH_SECTION mfcr r12 li r11,0 /* Save syscall parameters in r3-r8 */ - std r3,GPR3(r1) - std r4,GPR4(r1) - std r5,GPR5(r1) - std r6,GPR6(r1) - std r7,GPR7(r1) - std r8,GPR8(r1) + SAVE_GPRS(3, 8, r1) /* Zero r9-r12, this should only be required when restoring all GPRs */ std r11,GPR9(r1) std r11,GPR10(r1)
Use the convenience macros for saving/clearing/restoring gprs in keeping with syscall calling conventions. The plural variants of these macros can store a range of registers for concision. This works well when the save-to-stack logic is simple (a gpr is saved to its corresponding offset in the struct pt_regs on the stack). Wherever a different gpr is storing the initial value of another gpr at the interrupt location, care must be taken to issue the correct save instruction and so the macros are not applied in neighbouring stores. Signed-off-by: Rohan McLure <rmclure@linux.ibm.com> --- V1 -> V2: Update summary --- arch/powerpc/kernel/interrupt_64.S | 29 ++++++---------------------- 1 file changed, 6 insertions(+), 23 deletions(-)