[U-Boot] [PATCH 2/6] common/cmd_boot: keep ARM v7M in thumb mode during do_go_exec()

[Matt Porter]

On Wed, Apr 15, 2015 at 08:34:30AM -0400, Tom Rini wrote:
> On Wed, Apr 15, 2015 at 12:33:43PM +0200, Kamil Lulko wrote:
> > 2015-04-14 20:07 GMT+02:00 Matt Porter <mporter at konsulko.com>:
> > >
> > > On ARM v7M, the processor will return to ARM mode when executing
> > > a blx instruction with bit 0 of the address == 0. Always set it
> > > to 1 to stay in thumb mode.
> > >
> > > Signed-off-by: Matt Porter <mporter at konsulko.com>
> > > ---
> > >  common/cmd_boot.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/common/cmd_boot.c b/common/cmd_boot.c
> > > index 8f2e070..20ce652 100644
> > > --- a/common/cmd_boot.c
> > > +++ b/common/cmd_boot.c
> > > @@ -38,6 +38,10 @@ static int do_go(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> > >          * pass address parameter as argv[0] (aka command name),
> > >          * and all remaining args
> > >          */
> > > +#ifdef CONFIG_CPU_V7M
> > > +       /* For ARM V7M, set bit zero to stay in Thumb mode */
> > > +       addr++;
> > > +#endif
> > >         rc = do_go_exec ((void *)addr, argc - 1, argv + 1);
> > >         if (rc != 0) rcode = 1;
> > 
> > I think addr |= 1 would be better - there is always a possibility that
> > kernel image has the zero bit already set (this is the case in my own
> > Buildroot build setup I am using for STM32F4 builds). Anyways -
> > keeping this bit set should be the responsibility of kernel image
> > build process so such machine specific quirk can be kept out of the
> > common code.
> 
> I'd agree about |='ing in 1.  But it's not a machine quirk, it's a
> requirement of the Cortex-M family that you not exit Thumb-mode (since
> it's Thumb-only) and given how we end up trying to jump to the address
> (here or in 'go' which Matt didn't post the patch for, but same logic)
> we can / will have problems if we don't do this.  You can work around it
> for images we throw a header into but 'go' is where this gets really
> annoying.

"|1" is indeed much better, I will update it for that implementation. Just
to be clear, this patch *is* for the go command which is what I use to
load my RTOS. Kamil's comment implies the bootm path which I don't touch
at all since I don't even support it in this config (we're running full
U-Boot from SRAM and the CMD_BOOTM support is quite large and not
necessary for my application).

I understand the concern about cluttering up common code with this. An
alternative is to force this knowledge on the user such that they need
to "go 080203a9" to run a Thumb-2 application located at 0x080203a8.
It's ugly, but could be documented. I'd rather see an address fixup
function or similar approach if we want to avoid cluttering the common
path with ifdefry.

-Matt
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150415/090b775f/attachment.sig>