diff mbox series

[1/5] powerpc/64: use 32-bit immediate for STACK_FRAME_REGS_MARKER

Message ID 20220923032512.535725-2-npiggin@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series powerpc/64: avoid GOT addressing, don't put data in TOC | expand

Commit Message

Nicholas Piggin Sept. 23, 2022, 3:25 a.m. UTC
Using a 32-bit constant for this marker allows it to be loaded with
two ALU instructions, like 32-bit. This avoids a TOC entry and a
TOC load that depends on the r2 value that has just been loaded from
the PACA.

This changes the value for 32-bit as well, so both have the same
value in the low 4 bytes and 64-bit has 0xffffffff in the top bytes.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/ptrace.h    | 5 +++--
 arch/powerpc/kernel/entry_32.S       | 6 +++---
 arch/powerpc/kernel/exceptions-64e.S | 8 +-------
 arch/powerpc/kernel/exceptions-64s.S | 2 +-
 arch/powerpc/kernel/head_64.S        | 7 -------
 arch/powerpc/kernel/interrupt_64.S   | 6 +++---
 6 files changed, 11 insertions(+), 23 deletions(-)

Comments

Christophe Leroy Sept. 23, 2022, 5:34 a.m. UTC | #1
Le 23/09/2022 à 05:25, Nicholas Piggin a écrit :
> Using a 32-bit constant for this marker allows it to be loaded with
> two ALU instructions, like 32-bit. This avoids a TOC entry and a
> TOC load that depends on the r2 value that has just been loaded from
> the PACA.
> 
> This changes the value for 32-bit as well, so both have the same
> value in the low 4 bytes and 64-bit has 0xffffffff in the top bytes.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   arch/powerpc/include/asm/ptrace.h    | 5 +++--
>   arch/powerpc/kernel/entry_32.S       | 6 +++---
>   arch/powerpc/kernel/exceptions-64e.S | 8 +-------
>   arch/powerpc/kernel/exceptions-64s.S | 2 +-
>   arch/powerpc/kernel/head_64.S        | 7 -------
>   arch/powerpc/kernel/interrupt_64.S   | 6 +++---
>   6 files changed, 11 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> index a03403695cd4..49d720bb888b 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -99,6 +99,9 @@ struct pt_regs
>   
>   #define STACK_FRAME_WITH_PT_REGS (STACK_FRAME_OVERHEAD + sizeof(struct pt_regs))
>   
> +/* 0xffffffff8d9a988d on 64-bit */
> +#define STACK_FRAME_REGS_MARKER	ASM_CONST(-0x72656773) /* 0x8d9a988d */
> +

0x72656773 is "REGS" in ASCII (Big Endian) and you can spot it 
immediatly in a memory dump.
0x7265677368657265 is "REGSHERE".

0x8d9a988d is not printable.

Don't know if it can be a problem.

Christophe
Nicholas Piggin Sept. 23, 2022, 6:14 a.m. UTC | #2
On Fri Sep 23, 2022 at 3:34 PM AEST, Christophe Leroy wrote:
>
>
> Le 23/09/2022 à 05:25, Nicholas Piggin a écrit :
> > Using a 32-bit constant for this marker allows it to be loaded with
> > two ALU instructions, like 32-bit. This avoids a TOC entry and a
> > TOC load that depends on the r2 value that has just been loaded from
> > the PACA.
> > 
> > This changes the value for 32-bit as well, so both have the same
> > value in the low 4 bytes and 64-bit has 0xffffffff in the top bytes.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   arch/powerpc/include/asm/ptrace.h    | 5 +++--
> >   arch/powerpc/kernel/entry_32.S       | 6 +++---
> >   arch/powerpc/kernel/exceptions-64e.S | 8 +-------
> >   arch/powerpc/kernel/exceptions-64s.S | 2 +-
> >   arch/powerpc/kernel/head_64.S        | 7 -------
> >   arch/powerpc/kernel/interrupt_64.S   | 6 +++---
> >   6 files changed, 11 insertions(+), 23 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> > index a03403695cd4..49d720bb888b 100644
> > --- a/arch/powerpc/include/asm/ptrace.h
> > +++ b/arch/powerpc/include/asm/ptrace.h
> > @@ -99,6 +99,9 @@ struct pt_regs
> >   
> >   #define STACK_FRAME_WITH_PT_REGS (STACK_FRAME_OVERHEAD + sizeof(struct pt_regs))
> >   
> > +/* 0xffffffff8d9a988d on 64-bit */
> > +#define STACK_FRAME_REGS_MARKER	ASM_CONST(-0x72656773) /* 0x8d9a988d */
> > +
>
> 0x72656773 is "REGS" in ASCII (Big Endian) and you can spot it 
> immediatly in a memory dump.
> 0x7265677368657265 is "REGSHERE".
>
> 0x8d9a988d is not printable.
>
> Don't know if it can be a problem.

