On Jul 31 17:06, Alex Bennée wrote:
> Now we do all our checking and use a common EXCP_SEMIHOST for
> semihosting operations we can make helper code a lot simpler.
>
> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
>
> ---
> v2
> - fix re-base conflicts
> - hoist EXCP_SEMIHOST check
> - comment cleanups
> ---
> target/arm/helper.c | 90 +++++++++------------------------------------
> 1 file changed, 18 insertions(+), 72 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index b74c23a9bc0..c5b90a83d36 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -8259,86 +8259,30 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
> new_el, env->pc, pstate_read(env));
> }
>
> -static inline bool check_for_semihosting(CPUState *cs)
> +/*
> + * Do semihosting call and set the appropriate return value. All the
> + * permission and validity checks have been done at translate time.
> + *
> + * We only see semihosting exceptions in TCG only as they are not
> + * trapped to the hypervisor in KVM.
> + */
> +static void handle_semihosting(CPUState *cs)
> {
> #ifdef CONFIG_TCG
> - /* Check whether this exception is a semihosting call; if so
> - * then handle it and return true; otherwise return false.
> - */
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
>
> if (is_a64(env)) {
> - if (cs->exception_index == EXCP_SEMIHOST) {
> - /* This is always the 64-bit semihosting exception.
> - * The "is this usermode" and "is semihosting enabled"
> - * checks have been done at translate time.
> - */
> - qemu_log_mask(CPU_LOG_INT,
> - "...handling as semihosting call 0x%" PRIx64 "\n",
> - env->xregs[0]);
> - env->xregs[0] = do_arm_semihosting(env);
> - return true;
> - }
> - return false;
> + qemu_log_mask(CPU_LOG_INT,
> + "...handling as semihosting call 0x%" PRIx64 "\n",
> + env->xregs[0]);
> + env->xregs[0] = do_arm_semihosting(env);
> } else {
> - uint32_t imm;
> -
> - /* Only intercept calls from privileged modes, to provide some
> - * semblance of security.
> - */
> - if (cs->exception_index != EXCP_SEMIHOST &&
> - (!semihosting_enabled() ||
> - ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_USR))) {
> - return false;
> - }
> -
> - switch (cs->exception_index) {
> - case EXCP_SEMIHOST:
> - /* This is always a semihosting call; the "is this usermode"
> - * and "is semihosting enabled" checks have been done at
> - * translate time.
> - */
> - break;
> - case EXCP_SWI:
> - /* Check for semihosting interrupt. */
> - if (env->thumb) {
> - imm = arm_lduw_code(env, env->regs[15] - 2, arm_sctlr_b(env))
> - & 0xff;
> - if (imm == 0xab) {
> - break;
> - }
> - } else {
> - imm = arm_ldl_code(env, env->regs[15] - 4, arm_sctlr_b(env))
> - & 0xffffff;
> - if (imm == 0x123456) {
> - break;
> - }
> - }
> - return false;
> - case EXCP_BKPT:
> - /* See if this is a semihosting syscall. */
> - if (env->thumb) {
> - imm = arm_lduw_code(env, env->regs[15], arm_sctlr_b(env))
> - & 0xff;
> - if (imm == 0xab) {
> - env->regs[15] += 2;
> - break;
> - }
> - }
> - return false;
> - default:
> - return false;
> - }
> -
> qemu_log_mask(CPU_LOG_INT,
> "...handling as semihosting call 0x%x\n",
> env->regs[0]);
> env->regs[0] = do_arm_semihosting(env);
> - return true;
> }
> -#else
> - return false;
> #endif
> }
>
> @@ -8371,11 +8315,13 @@ void arm_cpu_do_interrupt(CPUState *cs)
> return;
> }
>
> - /* Semihosting semantics depend on the register width of the
> - * code that caused the exception, not the target exception level,
> - * so must be handled here.
> + /*
> + * Semihosting semantics depend on the register width of the code
> + * that caused the exception, not the target exception level, so
> + * must be handled here.
> */
> - if (check_for_semihosting(cs)) {
> + if (cs->exception_index == EXCP_SEMIHOST) {
> + handle_semihosting(cs);
> return;
> }
Previously, this code would never return here if CONFIG_TCG was not
defined because check_for_semihosting() always returned false in that
case. Is it now true that `cs->exception_index` will never hold a value
of EXCP_SEMIHOST if CONFIG_TCG is not defined (or that it is otherwise
correct to return here in that case where it wasn't previously)?
-Aaron
