| Message ID | 1429034842-5260-3-git-send-email-mporter@konsulko.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
2015-04-14 20:07 GMT+02:00 Matt Porter <mporter@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@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; > > -- > 2.1.0 > > 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. /Kamil
On Wed, Apr 15, 2015 at 12:33:43PM +0200, Kamil Lulko wrote: > 2015-04-14 20:07 GMT+02:00 Matt Porter <mporter@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@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.
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@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@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
Hello Matt, On Tue, 14 Apr 2015 14:07:18 -0400, Matt Porter <mporter@konsulko.com> 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 > to 1 to stay in thumb mode. This should be done for all targets which build with Thumb instruction set, not only ARMv7M, should it not? > Signed-off-by: Matt Porter <mporter@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; > > -- > 2.1.0 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot Amicalement,
On Thu, Apr 16, 2015 at 03:53:56PM +0200, Albert ARIBAUD wrote: > Hello Matt, > > On Tue, 14 Apr 2015 14:07:18 -0400, Matt Porter <mporter@konsulko.com> > 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 > > to 1 to stay in thumb mode. > > This should be done for all targets which build with Thumb instruction > set, not only ARMv7M, should it not? No, because it's not a problem on CPUs where we can do Thumb or non-Thumb instructions, only on CPUs where we can _only_ do Thumb instructions (or the special subset of "other" instructions the core allows). On ARM v7M trying to do the blx in ARM mode isn't allowed and causes an abort (or some other exception, I forget which) to happen upon execution.
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;
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@konsulko.com> --- common/cmd_boot.c | 4 ++++ 1 file changed, 4 insertions(+)