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

[Felipe Balbi]

Hi,

On Tue, Apr 21, 2015 at 03:07:48PM -0400, Tom Rini wrote:
> On Tue, Apr 21, 2015 at 01:05:24PM -0500, Felipe Balbi wrote:
> > On Tue, Apr 21, 2015 at 02:01:31PM -0400, Matt Porter wrote:
> > > On Tue, Apr 21, 2015 at 12:47:24PM -0500, Felipe Balbi wrote:
> > > > On Tue, Apr 21, 2015 at 01:36:54PM -0400, Matt Porter wrote:
> > > > > 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
> > > > 
> > > > but that's what the 'x' is for, right ? eXchange the CPU mode.
> > > 
> > > Yes.
> > > 
> > > > > 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
> > > > 
> > > > what if we were in ARM state when we reached this point ? You're now
> > > > telling CPU to always switch to Thumb. Is this really what we want ?
> > > 
> > > We have no ARM state on this core so that's not possible.
> > > 
> > > > From ARM's instruction manual:
> > > > 
> > > > 
> > > > 
> > > > "
> > > > The BX and BLX instructions can change the processor state from ARM to
> > > > Thumb, or from Thumb to ARM.
> > > > 
> > > > BLX label always changes the state.
> > > > 
> > > > BX Rm and BLX Rm derive the target state from bit[0] of Rm:
> > > > 
> > > >     if bit[0] of Rm is 0, the processor changes to, or remains in, ARM
> > > >     state
> > > > 
> > > >     if bit[0] of Rm is 1, the processor changes to, or remains in, Thumb
> > > >     state.
> > > 
> > > Correct. The last statement is why this patch exists. There is no ARM
> > > mode on M3, we immediately fault. Having bit[0]=0 for BX/BLX is not
> > > permitted on V7M...something not covered in the generic description
> > > of these instructions.
> > 
> > it just seems weird that bit0 wouldn't just be assume 1 by the core
> > itself. I suppose as a consequence we can't use blx label with v7m
> > either ? :-s
> 
> At issue is that the code really reads like this (expanding the
> functions a little bit:
> printf ("## Starting application at 0x%08lX ...\n", addr);
> rc = (void *)addr(argc - 1, argv + 1);
> 
> So the compiler translates this as "do what I say" and generates a
> branch to whatever addr is.

yeah, you're right. There's nothing the compiler can do. Well, it could
refuse from using bx/blx instructions, would a 'bl' do the same thing in
this particular case ? (yes, off-topic, sorry)

> If people really feel strongly about it being too wierd to do "addr |=
> 1" in the common code, do_go_exec is a weak function and we can put
> something into arch/arm/lib/ (new file) that provides a do_go_exec for
> Cortex-M and that ensures that we have the right bit set for the branch
> instruction that will be generated.

I think that's probably a lot clearer, yes.

-- 
balbi
-------------- 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/20150421/e5d1057e/attachment.sig>