From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Nicholas Piggin <npiggin@gmail.com>,
"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Cc: Alan Modra <amodra@gmail.com>
Subject: Re: [PATCH v2 1/5] powerpc/64: use 32-bit immediate for STACK_FRAME_REGS_MARKER
Date: Mon, 26 Sep 2022 06:00:54 +0000 [thread overview]
Message-ID: <bdd608bc-8488-ee21-656d-6ff5a8b552f0@csgroup.eu> (raw)
In-Reply-To: <20220926034057.2360083-2-npiggin@gmail.com>
Le 26/09/2022 à 05:40, 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.
The above is wrong now, isn't it ?
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/ptrace.h | 4 ++--
> 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, 10 insertions(+), 23 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> index a03403695cd4..5b496e589d54 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -99,6 +99,8 @@ struct pt_regs
>
> #define STACK_FRAME_WITH_PT_REGS (STACK_FRAME_OVERHEAD + sizeof(struct pt_regs))
>
> +#define STACK_FRAME_REGS_MARKER ASM_CONST(0x72656773)
> +
> #ifdef __powerpc64__
>
> /*
> @@ -115,7 +117,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 +137,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)
next prev parent reply other threads:[~2022-09-26 6:01 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-26 3:40 [PATCH v2 0/5] powerpc/64: avoid GOT addressing, don't put data in TOC Nicholas Piggin
2022-09-26 3:40 ` [PATCH v2 1/5] powerpc/64: use 32-bit immediate for STACK_FRAME_REGS_MARKER Nicholas Piggin
2022-09-26 6:00 ` Christophe Leroy [this message]
2022-09-27 14:21 ` Michael Ellerman
2022-09-26 3:40 ` [PATCH v2 2/5] powerpc/64: asm use consistent global variable declaration and access Nicholas Piggin
2022-09-26 3:40 ` [PATCH v2 3/5] powerpc/64: switch asm helpers from GOT to TOC relative addressing Nicholas Piggin
2022-09-26 3:40 ` [PATCH v2 4/5] powerpc/64: provide a helper macro to load r2 with the kernel TOC Nicholas Piggin
2022-09-26 3:40 ` [PATCH v2 5/5] powerpc/64e: provide an addressing macro for use with TOC in alternate register Nicholas Piggin
2022-10-04 13:25 ` [PATCH v2 0/5] powerpc/64: avoid GOT addressing, don't put data in TOC Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bdd608bc-8488-ee21-656d-6ff5a8b552f0@csgroup.eu \
--to=christophe.leroy@csgroup.eu \
--cc=amodra@gmail.com \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=npiggin@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).