Oh. I guess it doesn't really matter if it has zeroes or f in the
top 4 bytes so I should keep it "REGS" then. I was thinking about
later moving it into the reserved word next to the CR word on
64-bit so we would only have 4 bytes for it anyway.

Thanks,
Nick
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index a03403695cd4..49d720bb888b 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -99,6 +99,9 @@  struct pt_regs
 
 #define STACK_FRAME_WITH_PT_REGS (STACK_FRAME_OVERHEAD + sizeof(struct pt_regs))
 
+/* 0xffffffff8d9a988d on 64-bit */
+#define STACK_FRAME_REGS_MARKER	ASM_CONST(-0x72656773) /* 0x8d9a988d */
+
 #ifdef __powerpc64__
 
 /*
@@ -115,7 +118,6 @@  struct pt_regs
 
 #define STACK_FRAME_OVERHEAD	112	/* size of minimum stack frame */
 #define STACK_FRAME_LR_SAVE	2	/* Location of LR in stack frame */
-#define STACK_FRAME_REGS_MARKER	ASM_CONST(0x7265677368657265)
 #define STACK_INT_FRAME_SIZE	(sizeof(struct pt_regs) + \
 				 STACK_FRAME_OVERHEAD + KERNEL_REDZONE_SIZE)
 #define STACK_FRAME_MARKER	12
@@ -136,7 +138,6 @@  struct pt_regs
 #define KERNEL_REDZONE_SIZE	0
 #define STACK_FRAME_OVERHEAD	16	/* size of minimum stack frame */
 #define STACK_FRAME_LR_SAVE	1	/* Location of LR in stack frame */
-#define STACK_FRAME_REGS_MARKER	ASM_CONST(0x72656773)
 #define STACK_INT_FRAME_SIZE	(sizeof(struct pt_regs) + STACK_FRAME_OVERHEAD)
 #define STACK_FRAME_MARKER	2
 #define STACK_FRAME_MIN_SIZE	STACK_FRAME_OVERHEAD
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 1d599df6f169..c2516f982cfa 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -265,7 +265,7 @@  fast_exception_return:
 	mtcr	r10
 	lwz	r10,_LINK(r11)
 	mtlr	r10
-	/* Clear the exception_marker on the stack to avoid confusing stacktrace */
+	/* Clear the exception marker on the stack to avoid confusing stacktrace */
 	li	r10, 0
 	stw	r10, 8(r11)
 	REST_GPR(10, r11)
@@ -322,7 +322,7 @@  ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
 	li	r0,0
 
 	/*
-	 * Leaving a stale exception_marker on the stack can confuse
+	 * Leaving a stale exception marker on the stack can confuse
 	 * the reliable stack unwinder later on. Clear it.
 	 */
 	stw	r0,8(r1)
