| Message ID | 20221005163849.3205046-2-andre.przywara@arm.com |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | arm: smh: Fix and improve semihosting traps | expand |
On Wed, Oct 05, 2022 at 05:38:47PM +0100, Andre Przywara wrote: > The ARM semihosting interface uses different trap instructions for > different architectures and instruction sets. So far we were using > AArch64 and ARMv7-M, and had an untested v7-A entry. The latter does > not work when building for Thumb, as can be verified by using > qemu_arm_defconfig, then enabling SEMIHOSTING and SYS_THUMB_BUILD: > ========== > {standard input}:35: Error: invalid swi expression > {standard input}:35: Error: value of 1193046 too large for field of 2 bytes at 0 > ========== > > Fix this by providing the recommended instruction[1] for Thumb, and > using the ARM instruction only when not building for Thumb. This also > removes some comment, as QEMU for ARM allows to now test this case. > Also use the opportunity to clean up the inline assembly, and just define > the actual trap instruction inside #ifdef's, to improve readability. > > [1] https://developer.arm.com/documentation/dui0471/g/Semihosting/The-semihosting-interface?lang=en > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> Applied to u-boot/master, thanks!
diff --git a/arch/arm/lib/semihosting.c b/arch/arm/lib/semihosting.c index 01d652a6b83..acc6b1be4f3 100644 --- a/arch/arm/lib/semihosting.c +++ b/arch/arm/lib/semihosting.c @@ -4,11 +4,6 @@ * Copyright 2014 Broadcom Corporation */ -/* - * This code has been tested on arm64/aarch64 fastmodel only. An untested - * placeholder exists for armv7 architectures, but since they are commonly - * available in silicon now, fastmodel usage makes less sense for them. - */ #include <common.h> #include <log.h> #include <semihosting.h> @@ -25,20 +20,28 @@ #define SYSFLEN 0x0C #define SYSERRNO 0x13 +#if defined(CONFIG_ARM64) + #define SMH_TRAP "hlt #0xf000" +#elif defined(CONFIG_CPU_V7M) + #define SMH_TRAP "bkpt #0xAB" +#elif defined(CONFIG_SYS_THUMB_BUILD) + #define SMH_TRAP "svc #0xab" +#else + #define SMH_TRAP "svc #0x123456" +#endif + /* * Call the handler */ static noinline long smh_trap(unsigned int sysnum, void *addr) { register long result asm("r0"); -#if defined(CONFIG_ARM64) - asm volatile ("hlt #0xf000" : "=r" (result) : "0"(sysnum), "r"(addr) : "memory"); -#elif defined(CONFIG_CPU_V7M) - asm volatile ("bkpt #0xAB" : "=r" (result) : "0"(sysnum), "r"(addr) : "memory"); -#else - /* Note - untested placeholder */ - asm volatile ("svc #0x123456" : "=r" (result) : "0"(sysnum), "r"(addr) : "memory"); -#endif + + asm volatile (SMH_TRAP + : "=r" (result) + : "0"(sysnum), "r"(addr) + : "memory"); + return result; }
The ARM semihosting interface uses different trap instructions for different architectures and instruction sets. So far we were using AArch64 and ARMv7-M, and had an untested v7-A entry. The latter does not work when building for Thumb, as can be verified by using qemu_arm_defconfig, then enabling SEMIHOSTING and SYS_THUMB_BUILD: ========== {standard input}:35: Error: invalid swi expression {standard input}:35: Error: value of 1193046 too large for field of 2 bytes at 0 ========== Fix this by providing the recommended instruction[1] for Thumb, and using the ARM instruction only when not building for Thumb. This also removes some comment, as QEMU for ARM allows to now test this case. Also use the opportunity to clean up the inline assembly, and just define the actual trap instruction inside #ifdef's, to improve readability. [1] https://developer.arm.com/documentation/dui0471/g/Semihosting/The-semihosting-interface?lang=en Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- arch/arm/lib/semihosting.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-)