linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 12/14] powerpc/64s: Use {NULLIFY,SAVE,REST}_GPRS macros in sc, scv 0 handlers
@ 2022-07-25  6:31 Rohan McLure
  2022-08-08  9:02 ` Andrew Donnellan
  2022-08-08 10:59 ` Christophe Leroy
  0 siblings, 2 replies; 3+ messages in thread
From: Rohan McLure @ 2022-07-25  6:31 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Rohan McLure, npiggin

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(-)

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)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 12/14] powerpc/64s: Use {NULLIFY,SAVE,REST}_GPRS macros in sc, scv 0 handlers
  2022-07-25  6:31 [PATCH v2 12/14] powerpc/64s: Use {NULLIFY,SAVE,REST}_GPRS macros in sc, scv 0 handlers Rohan McLure
@ 2022-08-08  9:02 ` Andrew Donnellan
  2022-08-08 10:59 ` Christophe Leroy
  1 sibling, 0 replies; 3+ messages in thread
From: Andrew Donnellan @ 2022-08-08  9:02 UTC (permalink / raw)
  To: Rohan McLure, linuxppc-dev; +Cc: npiggin

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)

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 12/14] powerpc/64s: Use {NULLIFY,SAVE,REST}_GPRS macros in sc, scv 0 handlers
  2022-07-25  6:31 [PATCH v2 12/14] powerpc/64s: Use {NULLIFY,SAVE,REST}_GPRS macros in sc, scv 0 handlers Rohan McLure
  2022-08-08  9:02 ` Andrew Donnellan
@ 2022-08-08 10:59 ` Christophe Leroy
  1 sibling, 0 replies; 3+ messages in thread
From: Christophe Leroy @ 2022-08-08 10:59 UTC (permalink / raw)
  To: Rohan McLure, linuxppc-dev; +Cc: npiggin



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)

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-08-08 11:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-25  6:31 [PATCH v2 12/14] powerpc/64s: Use {NULLIFY,SAVE,REST}_GPRS macros in sc, scv 0 handlers Rohan McLure
2022-08-08  9:02 ` Andrew Donnellan
2022-08-08 10:59 ` Christophe Leroy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).