* Re: [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000
@ 2015-04-09 17:23 Christopher Covington
2015-04-09 17:57 ` Peter Maydell
0 siblings, 1 reply; 10+ messages in thread
From: Christopher Covington @ 2015-04-09 17:23 UTC (permalink / raw)
To: Peter Maydell; +Cc: Liviu Ionescu, QEMU Developers
Hi Peter,
On Fri, Mar 27, 2015 at 12:40 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
>> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
>> index 0b192a1..3b5b875 100644
>> --- a/target-arm/translate-a64.c
>> +++ b/target-arm/translate-a64.c
>> @@ -1544,7 +1544,11 @@ static void disas_exc(DisasContext *s, uint32_t insn)
>> break;
>> }
>> /* HLT */
>> - unsupported_encoding(s, insn);
>> + if (imm16 == 0xf000) {
>
> You need to have the semihosting_enabled check here rather
> than in the do_interrupt code, because otherwise we won't
> behave correctly in the disabled case.
Do you have suggestions for getting semihosting_enabled defined in
translate-a64.c? I'm likely doing something dumb, but while #include
"sysemu/sysemu.h" at first seemed like the obvious approach, and
appears to work for -softmmu, I'm getting errors with that when
building -linux-user.
Thanks,
Chris
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000 2015-04-09 17:23 [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000 Christopher Covington @ 2015-04-09 17:57 ` Peter Maydell 2015-04-10 15:47 ` Christopher Covington 2015-04-23 11:00 ` Leon Alrae 0 siblings, 2 replies; 10+ messages in thread From: Peter Maydell @ 2015-04-09 17:57 UTC (permalink / raw) To: Christopher Covington; +Cc: Liviu Ionescu, QEMU Developers On 9 April 2015 at 18:23, Christopher Covington <christopher.covington@linaro.org> wrote: > On Fri, Mar 27, 2015 at 12:40 PM, Peter Maydell > <peter.maydell@linaro.org> wrote: >> You need to have the semihosting_enabled check here rather >> than in the do_interrupt code, because otherwise we won't >> behave correctly in the disabled case. > > Do you have suggestions for getting semihosting_enabled defined in > translate-a64.c? I'm likely doing something dumb, but while #include > "sysemu/sysemu.h" at first seemed like the obvious approach, and > appears to work for -softmmu, I'm getting errors with that when > building -linux-user. sysemu.h is an ugly grab-bag of things that are specific to the system emulator config. Semihosting is always enabled for linux-user, so it doesn't have an equivalent switch. Let's clean this up a bit, by creating a new include/exec/semihost.h, with contents something like: #ifdef CONFIG_SOFTMMU bool semihosting_enabled(void); #else /* In -user configurations semihosting is always enabled */ bool semihosting_enabled(void) { return true; } #endif [plus license header, ifdef guards, etc] and then in vl.c have a static bool semihosting_allowed; and bool semihosting_enabled(void) { return semihosting_allowed; } and change the places that set semihosting_enabled to set this private flag instead. [This is following the pattern of the existing defaults_enabled/usb_enabled functions.] And change all the tests of semihosting_enabled to calls to semihosting_enabled(), and delete the semihosting_enabled variable completely. Then we have a place to put future semihosting related config things, and a small self-contained header you can include in translate-a64.c without dragging in a lot of rubbish. This should all go in a preliminary patch at the start of your series. thanks -- PMM ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000 2015-04-09 17:57 ` Peter Maydell @ 2015-04-10 15:47 ` Christopher Covington 2015-04-10 15:52 ` Peter Maydell 2015-04-23 11:00 ` Leon Alrae 1 sibling, 1 reply; 10+ messages in thread From: Christopher Covington @ 2015-04-10 15:47 UTC (permalink / raw) To: Peter Maydell; +Cc: Liviu Ionescu, QEMU Developers Hi Peter, On Thu, Apr 9, 2015 at 1:57 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 9 April 2015 at 18:23, Christopher Covington > <christopher.covington@linaro.org> wrote: >> On Fri, Mar 27, 2015 at 12:40 PM, Peter Maydell >> <peter.maydell@linaro.org> wrote: >>> You need to have the semihosting_enabled check here rather >>> than in the do_interrupt code, because otherwise we won't >>> behave correctly in the disabled case. >> >> Do you have suggestions for getting semihosting_enabled defined in >> translate-a64.c? I'm likely doing something dumb, but while #include >> "sysemu/sysemu.h" at first seemed like the obvious approach, and >> appears to work for -softmmu, I'm getting errors with that when >> building -linux-user. > > sysemu.h is an ugly grab-bag of things that are specific to the > system emulator config. Semihosting is always enabled for linux-user, > so it doesn't have an equivalent switch. Why are Angel semihosting calls supported in arm-linux-user? The use case isn't obvious to me. Could omitting semihosting support in aarch64-linux-user simplify development and maintenance? (I happen to have some binaries that make both Angel and Linux calls, but I use them on arm-softmmu [after hacking out the curious restriction on semihosting calls from USR], aarch64-softmmu, and non-QEMU system emulators. The most frequently used tools are for passthrough file operations, which I'm replacing with VirtIO-MMIO 9P mounts, and an exit utility, which I'd like to replace with (PSCI-backed?) poweroff. There is also a take-a-checkpoint-now tool and a current-commandline tool that I'm not yet sure how to replace.) Thanks, Chris ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000 2015-04-10 15:47 ` Christopher Covington @ 2015-04-10 15:52 ` Peter Maydell 0 siblings, 0 replies; 10+ messages in thread From: Peter Maydell @ 2015-04-10 15:52 UTC (permalink / raw) To: Christopher Covington; +Cc: Liviu Ionescu, QEMU Developers On 10 April 2015 at 16:47, Christopher Covington <christopher.covington@linaro.org> wrote: > Why are Angel semihosting calls supported in arm-linux-user? The use > case isn't obvious to me. Could omitting semihosting support in > aarch64-linux-user simplify development and maintenance? They're used for simple bare-metal single-ELF-file tests (eg gcc test cases) -- this is an important use case, arguably more useful than semihosting in system-emulation mode. -- PMM ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000 2015-04-09 17:57 ` Peter Maydell 2015-04-10 15:47 ` Christopher Covington @ 2015-04-23 11:00 ` Leon Alrae 2015-04-23 17:39 ` Christopher Covington 1 sibling, 1 reply; 10+ messages in thread From: Leon Alrae @ 2015-04-23 11:00 UTC (permalink / raw) To: Christopher Covington; +Cc: Peter Maydell, QEMU Developers, Liviu Ionescu Hi Christopher, On 09/04/2015 18:57, Peter Maydell wrote: > On 9 April 2015 at 18:23, Christopher Covington > <christopher.covington@linaro.org> wrote: >> On Fri, Mar 27, 2015 at 12:40 PM, Peter Maydell >> <peter.maydell@linaro.org> wrote: >>> You need to have the semihosting_enabled check here rather >>> than in the do_interrupt code, because otherwise we won't >>> behave correctly in the disabled case. >> >> Do you have suggestions for getting semihosting_enabled defined in >> translate-a64.c? I'm likely doing something dumb, but while #include >> "sysemu/sysemu.h" at first seemed like the obvious approach, and >> appears to work for -softmmu, I'm getting errors with that when >> building -linux-user. > > sysemu.h is an ugly grab-bag of things that are specific to the > system emulator config. Semihosting is always enabled for linux-user, > so it doesn't have an equivalent switch. > > Let's clean this up a bit, by creating a new include/exec/semihost.h, Do you happen to have this clean up available somewhere? I just want to make sure I'm not duplicating the work as my patches will touch the same area. Thanks, Leon ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000 2015-04-23 11:00 ` Leon Alrae @ 2015-04-23 17:39 ` Christopher Covington 0 siblings, 0 replies; 10+ messages in thread From: Christopher Covington @ 2015-04-23 17:39 UTC (permalink / raw) To: Leon Alrae Cc: Christopher Covington, Liviu Ionescu, QEMU Developers, Peter Maydell Hi Leon, On 04/23/2015 07:00 AM, Leon Alrae wrote: > Hi Christopher, > > On 09/04/2015 18:57, Peter Maydell wrote: >> On 9 April 2015 at 18:23, Christopher Covington >> <christopher.covington@linaro.org> wrote: >>> On Fri, Mar 27, 2015 at 12:40 PM, Peter Maydell >>> <peter.maydell@linaro.org> wrote: >>>> You need to have the semihosting_enabled check here rather >>>> than in the do_interrupt code, because otherwise we won't >>>> behave correctly in the disabled case. >>> >>> Do you have suggestions for getting semihosting_enabled defined in >>> translate-a64.c? I'm likely doing something dumb, but while #include >>> "sysemu/sysemu.h" at first seemed like the obvious approach, and >>> appears to work for -softmmu, I'm getting errors with that when >>> building -linux-user. >> >> sysemu.h is an ugly grab-bag of things that are specific to the >> system emulator config. Semihosting is always enabled for linux-user, >> so it doesn't have an equivalent switch. >> >> Let's clean this up a bit, by creating a new include/exec/semihost.h, > > Do you happen to have this clean up available somewhere? I just want to > make sure I'm not duplicating the work as my patches will touch the same > area. I'm afraid I've not yet had a chance to clean this up, and I'm not sure when I'll be able to get to it. Chris -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] arm: semihosting: Preliminary AArch64 support @ 2015-03-27 16:22 Christopher Covington 2015-03-27 16:22 ` [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000 Christopher Covington 0 siblings, 1 reply; 10+ messages in thread From: Christopher Covington @ 2015-03-27 16:22 UTC (permalink / raw) To: qemu-devel, Peter Maydell; +Cc: Liviu Ionescu Hi, Here are a few patches preparing for and adding AArch64 Angel semihosting support. I've been testing them with some simple tests from the following repository. This series only adds support for exit, but support for the rest of the calls will hopefully follow shortly. http://git.linaro.org/people/christopher.covington/angel-semihosting.git Thanks, Chris ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000 2015-03-27 16:22 [Qemu-devel] arm: semihosting: Preliminary AArch64 support Christopher Covington @ 2015-03-27 16:22 ` Christopher Covington 2015-03-27 16:40 ` Peter Maydell 0 siblings, 1 reply; 10+ messages in thread From: Christopher Covington @ 2015-03-27 16:22 UTC (permalink / raw) To: qemu-devel, Peter Maydell; +Cc: Liviu Ionescu, Christopher Covington In AArch64, Angel semihosting uses HLT instructions with a special immediate value, 0xf000. Use a new exception type EXCP_SEMI for this, since I'm not aware of plans for full HLT support, and EXCP_HLT is already defined as a QEMU internal exception type. Support just the exit call in A64 to start with. Signed-off-by: Christopher Covington <christopher.covington@linaro.org> --- target-arm/arm-semi.c | 14 +++++++++++--- target-arm/cpu.h | 3 ++- target-arm/helper-a64.c | 11 +++++++++++ target-arm/internals.h | 1 + target-arm/translate-a64.c | 6 +++++- 5 files changed, 30 insertions(+), 5 deletions(-) diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c index d915123..c71d47a 100644 --- a/target-arm/arm-semi.c +++ b/target-arm/arm-semi.c @@ -191,7 +191,7 @@ static void QEMU_NORETURN unsupported_semihosting(uint32_t nr, CPUState *cs) } while (0) #define SET_ARG(n, val) put_user_ual(val, args + (n) * 4) -uint32_t do_arm_semihosting(CPUARMState *env) +target_ulong do_arm_semihosting(CPUARMState *env) { ARMCPU *cpu = arm_env_get_cpu(env); CPUState *cs = CPU(cpu); @@ -207,8 +207,16 @@ uint32_t do_arm_semihosting(CPUARMState *env) CPUARMState *ts = env; #endif - nr = env->regs[0]; - args = env->regs[1]; + if (is_a64(env)) { + nr = env->xregs[0]; + args = env->xregs[1]; + if (nr != env->xregs[0] || nr != TARGET_SYS_EXIT) { + unsupported_semihosting(nr, cs); + } + } else { + nr = env->regs[0]; + args = env->regs[1]; + } switch (nr) { case TARGET_SYS_OPEN: GET_ARG(0); diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 083211c..d5c5cfa 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -56,6 +56,7 @@ #define EXCP_SMC 13 /* Secure Monitor Call */ #define EXCP_VIRQ 14 #define EXCP_VFIQ 15 +#define EXCP_SEMI 16 /* Angel Semihosting Call */ #define ARMV7M_EXCP_RESET 1 #define ARMV7M_EXCP_NMI 2 @@ -494,7 +495,7 @@ typedef struct CPUARMState { ARMCPU *cpu_arm_init(const char *cpu_model); int cpu_arm_exec(CPUARMState *s); -uint32_t do_arm_semihosting(CPUARMState *env); +target_ulong do_arm_semihosting(CPUARMState *env); void aarch64_sync_32_to_64(CPUARMState *env); void aarch64_sync_64_to_32(CPUARMState *env); diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c index 7e0d038..9ecd488 100644 --- a/target-arm/helper-a64.c +++ b/target-arm/helper-a64.c @@ -514,6 +514,17 @@ void aarch64_cpu_do_interrupt(CPUState *cs) case EXCP_VFIQ: addr += 0x100; break; + case EXCP_SEMI: + if (semihosting_enabled) { + qemu_log_mask(CPU_LOG_INT, + "...handling as semihosting call 0x%" PRIx64 "\n", + env->xregs[0]); + env->xregs[0] = do_arm_semihosting(env); + break; + } else { + qemu_log_mask(CPU_LOG_INT, + "...not handled because semihosting is disabled\n"); + } default: cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index); } diff --git a/target-arm/internals.h b/target-arm/internals.h index bb171a7..86b5854 100644 --- a/target-arm/internals.h +++ b/target-arm/internals.h @@ -58,6 +58,7 @@ static const char * const excnames[] = { [EXCP_SMC] = "Secure Monitor Call", [EXCP_VIRQ] = "Virtual IRQ", [EXCP_VFIQ] = "Virtual FIQ", + [EXCP_SEMI] = "Angel Semihosting Call", }; static inline void arm_log_exception(int idx) diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c index 0b192a1..3b5b875 100644 --- a/target-arm/translate-a64.c +++ b/target-arm/translate-a64.c @@ -1544,7 +1544,11 @@ static void disas_exc(DisasContext *s, uint32_t insn) break; } /* HLT */ - unsupported_encoding(s, insn); + if (imm16 == 0xf000) { + gen_exception_insn(s, 4, EXCP_SEMI, 0); + } else { + unsupported_encoding(s, insn); + } break; case 5: if (op2_ll < 1 || op2_ll > 3) { -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000 2015-03-27 16:22 ` [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000 Christopher Covington @ 2015-03-27 16:40 ` Peter Maydell 2015-03-28 12:27 ` Christopher Covington 0 siblings, 1 reply; 10+ messages in thread From: Peter Maydell @ 2015-03-27 16:40 UTC (permalink / raw) To: Christopher Covington; +Cc: Liviu Ionescu, QEMU Developers On 27 March 2015 at 16:22, Christopher Covington <christopher.covington@linaro.org> wrote: > In AArch64, Angel semihosting uses HLT instructions with a special > immediate value, 0xf000. Use a new exception type EXCP_SEMI for this, > since I'm not aware of plans for full HLT support, and EXCP_HLT is > already defined as a QEMU internal exception type. Support just the > exit call in A64 to start with. > > Signed-off-by: Christopher Covington <christopher.covington@linaro.org> > --- > target-arm/arm-semi.c | 14 +++++++++++--- > target-arm/cpu.h | 3 ++- > target-arm/helper-a64.c | 11 +++++++++++ > target-arm/internals.h | 1 + > target-arm/translate-a64.c | 6 +++++- > 5 files changed, 30 insertions(+), 5 deletions(-) > > diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c > index d915123..c71d47a 100644 > --- a/target-arm/arm-semi.c > +++ b/target-arm/arm-semi.c > @@ -191,7 +191,7 @@ static void QEMU_NORETURN unsupported_semihosting(uint32_t nr, CPUState *cs) > } while (0) > > #define SET_ARG(n, val) put_user_ual(val, args + (n) * 4) > -uint32_t do_arm_semihosting(CPUARMState *env) > +target_ulong do_arm_semihosting(CPUARMState *env) > { > ARMCPU *cpu = arm_env_get_cpu(env); > CPUState *cs = CPU(cpu); > @@ -207,8 +207,16 @@ uint32_t do_arm_semihosting(CPUARMState *env) > CPUARMState *ts = env; > #endif > > - nr = env->regs[0]; > - args = env->regs[1]; > + if (is_a64(env)) { > + nr = env->xregs[0]; > + args = env->xregs[1]; > + if (nr != env->xregs[0] || nr != TARGET_SYS_EXIT) { What is the first part of this if condition intended to do? (Note that the semihosting API number is passed in W0, not X0...) > + unsupported_semihosting(nr, cs); > + } > + } else { > + nr = env->regs[0]; > + args = env->regs[1]; > + } > switch (nr) { > case TARGET_SYS_OPEN: > GET_ARG(0); This doesn't look right -- in the AArch64 version the SYS_EXIT (aka AngelSWI_Reason_ReportException) takes a parameter block, so you can't just fall through to the existing code without modifying it to support this. It's also a bit difficult to tell if this code is right without seeing some actual implementations of the semihosting calls. > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index 083211c..d5c5cfa 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -56,6 +56,7 @@ > #define EXCP_SMC 13 /* Secure Monitor Call */ > #define EXCP_VIRQ 14 > #define EXCP_VFIQ 15 > +#define EXCP_SEMI 16 /* Angel Semihosting Call */ > > #define ARMV7M_EXCP_RESET 1 > #define ARMV7M_EXCP_NMI 2 > @@ -494,7 +495,7 @@ typedef struct CPUARMState { > > ARMCPU *cpu_arm_init(const char *cpu_model); > int cpu_arm_exec(CPUARMState *s); > -uint32_t do_arm_semihosting(CPUARMState *env); > +target_ulong do_arm_semihosting(CPUARMState *env); > void aarch64_sync_32_to_64(CPUARMState *env); > void aarch64_sync_64_to_32(CPUARMState *env); > > diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c > index 7e0d038..9ecd488 100644 > --- a/target-arm/helper-a64.c > +++ b/target-arm/helper-a64.c > @@ -514,6 +514,17 @@ void aarch64_cpu_do_interrupt(CPUState *cs) > case EXCP_VFIQ: > addr += 0x100; > break; > + case EXCP_SEMI: > + if (semihosting_enabled) { > + qemu_log_mask(CPU_LOG_INT, > + "...handling as semihosting call 0x%" PRIx64 "\n", > + env->xregs[0]); > + env->xregs[0] = do_arm_semihosting(env); > + break; > + } else { > + qemu_log_mask(CPU_LOG_INT, > + "...not handled because semihosting is disabled\n"); > + } > default: > cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index); > } > diff --git a/target-arm/internals.h b/target-arm/internals.h > index bb171a7..86b5854 100644 > --- a/target-arm/internals.h > +++ b/target-arm/internals.h > @@ -58,6 +58,7 @@ static const char * const excnames[] = { > [EXCP_SMC] = "Secure Monitor Call", > [EXCP_VIRQ] = "Virtual IRQ", > [EXCP_VFIQ] = "Virtual FIQ", > + [EXCP_SEMI] = "Angel Semihosting Call", > }; > > static inline void arm_log_exception(int idx) > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > index 0b192a1..3b5b875 100644 > --- a/target-arm/translate-a64.c > +++ b/target-arm/translate-a64.c > @@ -1544,7 +1544,11 @@ static void disas_exc(DisasContext *s, uint32_t insn) > break; > } > /* HLT */ > - unsupported_encoding(s, insn); > + if (imm16 == 0xf000) { You need to have the semihosting_enabled check here rather than in the do_interrupt code, because otherwise we won't behave correctly in the disabled case. > + gen_exception_insn(s, 4, EXCP_SEMI, 0); > + } else { > + unsupported_encoding(s, insn); > + } > break; > case 5: > if (op2_ll < 1 || op2_ll > 3) { > -- > 1.9.1 thanks -- PMM ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000 2015-03-27 16:40 ` Peter Maydell @ 2015-03-28 12:27 ` Christopher Covington 2015-03-31 11:22 ` Peter Maydell 0 siblings, 1 reply; 10+ messages in thread From: Christopher Covington @ 2015-03-28 12:27 UTC (permalink / raw) To: Peter Maydell; +Cc: Liviu Ionescu, QEMU Developers Hi Peter, On Fri, Mar 27, 2015 at 12:40 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 27 March 2015 at 16:22, Christopher Covington > <christopher.covington@linaro.org> wrote: >> In AArch64, Angel semihosting uses HLT instructions with a special >> immediate value, 0xf000. Use a new exception type EXCP_SEMI for this, >> since I'm not aware of plans for full HLT support, and EXCP_HLT is >> already defined as a QEMU internal exception type. Support just the >> exit call in A64 to start with. >> >> Signed-off-by: Christopher Covington <christopher.covington@linaro.org> >> --- >> target-arm/arm-semi.c | 14 +++++++++++--- >> target-arm/cpu.h | 3 ++- >> target-arm/helper-a64.c | 11 +++++++++++ >> target-arm/internals.h | 1 + >> target-arm/translate-a64.c | 6 +++++- >> 5 files changed, 30 insertions(+), 5 deletions(-) >> >> diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c >> index d915123..c71d47a 100644 >> --- a/target-arm/arm-semi.c >> +++ b/target-arm/arm-semi.c >> @@ -191,7 +191,7 @@ static void QEMU_NORETURN unsupported_semihosting(uint32_t nr, CPUState *cs) >> } while (0) >> >> #define SET_ARG(n, val) put_user_ual(val, args + (n) * 4) >> -uint32_t do_arm_semihosting(CPUARMState *env) >> +target_ulong do_arm_semihosting(CPUARMState *env) >> { >> ARMCPU *cpu = arm_env_get_cpu(env); >> CPUState *cs = CPU(cpu); >> @@ -207,8 +207,16 @@ uint32_t do_arm_semihosting(CPUARMState *env) >> CPUARMState *ts = env; >> #endif >> >> - nr = env->regs[0]; >> - args = env->regs[1]; >> + if (is_a64(env)) { >> + nr = env->xregs[0]; > >> + args = env->xregs[1]; >> + if (nr != env->xregs[0] || nr != TARGET_SYS_EXIT) { > > What is the first part of this if condition intended to do? > (Note that the semihosting API number is passed in W0, > not X0...) The intention was to check that none of bits 63 through 32 were set, even if the lower half looked good. Yes, w0 as opposed to x0 makes the most sense for moving the call number into its register, but I'd prefer to double check. Maybe using target_ulong for args would be better, as the default case of the switch statement would handle high bits being set on A64. >> + unsupported_semihosting(nr, cs); >> + } >> + } else { >> + nr = env->regs[0]; >> + args = env->regs[1]; >> + } >> switch (nr) { >> case TARGET_SYS_OPEN: >> GET_ARG(0); > > This doesn't look right -- in the AArch64 version > the SYS_EXIT (aka AngelSWI_Reason_ReportException) > takes a parameter block, so you can't just fall through > to the existing code without modifying it to support this. > > It's also a bit difficult to tell if this code is > right without seeing some actual implementations of > the semihosting calls. Thanks for pointing that out. I'll update my hand-coded exit sequence to better match what Newlib does. My plan for the time being is to code against one of the recent Linaro GCC+Newlib toolchains, but once most of the functionality is in place I may be able to check against other toolchains and simulators/debuggers. >> diff --git a/target-arm/cpu.h b/target-arm/cpu.h >> index 083211c..d5c5cfa 100644 >> --- a/target-arm/cpu.h >> +++ b/target-arm/cpu.h >> @@ -56,6 +56,7 @@ >> #define EXCP_SMC 13 /* Secure Monitor Call */ >> #define EXCP_VIRQ 14 >> #define EXCP_VFIQ 15 >> +#define EXCP_SEMI 16 /* Angel Semihosting Call */ >> >> #define ARMV7M_EXCP_RESET 1 >> #define ARMV7M_EXCP_NMI 2 >> @@ -494,7 +495,7 @@ typedef struct CPUARMState { >> >> ARMCPU *cpu_arm_init(const char *cpu_model); >> int cpu_arm_exec(CPUARMState *s); >> -uint32_t do_arm_semihosting(CPUARMState *env); >> +target_ulong do_arm_semihosting(CPUARMState *env); >> void aarch64_sync_32_to_64(CPUARMState *env); >> void aarch64_sync_64_to_32(CPUARMState *env); >> >> diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c >> index 7e0d038..9ecd488 100644 >> --- a/target-arm/helper-a64.c >> +++ b/target-arm/helper-a64.c >> @@ -514,6 +514,17 @@ void aarch64_cpu_do_interrupt(CPUState *cs) >> case EXCP_VFIQ: >> addr += 0x100; >> break; >> + case EXCP_SEMI: >> + if (semihosting_enabled) { >> + qemu_log_mask(CPU_LOG_INT, >> + "...handling as semihosting call 0x%" PRIx64 "\n", >> + env->xregs[0]); >> + env->xregs[0] = do_arm_semihosting(env); >> + break; >> + } else { >> + qemu_log_mask(CPU_LOG_INT, >> + "...not handled because semihosting is disabled\n"); >> + } >> default: >> cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index); >> } >> diff --git a/target-arm/internals.h b/target-arm/internals.h >> index bb171a7..86b5854 100644 >> --- a/target-arm/internals.h >> +++ b/target-arm/internals.h >> @@ -58,6 +58,7 @@ static const char * const excnames[] = { >> [EXCP_SMC] = "Secure Monitor Call", >> [EXCP_VIRQ] = "Virtual IRQ", >> [EXCP_VFIQ] = "Virtual FIQ", >> + [EXCP_SEMI] = "Angel Semihosting Call", >> }; >> >> static inline void arm_log_exception(int idx) >> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c >> index 0b192a1..3b5b875 100644 >> --- a/target-arm/translate-a64.c >> +++ b/target-arm/translate-a64.c >> @@ -1544,7 +1544,11 @@ static void disas_exc(DisasContext *s, uint32_t insn) >> break; >> } >> /* HLT */ >> - unsupported_encoding(s, insn); >> + if (imm16 == 0xf000) { > > You need to have the semihosting_enabled check here rather > than in the do_interrupt code, because otherwise we won't > behave correctly in the disabled case. I don't think that's what A32 does, but I like it. Thanks, Chrish ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000 2015-03-28 12:27 ` Christopher Covington @ 2015-03-31 11:22 ` Peter Maydell 0 siblings, 0 replies; 10+ messages in thread From: Peter Maydell @ 2015-03-31 11:22 UTC (permalink / raw) To: Christopher Covington; +Cc: Liviu Ionescu, QEMU Developers On 28 March 2015 at 12:27, Christopher Covington <christopher.covington@linaro.org> wrote: > Hi Peter, > > On Fri, Mar 27, 2015 at 12:40 PM, Peter Maydell > <peter.maydell@linaro.org> wrote: >> On 27 March 2015 at 16:22, Christopher Covington >> <christopher.covington@linaro.org> wrote: >>> + args = env->xregs[1]; >>> + if (nr != env->xregs[0] || nr != TARGET_SYS_EXIT) { >> >> What is the first part of this if condition intended to do? >> (Note that the semihosting API number is passed in W0, >> not X0...) > > The intention was to check that none of bits 63 through 32 were set, > even if the lower half looked good. However the spec for this API says w0, so we should ignore the upper bits. > Yes, w0 as opposed to x0 makes the > most sense for moving the call number into its register, but I'd > prefer to double check. Maybe using target_ulong for args would be > better, as the default case of the switch statement would handle high > bits being set on A64. target_ulong is a bit odd here, because for a 32-bit CPU being run from qemu-system-aarch64 it will be a 64 bit type even though the semihosting ABI should be using 32 bit types. I would be wary of using it... >>> @@ -1544,7 +1544,11 @@ static void disas_exc(DisasContext *s, uint32_t insn) >>> break; >>> } >>> /* HLT */ >>> - unsupported_encoding(s, insn); >>> + if (imm16 == 0xf000) { >> >> You need to have the semihosting_enabled check here rather >> than in the do_interrupt code, because otherwise we won't >> behave correctly in the disabled case. > > I don't think that's what A32 does, but I like it. For A32/T32 we always take the exception, because the "not enabled" case can fall through to the standard bkpt/SWI handling code. Because for A64 there is no handling for HLT there's nothing to fall through to. In theory you could make the do_interrupt code handle EXCP_SEMI with semihosting disabled correctly, but it's much easier to just not generate it in the first place. -- PMM ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-04-23 17:39 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-09 17:23 [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000 Christopher Covington 2015-04-09 17:57 ` Peter Maydell 2015-04-10 15:47 ` Christopher Covington 2015-04-10 15:52 ` Peter Maydell 2015-04-23 11:00 ` Leon Alrae 2015-04-23 17:39 ` Christopher Covington -- strict thread matches above, loose matches on Subject: below -- 2015-03-27 16:22 [Qemu-devel] arm: semihosting: Preliminary AArch64 support Christopher Covington 2015-03-27 16:22 ` [Qemu-devel] [PATCH 3/3] arm: semihosting: Wire up A64 HLT 0xf000 Christopher Covington 2015-03-27 16:40 ` Peter Maydell 2015-03-28 12:27 ` Christopher Covington 2015-03-31 11:22 ` Peter Maydell
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.