diff mbox

[PATCH/RESEND] recordmcount: arm: Implement make_nop

Message ID 20161018234200.5804-1-sboyd@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Stephen Boyd Oct. 18, 2016, 11:42 p.m. UTC
In similar spirit to x86 and arm64 support, add a make_nop_arm()
to replace calls to mcount with a nop in sections that aren't
traced.

Cc: Russell King <linux@arm.linux.org.uk>
Acked-by: Rabin Vincent <rabin@rab.in>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 scripts/recordmcount.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

Comments

Steven Rostedt Oct. 19, 2016, 12:07 a.m. UTC | #1
On Tue, 18 Oct 2016 16:42:00 -0700
Stephen Boyd <sboyd@codeaurora.org> wrote:

> In similar spirit to x86 and arm64 support, add a make_nop_arm()
> to replace calls to mcount with a nop in sections that aren't
> traced.
> 
> Cc: Russell King <linux@arm.linux.org.uk>
> Acked-by: Rabin Vincent <rabin@rab.in>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>

I can take this if I can get an ack from the ARM maintainers.

-- Steve
Steven Rostedt Nov. 14, 2016, 6:36 p.m. UTC | #2
On Tue, 18 Oct 2016 20:07:07 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 18 Oct 2016 16:42:00 -0700
> Stephen Boyd <sboyd@codeaurora.org> wrote:
> 
> > In similar spirit to x86 and arm64 support, add a make_nop_arm()
> > to replace calls to mcount with a nop in sections that aren't
> > traced.
> > 
> > Cc: Russell King <linux@arm.linux.org.uk>
> > Acked-by: Rabin Vincent <rabin@rab.in>
> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>  
> 
> I can take this if I can get an ack from the ARM maintainers.

Any ARM maintainer want to ack this, or take it in their tree if they
haven't already?

-- Steve
Russell King (Oracle) Nov. 14, 2016, 6:41 p.m. UTC | #3
On Mon, Nov 14, 2016 at 01:36:41PM -0500, Steven Rostedt wrote:
> On Tue, 18 Oct 2016 20:07:07 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Tue, 18 Oct 2016 16:42:00 -0700
> > Stephen Boyd <sboyd@codeaurora.org> wrote:
> > 
> > > In similar spirit to x86 and arm64 support, add a make_nop_arm()
> > > to replace calls to mcount with a nop in sections that aren't
> > > traced.
> > > 
> > > Cc: Russell King <linux@arm.linux.org.uk>
> > > Acked-by: Rabin Vincent <rabin@rab.in>
> > > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>  
> > 
> > I can take this if I can get an ack from the ARM maintainers.
> 
> Any ARM maintainer want to ack this, or take it in their tree if they
> haven't already?

Assuming it's been tested:

Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Ard Biesheuvel Nov. 15, 2016, 2:19 p.m. UTC | #4
On 19 October 2016 at 00:42, Stephen Boyd <sboyd@codeaurora.org> wrote:
> In similar spirit to x86 and arm64 support, add a make_nop_arm()
> to replace calls to mcount with a nop in sections that aren't
> traced.
>
> Cc: Russell King <linux@arm.linux.org.uk>
> Acked-by: Rabin Vincent <rabin@rab.in>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  scripts/recordmcount.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
>
> diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
> index 5423a58d1b06..aeb34223167c 100644
> --- a/scripts/recordmcount.c
> +++ b/scripts/recordmcount.c
> @@ -213,6 +213,59 @@ static int make_nop_x86(void *map, size_t const offset)
>         return 0;
>  }
>
> +static unsigned char ideal_nop4_arm_le[4] = { 0x00, 0x00, 0xa0, 0xe1 }; /* mov r0, r0 */
> +static unsigned char ideal_nop4_arm_be[4] = { 0xe1, 0xa0, 0x00, 0x00 }; /* mov r0, r0 */

Shouldn't you be taking the difference between BE8 and BE32 into
account here? IIRC, BE8 uses little endian encoding for instructions.

