All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Will Deacon <will@kernel.org>
Cc: linux-kernel@vger.kernel.org, catalin.marinas@arm.com,
	linux-arm-kernel@lists.infradead.org, mhiramat@kernel.org,
	revest@chromium.org, rostedt@goodmis.org
Subject: Re: [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS
Date: Thu, 17 Nov 2022 10:52:15 +0000	[thread overview]
Message-ID: <Y3YRqvfYOP+RBk8r@FVFF77S0Q05N> (raw)
In-Reply-To: <20221115112701.GB32523@willie-the-truck>

On Tue, Nov 15, 2022 at 11:27:03AM +0000, Will Deacon wrote:
> On Thu, Nov 03, 2022 at 05:05:20PM +0000, Mark Rutland wrote:
> > This commit replaces arm64's support for FTRACE_WITH_REGS with support
> > for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> > removes some latent issues with inconsistent presentation of struct
> > pt_regs (which can only be reliably saved/restored at exception
> > boundaries).
> 
> [...]
> 
> > @@ -78,10 +77,71 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
> >  	return addr;
> >  }
> >  
> > -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> >  struct dyn_ftrace;
> >  struct ftrace_ops;
> > -struct ftrace_regs;
> > +
> > +#define arch_ftrace_get_regs(regs) NULL
> > +
> > +struct ftrace_regs {
> > +	/* x0 - x8 */
> > +	unsigned long regs[9];
> > +	unsigned long __unused;
> > +
> > +	unsigned long fp;
> > +	unsigned long lr;
> > +
> > +	unsigned long sp;
> > +	unsigned long pc;
> > +};
> > +
> > +static __always_inline unsigned long
> > +ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
> > +{
> > +	return fregs->pc;
> > +}
> > +
> > +static __always_inline void
> > +ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
> > +				    unsigned long pc)
> > +{
> > +	fregs->pc = pc;
> > +}
> > +
> > +static __always_inline unsigned long
> > +ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs)
> > +{
> > +	return fregs->sp;
> > +}
> > +
> > +static __always_inline unsigned long
> > +ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
> > +{
> > +	if (n < 8)
> > +		return fregs->regs[n];
> 
> Where does this '8' come from?

Because in AAPCS64 the arguments are in x0 to x7, as mentioned in the commit
message:

| Per AAPCS64, all function call argument and return values are held in
| the following GPRs:
| 
| * X0 - X7 : parameter / result registers
| * X8      : indirect result location register
| * SP      : stack pointer (AKA SP)

The 'indirect result location register' would be used when returning large
structures, and isn't a function argument as such.

The logic is the same as in regs_get_kernel_argument() for pt_regs.

I can add a comment here to explain that, if that would help?

The rest of the registers are as described in the commit message (and I now
spot a typo that I'll go fix):

| Additionally, ad function call boundaries, the following GPRs hold
| context/return information:
| 
| * X29 : frame pointer (AKA FP)
| * X30 : link register (AKA LR)
| 
| ... and for ftrace we need to capture the instrumented address:
| 
|  * PC  : program counter
| 
| No other GPRs are relevant, as none of the other arguments hold
| parameters or return values:
| 
| * X9  - X17 : temporaries, may be clobbered
| * X18       : shadow call stack pointer (or temorary)
| * X19 - X28 : callee saved

Thanks,
Mark.

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Will Deacon <will@kernel.org>
Cc: linux-kernel@vger.kernel.org, catalin.marinas@arm.com,
	linux-arm-kernel@lists.infradead.org, mhiramat@kernel.org,
	revest@chromium.org, rostedt@goodmis.org
Subject: Re: [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS
Date: Thu, 17 Nov 2022 10:52:15 +0000	[thread overview]
Message-ID: <Y3YRqvfYOP+RBk8r@FVFF77S0Q05N> (raw)
In-Reply-To: <20221115112701.GB32523@willie-the-truck>

On Tue, Nov 15, 2022 at 11:27:03AM +0000, Will Deacon wrote:
> On Thu, Nov 03, 2022 at 05:05:20PM +0000, Mark Rutland wrote:
> > This commit replaces arm64's support for FTRACE_WITH_REGS with support
> > for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> > removes some latent issues with inconsistent presentation of struct
> > pt_regs (which can only be reliably saved/restored at exception
> > boundaries).
> 
> [...]
> 
> > @@ -78,10 +77,71 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
> >  	return addr;
> >  }
> >  
> > -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
> >  struct dyn_ftrace;
> >  struct ftrace_ops;
> > -struct ftrace_regs;
> > +
> > +#define arch_ftrace_get_regs(regs) NULL
> > +
> > +struct ftrace_regs {
> > +	/* x0 - x8 */
> > +	unsigned long regs[9];
> > +	unsigned long __unused;
> > +
> > +	unsigned long fp;
> > +	unsigned long lr;
> > +
> > +	unsigned long sp;
> > +	unsigned long pc;
> > +};
> > +
> > +static __always_inline unsigned long
> > +ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs)
> > +{
> > +	return fregs->pc;
> > +}
> > +
> > +static __always_inline void
> > +ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
> > +				    unsigned long pc)
> > +{
> > +	fregs->pc = pc;
> > +}
> > +
> > +static __always_inline unsigned long
> > +ftrace_regs_get_stack_pointer(const struct ftrace_regs *fregs)
> > +{
> > +	return fregs->sp;
> > +}
> > +
> > +static __always_inline unsigned long
> > +ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n)
> > +{
> > +	if (n < 8)
> > +		return fregs->regs[n];
> 
> Where does this '8' come from?

Because in AAPCS64 the arguments are in x0 to x7, as mentioned in the commit
message:

| Per AAPCS64, all function call argument and return values are held in
| the following GPRs:
| 
| * X0 - X7 : parameter / result registers
| * X8      : indirect result location register
| * SP      : stack pointer (AKA SP)

The 'indirect result location register' would be used when returning large
structures, and isn't a function argument as such.

The logic is the same as in regs_get_kernel_argument() for pt_regs.

I can add a comment here to explain that, if that would help?

The rest of the registers are as described in the commit message (and I now
spot a typo that I'll go fix):

| Additionally, ad function call boundaries, the following GPRs hold
| context/return information:
| 
| * X29 : frame pointer (AKA FP)
| * X30 : link register (AKA LR)
| 
| ... and for ftrace we need to capture the instrumented address:
| 
|  * PC  : program counter
| 
| No other GPRs are relevant, as none of the other arguments hold
| parameters or return values:
| 
| * X9  - X17 : temporaries, may be clobbered
| * X18       : shadow call stack pointer (or temorary)
| * X19 - X28 : callee saved

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-11-17 10:52 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-03 17:05 [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS Mark Rutland
2022-11-03 17:05 ` Mark Rutland
2022-11-03 17:05 ` [PATCH v2 1/4] ftrace: pass fregs to arch_ftrace_set_direct_caller() Mark Rutland
2022-11-03 17:05   ` Mark Rutland
2022-11-03 17:05 ` [PATCH v2 2/4] ftrace: rename ftrace_instruction_pointer_set() -> ftrace_regs_set_instruction_pointer() Mark Rutland
2022-11-03 17:05   ` Mark Rutland
2022-11-03 17:05 ` [PATCH v2 3/4] ftrace: abstract DYNAMIC_FTRACE_WITH_ARGS accesses Mark Rutland
2022-11-03 17:05   ` Mark Rutland
2022-11-03 17:05 ` [PATCH v2 4/4] ftrace: arm64: move from REGS to ARGS Mark Rutland
2022-11-03 17:05   ` Mark Rutland
2022-11-15 11:27   ` Will Deacon
2022-11-15 11:27     ` Will Deacon
2022-11-17 10:52     ` Mark Rutland [this message]
2022-11-17 10:52       ` Mark Rutland
2022-11-18 12:31       ` Will Deacon
2022-11-18 12:31         ` Will Deacon
2022-11-18 13:57         ` Mark Rutland
2022-11-18 13:57           ` Mark Rutland
2022-11-18 14:09           ` Will Deacon
2022-11-18 14:09             ` Will Deacon
2022-11-15 15:01 ` [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS Steven Rostedt
2022-11-15 15:01   ` Steven Rostedt
2022-11-15 15:48   ` Mark Rutland
2022-11-15 15:48     ` Mark Rutland
2022-11-18  0:04 ` Masami Hiramatsu
2022-11-18  0:04   ` Masami Hiramatsu
2022-11-18 19:40 ` Will Deacon
2022-11-18 19:40   ` Will Deacon

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=Y3YRqvfYOP+RBk8r@FVFF77S0Q05N \
    --to=mark.rutland@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=revest@chromium.org \
    --cc=rostedt@goodmis.org \
    --cc=will@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.