Re: [PATCH v7 2/3] arm64: implement ftrace with regs

From: Ard Biesheuvel
Date: Mon Feb 04 2019 - 08:43:56 EST


On Mon, 4 Feb 2019 at 13:03, Torsten Duwe <duwe@xxxxxx> wrote:
>
> On Tue, Jan 22, 2019 at 02:55:12PM +0100, Ard Biesheuvel wrote:
> > On Tue, 22 Jan 2019 at 14:28, Torsten Duwe <duwe@xxxxxx> wrote:
> > >
> > > On Tue, Jan 22, 2019 at 10:18:17AM +0000, Julien Thierry wrote:
> > > > Hi Torsten,
> > > >
> > > > A few suggestions below.
> > > >
> > > > > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > > > > +#define ARCH_SUPPORTS_FTRACE_OPS 1
> > > > > +#define REC_IP_BRANCH_OFFSET AARCH64_INSN_SIZE
> > > > > +/* All we need is some magic value. Simply use "_mCount:" */
> > > >
> > > > Nit: Should the casing be "_mcount" ?
> > >
> > > No! The string makes it clear what it's supposed to be and the peculiar
> > > casing makes it unique and leaves no doubt were it came from. So whenever
> > > you see this register value in a crash dump you don't have to wonder about
> > > weird linkage errors, as it surely did not originate from a symtab.
> > >
> >
> > In that case, do you need to deal with endianness here?
> >
> > > > > +#define MCOUNT_ADDR (0x5f6d436f756e743a)
>
> Strictly speaking, yes. OTOH "wrong-andian" machines always show a difference
> when memory is dumped in bigger units than bytes, so when you see the register
> value in hex...
>
> Since all that's needed is a somewhat unique constant, let's not over-engineer
> this ok?
>

Ah ok, if it is only for visual recognition, then I guess it doesn't matter.

> If there are no other comments I'll send out v8 with the discussed changes.
>
> Torsten
>