> +static unsigned char *ideal_nop4_arm;
> +
> +static unsigned char bl_mcount_arm_le[4] = { 0xfe, 0xff, 0xff, 0xeb }; /* bl */
> +static unsigned char bl_mcount_arm_be[4] = { 0xeb, 0xff, 0xff, 0xfe }; /* bl */
> +static unsigned char *bl_mcount_arm;
> +
> +static unsigned char push_arm_le[4] = { 0x04, 0xe0, 0x2d, 0xe5 }; /* push {lr} */
> +static unsigned char push_arm_be[4] = { 0xe5, 0x2d, 0xe0, 0x04 }; /* push {lr} */
> +static unsigned char *push_arm;
> +
> +static unsigned char ideal_nop2_thumb_le[2] = { 0x00, 0xbf }; /* nop */
> +static unsigned char ideal_nop2_thumb_be[2] = { 0xbf, 0x00 }; /* nop */
> +static unsigned char *ideal_nop2_thumb;
> +
> +static unsigned char push_bl_mcount_thumb_le[6] = { 0x00, 0xb5, 0xff, 0xf7, 0xfe, 0xff }; /* push {lr}, bl */
> +static unsigned char push_bl_mcount_thumb_be[6] = { 0xb5, 0x00, 0xf7, 0xff, 0xff, 0xfe }; /* push {lr}, bl */
> +static unsigned char *push_bl_mcount_thumb;
> +
> +static int make_nop_arm(void *map, size_t const offset)
> +{
> +       char *ptr;
> +       int cnt = 1;
> +       int nop_size;
> +       size_t off = offset;
> +
> +       ptr = map + offset;
> +       if (memcmp(ptr, bl_mcount_arm, 4) == 0) {
> +               if (memcmp(ptr - 4, push_arm, 4) == 0) {
> +                       off -= 4;
> +                       cnt = 2;
> +               }
> +               ideal_nop = ideal_nop4_arm;
> +               nop_size = 4;
> +       } else if (memcmp(ptr - 2, push_bl_mcount_thumb, 6) == 0) {
> +               cnt = 3;
> +               nop_size = 2;
> +               off -= 2;
> +               ideal_nop = ideal_nop2_thumb;
> +       } else
> +               return -1;
> +
> +       /* Convert to nop */
> +       ulseek(fd_map, off, SEEK_SET);
> +
> +       do {
> +               uwrite(fd_map, ideal_nop, nop_size);
> +       } while (--cnt > 0);
> +
> +       return 0;
> +}
> +
>  static unsigned char ideal_nop4_arm64[4] = {0x1f, 0x20, 0x03, 0xd5};
>  static int make_nop_arm64(void *map, size_t const offset)
>  {
> @@ -430,6 +483,11 @@ do_file(char const *const fname)
>                         w2 = w2rev;
>                         w8 = w8rev;
>                 }
> +               ideal_nop4_arm = ideal_nop4_arm_le;
> +               bl_mcount_arm = bl_mcount_arm_le;
> +               push_arm = push_arm_le;
> +               ideal_nop2_thumb = ideal_nop2_thumb_le;
> +               push_bl_mcount_thumb = push_bl_mcount_thumb_le;
>                 break;
>         case ELFDATA2MSB:
>                 if (*(unsigned char const *)&endian != 0) {
> @@ -438,6 +496,11 @@ do_file(char const *const fname)
>                         w2 = w2rev;
>                         w8 = w8rev;
>                 }
> +               ideal_nop4_arm = ideal_nop4_arm_be;
> +               bl_mcount_arm = bl_mcount_arm_be;
> +               push_arm = push_arm_be;
> +               ideal_nop2_thumb = ideal_nop2_thumb_be;
> +               push_bl_mcount_thumb = push_bl_mcount_thumb_be;
>                 break;
>         }  /* end switch */
>         if (memcmp(ELFMAG, ehdr->e_ident, SELFMAG) != 0
> @@ -463,6 +526,8 @@ do_file(char const *const fname)
>                 break;
>         case EM_ARM:     reltype = R_ARM_ABS32;
>                          altmcount = "__gnu_mcount_nc";
> +                        make_nop = make_nop_arm;
> +                        rel_type_nop = R_ARM_NONE;
>                          break;
>         case EM_AARCH64:
>                         reltype = R_AARCH64_ABS64;
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Steven Rostedt Nov. 15, 2016, 3:20 p.m. UTC | #5
On Tue, 15 Nov 2016 14:19:44 +0000
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> On 19 October 2016 at 00:42, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > In similar spirit to x86 and arm64 support, add a make_nop_arm()
> > to replace calls to mcount with a nop in sections that aren't
> > traced.
> >
> > Cc: Russell King <linux@arm.linux.org.uk>
> > Acked-by: Rabin Vincent <rabin@rab.in>
> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> > ---
> >  scripts/recordmcount.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> >
> > diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
> > index 5423a58d1b06..aeb34223167c 100644
> > --- a/scripts/recordmcount.c
> > +++ b/scripts/recordmcount.c
> > @@ -213,6 +213,59 @@ static int make_nop_x86(void *map, size_t const offset)
> >         return 0;
> >  }
> >
> > +static unsigned char ideal_nop4_arm_le[4] = { 0x00, 0x00, 0xa0, 0xe1 }; /* mov r0, r0 */
> > +static unsigned char ideal_nop4_arm_be[4] = { 0xe1, 0xa0, 0x00, 0x00 }; /* mov r0, r0 */  
> 
> Shouldn't you be taking the difference between BE8 and BE32 into
> account here? IIRC, BE8 uses little endian encoding for instructions.
> 

