Unaligned access in ubldr.bin

[Warner Losh]

On Thu, Sep 8, 2016 at 5:23 PM, Erik Moe <e.moe at rcn.com> wrote:
>
> On Sep 7, 2016, at 8:22 AM, Warner Losh <imp at bsdimp.com> wrote:
>
> On Wed, Sep 7, 2016 at 12:46 AM, Erik Moe <e.moe at rcn.com> wrote:
>
> Hello,
>
> I’ve been trying to bring up FreeBSD on the USB Armory based on the i.MX53.
> I’ve made myself a u-boot patch that will load and start ubldr.bin but I’m
> running into a issue with what I think is an unaligned access issue:
>
>
> which version of u-boot are you using?
>
>
> I was using 2016.07, but also tried u-boot from head (b615267).

When forward porting the u-boot in the ports tree, I noticed that 2016.05 worked
w/o changing anything in ubldr and 2016.07 and head from a week ago both
gave me unaligned accesses. Maybe you could try it as an experiment? Failed
in exactly the same way on the two boards I tried it on (pandaboard and BBB).

> ## Starting application at 0x70800000 ...
> data abort
> pc : [<70824bac>]          lr : [<7081846c>]
> sp : 8f550c98  ip : 70835a00     fp : 8f550cb0
> r10: 00000002  r9 : 70838d58     r8 : 70833cd9
> r7 : 707fff08  r6 : 000054f0     r5 : 70833cd9  r4 : 00000000
> r3 : 70828fc4  r2 : 70833cd9     r1 : 00000001  r0 : 7083705c
> Flags: Nzcv  IRQs off  FIQs off  Mode SVC_32
> Resetting CPU ...
>
> I’ve hand dissambled the code at  pc=0x70824bac:
>
> 0x70824ba8: 0xe59f21d0 ldr r2, [pc, #464]
> 0x70824bac: 0xe5825000 str r5, [r2]
> 0x70824bb0: 0xe5d53000 ldrb r3, [r5]
> 0x70824bb4: 0xe353002d cmp r3, #45
> 0x70824bb8: 0x1a00000b bne #+44
>
> The offending instructions is "str r5, [r2]” where r2 = 0x70833cd9, which
> sort of makes sense since it isn’t aligned on a 4 byte boundary.  I’m new to
> arm ARM, so I’m not really sure.  My questions are these:
>
> 1.) Doesn’t ARMv6 and higher architecture allow unaligned access?  Is there
> something that u-boot needs be doing in initialization to allow unaligned
> access?
>
>
> It does, but usually that's configured later in boot.
>
> 2.) Does ubldr make the assumption that unaligned access is allowed and
> maybe shouldn’t?  I would think not since ubldr has been around for a while
> and works on numerous ARM processors.
>
>
> It may be that the compiler is generating bad code in this case? You
> might check to see how we're building it and to see if that's causing
> problems.
>
>
> Per Michal Stanek’s suggestion I tried adding "-mno-unaligned-access” to the
> build of ubldr.  It’s still failing in getopt,  though not it the same spot:

How about adding it to the ubldr build? IIRC, it should be default in
FreeBSD's clang though.

> ## Starting application at 0x70800000 ...
> data abort
> pc : [<70824bd0>]          lr : [<70818454>]
> sp : 8f559a78  ip : 70835a90     fp : 8f559a90
> r10: 00000002  r9 : 70838df0     r8 : 70833d73
> r7 : 707fff08  r6 : 00005530     r5 : 70833d73  r4 : 00000000
> r3 : 70828fe8  r2 : 70833d73     r1 : 00000001  r0 : 708370ec
> Flags: Nzcv  IRQs off  FIQs off  Mode SVC_32
> Resetting CPU ...
>
> 0x70824bcc 0xe59f21d0 ldr r2, [pc, #464]
> 0x70824bd0 0xe5825000 str r5, [r2]
> 0x70824bd4 0xe5d53000 ldrb r3, [r5]
> 0x70824bd8 0xe353002d cmp r3, #45
>
> 70833d72: 00 61 3a 00 55 42 6f 6f 74 41 50 49 00 49 44       .a:.UBootAPI.ID
>
> r2 contains 70833d73 which points to “0x61 0x3a 0x00”, which is the literal
> string “a:”.  Looking at the lr register the calling function is
> “api_parse_cmdline_sig”.  Looking at the code:
>
> api_parse_cmdline_sig(int argc, char **argv, struct api_signature **sig)
> {
>         unsigned long api_address;
>         int c;
>
>         api_address = 0;
>         opterr = 0;
>         optreset = 1;
>         optind = 1;
>
>         while ((c = getopt (argc, argv, "a:")) != -1)
>                 switch (c) {
>                 case 'a':
>                         api_address = strtoul(optarg, NULL, 16);
>                         break;
>                 default:
>                         break;
>                 }
>
> It’s the third argument to getopt that seems to be on an odd address.  As
> for the SCTLR.A bit being enabled, that is definitely something u-boot does,
> because the default state is for it to be cleared after reset.  I see this
> in arch/arm/cpu/armv7/start.S:
>
>         /*
>          * disable MMU stuff and caches
>          */
>         mrc     p15, 0, r0, c1, c0, 0
>         bic     r0, r0, #0x00002000     @ clear bits 13 (--V-)
>         bic     r0, r0, #0x00000007     @ clear bits 2:0 (-CAM)
>         orr     r0, r0, #0x00000002     @ set bit 1 (--A-) Align
>         orr     r0, r0, #0x00000800     @ set bit 11 (Z---) BTB
> #ifdef CONFIG_SYS_ICACHE_OFF
>         bic     r0, r0, #0x00001000     @ clear bit 12 (I) I-cache
> #else
>         orr     r0, r0, #0x00001000     @ set bit 12 (I) I-cache
> #endif
>         mcr     p15, 0, r0, c1, c0, 0
>
> I tried to clear that flag, but that didn’t work either, but I’m not sure if
> this is correct:
>
>      mac p15, 0, r0, c1, c0, 0
>      bic r0, r0, #0x00000002
>      mar p15, 0, r0, c1, c0, 0

That looks like it should do the right thing. You could just remove
the orr line that ors in bit 1 too. Have you tried that?

Warner