@@ -374,7 +374,7 @@  ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
 	mtspr	SPRN_XER,r5
 
 	/*
-	 * Leaving a stale exception_marker on the stack can confuse
+	 * Leaving a stale exception marker on the stack can confuse
 	 * the reliable stack unwinder later on. Clear it.
 	 */
 	stw	r0,8(r1)
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 67dc4e3179a0..08a3139079b7 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -389,7 +389,7 @@  exc_##n##_common:							    \
 	ld	r9,excf+EX_R1(r13);	/* load orig r1 back from PACA */   \
 	lwz	r10,excf+EX_CR(r13);	/* load orig CR back from PACA	*/  \
 	lbz	r11,PACAIRQSOFTMASK(r13); /* get current IRQ softe */	    \
-	ld	r12,exception_marker@toc(r2);				    \
+	LOAD_REG_IMMEDIATE(r12, STACK_FRAME_REGS_MARKER);		    \
 	li	r0,0;							    \
 	std	r3,GPR10(r1);		/* save r10 to stackframe */	    \
 	std	r4,GPR11(r1);		/* save r11 to stackframe */	    \
@@ -470,12 +470,6 @@  exc_##n##_bad_stack:							    \
 	bl	hdlr;							\
 	b	interrupt_return
 
-/* This value is used to mark exception frames on the stack. */
-	.section	".toc","aw"
-exception_marker:
-	.tc	ID_EXC_MARKER[TC],STACK_FRAME_REGS_MARKER
-
-
 /*
  * And here we have the exception vectors !
  */
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 3d0dc133a9ae..9d375ea58add 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -589,7 +589,7 @@  END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
 	li	r9,IVEC
 	std	r9,_TRAP(r1)		/* set trap number		*/
 	li	r10,0
-	ld	r11,exception_marker@toc(r2)
+	LOAD_REG_IMMEDIATE(r11, STACK_FRAME_REGS_MARKER)
 	std	r10,RESULT(r1)		/* clear regs->result		*/
 	std	r11,STACK_FRAME_OVERHEAD-16(r1) /* mark the frame	*/
 .endm
diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index cf2c08902c05..cac3e1b58360 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -192,13 +192,6 @@  __secondary_hold:
 #endif
 CLOSE_FIXED_SECTION(first_256B)
 
-/* This value is used to mark exception frames on the stack. */
-	.section ".toc","aw"
-/* This value is used to mark exception frames on the stack. */
-exception_marker:
-	.tc	ID_EXC_MARKER[TC],STACK_FRAME_REGS_MARKER
-	.previous
-
 /*
  * On server, we include the exception vectors code here as it
  * relies on absolute addressing which is only possible within
diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
index ce25b28cf418..ee2d2d410c5a 100644
--- a/arch/powerpc/kernel/interrupt_64.S
+++ b/arch/powerpc/kernel/interrupt_64.S
@@ -92,7 +92,7 @@  _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name)
 	std	r11,_TRAP(r1)
 	std	r12,_CCR(r1)
 	addi	r10,r1,STACK_FRAME_OVERHEAD
-	ld	r11,exception_marker@toc(r2)
+	LOAD_REG_IMMEDIATE(r11, STACK_FRAME_REGS_MARKER)
 	std	r11,-16(r10)		/* "regshere" marker */
 
 BEGIN_FTR_SECTION
@@ -276,7 +276,7 @@  END_BTB_FLUSH_SECTION
 	std	r11,_TRAP(r1)
 	std	r12,_CCR(r1)
 	addi	r10,r1,STACK_FRAME_OVERHEAD
-	ld	r11,exception_marker@toc(r2)
+	LOAD_REG_IMMEDIATE(r11, STACK_FRAME_REGS_MARKER)
 	std	r11,-16(r10)		/* "regshere" marker */
 
 #ifdef CONFIG_PPC_BOOK3S
@@ -619,7 +619,7 @@  ALT_FTR_SECTION_END_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
 	mtspr	SPRN_XER,r5
 
 	/*
-	 * Leaving a stale exception_marker on the stack can confuse
+	 * Leaving a stale STACK_FRAME_REGS_MARKER on the stack can confuse
 	 * the reliable stack unwinder later on. Clear it.
 	 */
 	std	r0,STACK_FRAME_OVERHEAD-16(r1)