I was just about to push this to linux-next (where I don't rebase). I'm
guessing I should hold off then.

Luckily, this was the last patch of my tree that I tested, and I can
just remove that one.

-- Steve
Stephen Boyd Nov. 15, 2016, 7:18 p.m. UTC | #6
On 11/15, Ard Biesheuvel wrote:
> On 19 October 2016 at 00:42, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > In similar spirit to x86 and arm64 support, add a make_nop_arm()
> > to replace calls to mcount with a nop in sections that aren't
> > traced.
> >
> > Cc: Russell King <linux@arm.linux.org.uk>
> > Acked-by: Rabin Vincent <rabin@rab.in>
> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> > ---
> >  scripts/recordmcount.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 65 insertions(+)
> >
> > diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
> > index 5423a58d1b06..aeb34223167c 100644
> > --- a/scripts/recordmcount.c
> > +++ b/scripts/recordmcount.c
> > @@ -213,6 +213,59 @@ static int make_nop_x86(void *map, size_t const offset)
> >         return 0;
> >  }
> >
> > +static unsigned char ideal_nop4_arm_le[4] = { 0x00, 0x00, 0xa0, 0xe1 }; /* mov r0, r0 */
> > +static unsigned char ideal_nop4_arm_be[4] = { 0xe1, 0xa0, 0x00, 0x00 }; /* mov r0, r0 */
> 
> Shouldn't you be taking the difference between BE8 and BE32 into
> account here? IIRC, BE8 uses little endian encoding for instructions.

I admit I haven't tested on a pre-armv6 CPU so I haven't come
across the case of a BE32 CPU. But from what I can tell that
doesn't matter.

According to scripts/Makefile.build, cmd_record_mcount only runs
the recordmcount program if CONFIG_FTRACE_MCOUNT_RECORD=y. That
config is defined as:

	config FTRACE_MCOUNT_RECORD
		def_bool y
		depends on DYNAMIC_FTRACE
		depends on HAVE_FTRACE_MCOUNT_RECORD


And in arch/arm/Kconfig we see that DYNAMIC_FTRACE is selected:

	select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU

which means that FTRACE_MCOUNT_RECORD can't be set when
CPU_ENDIAN_BE32 is set.

