Re: [PATCH 3/6] arm64: scs: Use 'scs_sp' register alias for x18

[Mark Rutland]

On Mon, May 18, 2020 at 02:03:36PM +0100, Will Deacon wrote:
> On Mon, May 18, 2020 at 12:55:47PM +0100, Mark Rutland wrote:
> > On Fri, May 15, 2020 at 06:27:53PM +0100, Will Deacon wrote:
> > > x18 holds the SCS stack pointer value, so introduce a register alias to
> > > make this easier to read in assembly code.
> > > 
> > > Signed-off-by: Will Deacon <will@xxxxxxxxxx>
> > 
> > I scanned through arm64 for all instances of x18, and it looks like
> > you've covered all the relevant uses here. In kvm we save/restore x18 a
> > bunch becasue it might be a platform register, but we do that
> > unconditionally and without knowledge of what it contains, so I think
> > that's fine to leave as-is. Therefore:
> > 
> > Reviewed-by: Mark Rutland <mark.rutland@xxxxxxx>
> > 
> > As an aside, the comment in entry-ftrace.S is now stale where it says
> > that x18 is safe to clobber. I can send a patch to clean that up, unless
> > you want to do that yourself.
> 
> Thanks, I'll fix that up (see below). Also, apologies for typo'ing your
> email address when I sent this out on Friday.

No worries; I'd already forgotten!

> 
> Will
> 
> --->8
> 
> From 7e86208cd6541c1229bc1fcd206596308d1727f8 Mon Sep 17 00:00:00 2001
> From: Will Deacon <will@xxxxxxxxxx>
> Date: Mon, 18 May 2020 14:01:01 +0100
> Subject: [PATCH] arm64: entry-ftrace.S: Update comment to indicate that x18 is
>  live
> 
> The Shadow Call Stack pointer is held in x18, so update the ftrace
> entry comment to indicate that it cannot be safely clobbered.
> 
> Reported-by: Mark Rutland <mark.rutland@xxxxxxx>
> Signed-off-by: Will Deacon <will@xxxxxxxxxx>
> ---
>  arch/arm64/kernel/entry-ftrace.S | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S
> index 833d48c9acb5..a338f40e64d3 100644
> --- a/arch/arm64/kernel/entry-ftrace.S
> +++ b/arch/arm64/kernel/entry-ftrace.S
> @@ -23,8 +23,9 @@
>   *
>   * ... where <entry> is either ftrace_caller or ftrace_regs_caller.
>   *
> - * Each instrumented function follows the AAPCS, so here x0-x8 and x19-x30 are
> - * live, and x9-x18 are safe to clobber.
> + * Each instrumented function follows the AAPCS, so here x0-x8 and x18-x30 are
> + * live (x18 holds the Shadow Call Stack pointer), and x9-x17 are safe to
> + * clobber.

I'd have called out x18 a bit more specifically:

| Each instrumented function follows the AAPCS, so here x0-x8 and x19-x30 are
| live, x18 is potentially live with a shadow call stack pointer, and
| x9-x17 are safe to clobber.

... but either way this looks good; thanks!

Mark.