Re: [PATCH] [RFC] aarch64: scs: reload shadow call stack in user exception entry

[Dan Li]

On 1/4/22 2:40 AM, Mark Rutland wrote:
> Hi,
>
> On Mon, Dec 27, 2021 at 05:06:04PM -0800, Dan Li wrote:
> > When el0 exception occurs, kernel_entry/exit will load/save tsk->scs_sp
> > to ensure scs working properly. AFAIK, the SCS offset should always be
> > 0 at this time.
> >
> > Is it reasonable to reload x18 to scs_base directly in kernel_entry
> > here, or am I missing something?
> >
> > Signed-off-by: Dan Li <ashimida@xxxxxxxxxxxxxxxxx>
>
> AFAICT the saving here is just that we avoid storing the SCS SP upon return to
> EL0, and I suspect that in practice that's not measureable.
>
> So without numbers to show otherwise, I'd prefer to leave this as-is, though I
> agree that this patch should work.  If we do want to change this, I think we
> need a comment in the `kernel_exit` path to pair with the `scs_load` in
> `kernel_entry`.
>
> Thanks,
> Mark.
Yes, there is only one instruction saved here, which should have a
minimal impact on performance.
The current code is also good for me :)

Thanks,
Dan.

> > ---
> >   arch/arm64/include/asm/scs.h | 7 +++++++
> >   arch/arm64/kernel/entry.S    | 3 +--
> >   2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/scs.h b/arch/arm64/include/asm/scs.h
> > index 8297bccf0784..2bc0d0575e75 100644
> > --- a/arch/arm64/include/asm/scs.h
> > +++ b/arch/arm64/include/asm/scs.h
> > @@ -9,6 +9,10 @@
> >   #ifdef CONFIG_SHADOW_CALL_STACK
> >   	scs_sp	.req	x18
> >   
> > +	.macro scs_reload tsk
> > +	ldr	scs_sp, [\tsk, #TSK_TI_SCS_BASE]
> > +	.endm
> > +
> >   	.macro scs_load tsk
> >   	ldr	scs_sp, [\tsk, #TSK_TI_SCS_SP]
> >   	.endm
> > @@ -17,6 +21,9 @@
> >   	str	scs_sp, [\tsk, #TSK_TI_SCS_SP]
> >   	.endm
> >   #else
> > +	.macro scs_reload tsk
> > +	.endm
> > +
> >   	.macro scs_load tsk
> >   	.endm
> >   
> > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> > index bc6d5a970a13..57547a3e4f7c 100644
> > --- a/arch/arm64/kernel/entry.S
> > +++ b/arch/arm64/kernel/entry.S
> > @@ -265,7 +265,7 @@ alternative_if ARM64_HAS_ADDRESS_AUTH
> >   alternative_else_nop_endif
> >   1:
> >   
> > -	scs_load tsk
> > +	scs_reload tsk
> >   	.else
> >   	add	x21, sp, #PT_REGS_SIZE
> >   	get_current_task tsk
> > @@ -365,7 +365,6 @@ alternative_if ARM64_WORKAROUND_845719
> >   alternative_else_nop_endif
> >   #endif
> >   3:
> > -	scs_save tsk
> >   
> >   	/* Ignore asynchronous tag check faults in the uaccess routines */
> >   	ldr	x0, [tsk, THREAD_SCTLR_USER]
> > --
> > 2.17.1