Do you agree that BE32 is not a concern here?
Ard Biesheuvel Nov. 15, 2016, 7:25 p.m. UTC | #7
On 15 November 2016 at 19:18, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 11/15, Ard Biesheuvel wrote:
>> On 19 October 2016 at 00:42, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> > In similar spirit to x86 and arm64 support, add a make_nop_arm()
>> > to replace calls to mcount with a nop in sections that aren't
>> > traced.
>> >
>> > Cc: Russell King <linux@arm.linux.org.uk>
>> > Acked-by: Rabin Vincent <rabin@rab.in>
>> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> > ---
>> >  scripts/recordmcount.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> >  1 file changed, 65 insertions(+)
>> >
>> > diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
>> > index 5423a58d1b06..aeb34223167c 100644
>> > --- a/scripts/recordmcount.c
>> > +++ b/scripts/recordmcount.c
>> > @@ -213,6 +213,59 @@ static int make_nop_x86(void *map, size_t const offset)
>> >         return 0;
>> >  }
>> >
>> > +static unsigned char ideal_nop4_arm_le[4] = { 0x00, 0x00, 0xa0, 0xe1 }; /* mov r0, r0 */
>> > +static unsigned char ideal_nop4_arm_be[4] = { 0xe1, 0xa0, 0x00, 0x00 }; /* mov r0, r0 */
>>
>> Shouldn't you be taking the difference between BE8 and BE32 into
>> account here? IIRC, BE8 uses little endian encoding for instructions.
>
> I admit I haven't tested on a pre-armv6 CPU so I haven't come
> across the case of a BE32 CPU. But from what I can tell that
> doesn't matter.
>
> According to scripts/Makefile.build, cmd_record_mcount only runs
> the recordmcount program if CONFIG_FTRACE_MCOUNT_RECORD=y. That
> config is defined as:
>
>         config FTRACE_MCOUNT_RECORD
>                 def_bool y
>                 depends on DYNAMIC_FTRACE
>                 depends on HAVE_FTRACE_MCOUNT_RECORD
>
>
> And in arch/arm/Kconfig we see that DYNAMIC_FTRACE is selected:
>
>         select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
>
> which means that FTRACE_MCOUNT_RECORD can't be set when
> CPU_ENDIAN_BE32 is set.
>
> Do you agree that BE32 is not a concern here?
>

Yes. But that implies then that you should not be using big-endian
instruction encodings at all, and simply use the _le variants for both
LE and BE8
Stephen Boyd Nov. 15, 2016, 11:53 p.m. UTC | #8
On 11/15, Ard Biesheuvel wrote:
> On 15 November 2016 at 19:18, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 11/15, Ard Biesheuvel wrote:
> >> On 19 October 2016 at 00:42, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >> >
> >> > +static unsigned char ideal_nop4_arm_le[4] = { 0x00, 0x00, 0xa0, 0xe1 }; /* mov r0, r0 */
> >> > +static unsigned char ideal_nop4_arm_be[4] = { 0xe1, 0xa0, 0x00, 0x00 }; /* mov r0, r0 */
> >>
> >> Shouldn't you be taking the difference between BE8 and BE32 into
> >> account here? IIRC, BE8 uses little endian encoding for instructions.
> >
> > I admit I haven't tested on a pre-armv6 CPU so I haven't come
> > across the case of a BE32 CPU. But from what I can tell that
> > doesn't matter.
> >
> > According to scripts/Makefile.build, cmd_record_mcount only runs
> > the recordmcount program if CONFIG_FTRACE_MCOUNT_RECORD=y. That
> > config is defined as:
> >
> >         config FTRACE_MCOUNT_RECORD
> >                 def_bool y
> >                 depends on DYNAMIC_FTRACE
> >                 depends on HAVE_FTRACE_MCOUNT_RECORD
> >
> >
> > And in arch/arm/Kconfig we see that DYNAMIC_FTRACE is selected:
> >
> >         select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
> >
> > which means that FTRACE_MCOUNT_RECORD can't be set when
> > CPU_ENDIAN_BE32 is set.
> >
> > Do you agree that BE32 is not a concern here?
> >
> 
> Yes. But that implies then that you should not be using big-endian
> instruction encodings at all, and simply use the _le variants for both
> LE and BE8

Ok. I understand what you're getting at now.

I believe the linker is the one that does the instruction endian
swap to little endian. So everything is built as big-endian data
and instructions in the assembler phase and then when the linker
runs to generate the final vmlinux elf file it does the swaps to
make instructions little endian. recordmcount runs on the object
files and not the vmlinux file.

For example, the do_undefinstr() function in
arch/arm/kernel/traps.c is one place we nop out. On an le host
and an le build without this patch I see:

(This is all ARM, not thumb)

00000000 <do_undefinstr>:
   0:   e1a0c00d        mov     ip, sp
   4:   e92dd9f0        push    {r4, r5, r6, r7, r8, fp, ip, lr, pc}
   8:   e24cb004        sub     fp, ip, #4
   c:   e24dd08c        sub     sp, sp, #140    ; 0x8c
  10:   e52de004        push    {lr}            ; (str lr, [sp, #-4]!)
  14:   ebfffffe        bl      0 <__gnu_mcount_nc>

After this patch on an le host and le build I see:

00000000 <do_undefinstr>:
   0:   e1a0c00d        mov     ip, sp
   4:   e92dd9f0        push    {r4, r5, r6, r7, r8, fp, ip, lr, pc}
   8:   e24cb004        sub     fp, ip, #4
   c:   e24dd08c        sub     sp, sp, #140    ; 0x8c
  10:   e1a00000        nop                     ; (mov r0, r0)
  14:   e1a00000        nop                     ; (mov r0, r0)

So far so good. Similarly, with this patch and an le host and be
build I see:

00000000 <do_undefinstr>:
   0:   e1a0c00d        mov     ip, sp
   4:   e92dd9f0        push    {r4, r5, r6, r7, r8, fp, ip, lr, pc}
   8:   e24cb004        sub     fp, ip, #4
   c:   e24dd08c        sub     sp, sp, #140    ; 0x8c
  10:   e1a00000        nop                     ; (mov r0, r0)
  14:   e1a00000        nop                     ; (mov r0, r0)

but with *_le instead of *_be used a be build I see:

00000000 <do_undefinstr>:
   0:   e1a0c00d        mov     ip, sp
   4:   e92dd9f0        push    {r4, r5, r6, r7, r8, fp, ip, lr, pc}
   8:   e24cb004        sub     fp, ip, #4
   c:   e24dd08c        sub     sp, sp, #140    ; 0x8c
  10:   0000a0e1        andeq   sl, r0, r1, ror #1
  14:   0000a0e1        andeq   sl, r0, r1, ror #1

I confirmed this by looking at the hexdump of the .exception.text
section for the traps.o object file and the .text section of the
vmlinux file. Basically objcopy the .exception.text of traps.o to
get the first few instructions of the do_undefinstr() function:

$ hexdump -C traps.o 
00000000  e1 a0 c0 0d e9 2d d9 f0  e2 4c b0 04 e2 4d d0 8c

And then objcopy the .text section in vmlinux and seek to the
same function offset (there are a bunch of zeroes in front of it
for padding):

$ hexdump -C vmlinux
...
00001000  0d c0 a0 e1 f0 d9 2d e9  04 b0 4c e2 8c d0 4d e2

As can be seen everything is swapped from the original object
file in big-endian to be in little endian.

Does that allay your concerns?
Ard Biesheuvel Nov. 16, 2016, 11:48 a.m. UTC | #9
On 15 November 2016 at 23:53, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 11/15, Ard Biesheuvel wrote:
>> On 15 November 2016 at 19:18, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> > On 11/15, Ard Biesheuvel wrote:
>> >> On 19 October 2016 at 00:42, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> >> >
>> >> > +static unsigned char ideal_nop4_arm_le[4] = { 0x00, 0x00, 0xa0, 0xe1 }; /* mov r0, r0 */
>> >> > +static unsigned char ideal_nop4_arm_be[4] = { 0xe1, 0xa0, 0x00, 0x00 }; /* mov r0, r0 */
>> >>
>> >> Shouldn't you be taking the difference between BE8 and BE32 into
>> >> account here? IIRC, BE8 uses little endian encoding for instructions.
>> >
>> > I admit I haven't tested on a pre-armv6 CPU so I haven't come
>> > across the case of a BE32 CPU. But from what I can tell that
>> > doesn't matter.
>> >
>> > According to scripts/Makefile.build, cmd_record_mcount only runs
>> > the recordmcount program if CONFIG_FTRACE_MCOUNT_RECORD=y. That
>> > config is defined as:
>> >
>> >         config FTRACE_MCOUNT_RECORD
>> >                 def_bool y
>> >                 depends on DYNAMIC_FTRACE
>> >                 depends on HAVE_FTRACE_MCOUNT_RECORD
>> >
>> >
>> > And in arch/arm/Kconfig we see that DYNAMIC_FTRACE is selected:
>> >
>> >         select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
>> >
>> > which means that FTRACE_MCOUNT_RECORD can't be set when
>> > CPU_ENDIAN_BE32 is set.
>> >
>> > Do you agree that BE32 is not a concern here?
>> >
>>
>> Yes. But that implies then that you should not be using big-endian
>> instruction encodings at all, and simply use the _le variants for both
>> LE and BE8
>
> Ok. I understand what you're getting at now.
>
> I believe the linker is the one that does the instruction endian
> swap to little endian. So everything is built as big-endian data
> and instructions in the assembler phase and then when the linker
> runs to generate the final vmlinux elf file it does the swaps to
> make instructions little endian. recordmcount runs on the object
> files and not the vmlinux file.
>

Very interesting, I did not know that.

> For example, the do_undefinstr() function in
> arch/arm/kernel/traps.c is one place we nop out. On an le host
> and an le build without this patch I see:
>
> (This is all ARM, not thumb)
>
> 00000000 <do_undefinstr>:
>    0:   e1a0c00d        mov     ip, sp
>    4:   e92dd9f0        push    {r4, r5, r6, r7, r8, fp, ip, lr, pc}
>    8:   e24cb004        sub     fp, ip, #4
>    c:   e24dd08c        sub     sp, sp, #140    ; 0x8c
>   10:   e52de004        push    {lr}            ; (str lr, [sp, #-4]!)
>   14:   ebfffffe        bl      0 <__gnu_mcount_nc>
>
> After this patch on an le host and le build I see:
>
> 00000000 <do_undefinstr>:
>    0:   e1a0c00d        mov     ip, sp
>    4:   e92dd9f0        push    {r4, r5, r6, r7, r8, fp, ip, lr, pc}
>    8:   e24cb004        sub     fp, ip, #4
>    c:   e24dd08c        sub     sp, sp, #140    ; 0x8c
>   10:   e1a00000        nop                     ; (mov r0, r0)
>   14:   e1a00000        nop                     ; (mov r0, r0)
>
> So far so good. Similarly, with this patch and an le host and be
> build I see:
>
> 00000000 <do_undefinstr>:
>    0:   e1a0c00d        mov     ip, sp
>    4:   e92dd9f0        push    {r4, r5, r6, r7, r8, fp, ip, lr, pc}
>    8:   e24cb004        sub     fp, ip, #4
>    c:   e24dd08c        sub     sp, sp, #140    ; 0x8c
>   10:   e1a00000        nop                     ; (mov r0, r0)
>   14:   e1a00000        nop                     ; (mov r0, r0)
>
> but with *_le instead of *_be used a be build I see:
>
> 00000000 <do_undefinstr>:
>    0:   e1a0c00d        mov     ip, sp
>    4:   e92dd9f0        push    {r4, r5, r6, r7, r8, fp, ip, lr, pc}
>    8:   e24cb004        sub     fp, ip, #4
>    c:   e24dd08c        sub     sp, sp, #140    ; 0x8c
>   10:   0000a0e1        andeq   sl, r0, r1, ror #1
>   14:   0000a0e1        andeq   sl, r0, r1, ror #1
>
> I confirmed this by looking at the hexdump of the .exception.text
> section for the traps.o object file and the .text section of the
> vmlinux file. Basically objcopy the .exception.text of traps.o to
> get the first few instructions of the do_undefinstr() function:
>
> $ hexdump -C traps.o
> 00000000  e1 a0 c0 0d e9 2d d9 f0  e2 4c b0 04 e2 4d d0 8c
>
> And then objcopy the .text section in vmlinux and seek to the
> same function offset (there are a bunch of zeroes in front of it
> for padding):
>
> $ hexdump -C vmlinux
> ...
> 00001000  0d c0 a0 e1 f0 d9 2d e9  04 b0 4c e2 8c d0 4d e2
>
> As can be seen everything is swapped from the original object
> file in big-endian to be in little endian.
>
> Does that allay your concerns?
>

Yes, it does. Thanks
Steven Rostedt Nov. 16, 2016, 2:08 p.m. UTC | #10
On Wed, 16 Nov 2016 11:48:38 +0000
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> >
> > Does that allay your concerns?
> >  
> 
> Yes, it does. Thanks

Does this mean I can pull this patch into my queue then?

-- Steve
Ard Biesheuvel Nov. 16, 2016, 3:22 p.m. UTC | #11
On 16 November 2016 at 14:08, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 16 Nov 2016 11:48:38 +0000
> Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> >
>> > Does that allay your concerns?
>> >
>>
>> Yes, it does. Thanks
>
> Does this mean I can pull this patch into my queue then?
>

Fine by me

Thanks,
Ard.
diff mbox

Patch

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index 5423a58d1b06..aeb34223167c 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -213,6 +213,59 @@  static int make_nop_x86(void *map, size_t const offset)
 	return 0;
 }
 
+static unsigned char ideal_nop4_arm_le[4] = { 0x00, 0x00, 0xa0, 0xe1 }; /* mov r0, r0 */
+static unsigned char ideal_nop4_arm_be[4] = { 0xe1, 0xa0, 0x00, 0x00 }; /* mov r0, r0 */
+static unsigned char *ideal_nop4_arm;
+
+static unsigned char bl_mcount_arm_le[4] = { 0xfe, 0xff, 0xff, 0xeb }; /* bl */
+static unsigned char bl_mcount_arm_be[4] = { 0xeb, 0xff, 0xff, 0xfe }; /* bl */
+static unsigned char *bl_mcount_arm;
+
+static unsigned char push_arm_le[4] = { 0x04, 0xe0, 0x2d, 0xe5 }; /* push {lr} */
+static unsigned char push_arm_be[4] = { 0xe5, 0x2d, 0xe0, 0x04 }; /* push {lr} */
+static unsigned char *push_arm;
+
+static unsigned char ideal_nop2_thumb_le[2] = { 0x00, 0xbf }; /* nop */
+static unsigned char ideal_nop2_thumb_be[2] = { 0xbf, 0x00 }; /* nop */
+static unsigned char *ideal_nop2_thumb;
+
+static unsigned char push_bl_mcount_thumb_le[6] = { 0x00, 0xb5, 0xff, 0xf7, 0xfe, 0xff }; /* push {lr}, bl */
+static unsigned char push_bl_mcount_thumb_be[6] = { 0xb5, 0x00, 0xf7, 0xff, 0xff, 0xfe }; /* push {lr}, bl */
+static unsigned char *push_bl_mcount_thumb;
+
+static int make_nop_arm(void *map, size_t const offset)
+{
+	char *ptr;
+	int cnt = 1;
+	int nop_size;
+	size_t off = offset;
+
+	ptr = map + offset;
+	if (memcmp(ptr, bl_mcount_arm, 4) == 0) {
+		if (memcmp(ptr - 4, push_arm, 4) == 0) {
+			off -= 4;
+			cnt = 2;
+		}
+		ideal_nop = ideal_nop4_arm;
+		nop_size = 4;
+	} else if (memcmp(ptr - 2, push_bl_mcount_thumb, 6) == 0) {
+		cnt = 3;
+		nop_size = 2;
+		off -= 2;
+		ideal_nop = ideal_nop2_thumb;
+	} else
+		return -1;
+
+	/* Convert to nop */
+	ulseek(fd_map, off, SEEK_SET);
+
+	do {
+		uwrite(fd_map, ideal_nop, nop_size);
+	} while (--cnt > 0);
+
+	return 0;
+}
+
 static unsigned char ideal_nop4_arm64[4] = {0x1f, 0x20, 0x03, 0xd5};
 static int make_nop_arm64(void *map, size_t const offset)
 {
@@ -430,6 +483,11 @@  do_file(char const *const fname)
 			w2 = w2rev;
 			w8 = w8rev;
 		}
+		ideal_nop4_arm = ideal_nop4_arm_le;
+		bl_mcount_arm = bl_mcount_arm_le;
+		push_arm = push_arm_le;
+		ideal_nop2_thumb = ideal_nop2_thumb_le;
+		push_bl_mcount_thumb = push_bl_mcount_thumb_le;
 		break;
 	case ELFDATA2MSB:
 		if (*(unsigned char const *)&endian != 0) {
@@ -438,6 +496,11 @@  do_file(char const *const fname)
 			w2 = w2rev;
 			w8 = w8rev;
 		}
+		ideal_nop4_arm = ideal_nop4_arm_be;
+		bl_mcount_arm = bl_mcount_arm_be;
+		push_arm = push_arm_be;
+		ideal_nop2_thumb = ideal_nop2_thumb_be;
+		push_bl_mcount_thumb = push_bl_mcount_thumb_be;
 		break;
 	}  /* end switch */
 	if (memcmp(ELFMAG, ehdr->e_ident, SELFMAG) != 0
@@ -463,6 +526,8 @@  do_file(char const *const fname)
 		break;
 	case EM_ARM:	 reltype = R_ARM_ABS32;
 			 altmcount = "__gnu_mcount_nc";
+			 make_nop = make_nop_arm;
+			 rel_type_nop = R_ARM_NONE;
 			 break;
 	case EM_AARCH64:
 			reltype = R_AARCH64_ABS64;