Re: [PATCH v3 07/13] target/hexagon: add semihosting support

Hi Matheus,

On 20/7/26 19:41, Matheus Tavares Bernardino wrote:
> Baremetal Hexagon programs use trap0 #0 to invoke
> semihosting calls for I/O and process control.  Wire up the
> arm-compatible semihosting framework for softmmu by enabling
> CONFIG_ARM_COMPATIBLE_SEMIHOSTING and routing trap0 to the
> semihosting handler.
> 
> Signed-off-by: Brian Cain <[email protected]>
> Reviewed-by: Pierrick Bouvier <[email protected]>
> Signed-off-by: Matheus Tavares Bernardino <[email protected]>
> ---
>   docs/system/target-hexagon.rst      |   8 +-
>   configs/targets/hexagon-softmmu.mak |   2 +
>   hw/hexagon/hexagon_dsp.c            |   2 +
>   target/hexagon/common-semi-target.c |  51 +++++++++
>   target/hexagon/hexswi.c             | 167 +++++++++++++++++++++++++++-
>   hw/hexagon/Kconfig                  |   1 +
>   qemu-options.hx                     |   8 +-
>   target/hexagon/meson.build          |   3 +
>   8 files changed, 232 insertions(+), 10 deletions(-)
>   create mode 100644 target/hexagon/common-semi-target.c


> +static void sim_handle_trap0(CPUHexagonState *env)
> +{
> +    target_ulong what_swi, swi_info;
> +    CPUState *cs = env_cpu(env);
> +
> +    g_assert(bql_locked());
> +    init_semihosting_guestfds();
> +
> +    what_swi = arch_get_thread_reg(env, HEX_REG_R00);
> +    swi_info = arch_get_thread_reg(env, HEX_REG_R01);
> +
> +    qemu_log_mask(CPU_LOG_INT,
> +                  "sim_handle_trap0: swi=0x%" PRIx32
> +                  " info=0x%" PRIx32 " PC=0x%" PRIx32
> +                  " thread=%" PRId32 "\n",
> +                  (uint32_t)what_swi, (uint32_t)swi_info,
> +                  (uint32_t)arch_get_thread_reg(env, HEX_REG_PC),
> +                  (uint32_t)env->threadId);
> +
> +    if (!is_hexagon_specific_swi_flag(what_swi)) {
> +        if (what_swi == HEX_SYS_READ || what_swi == HEX_SYS_READC ||
> +            what_swi == HEX_SYS_WRITE) {
> +            /*
> +             * Avoid page faults if the buffer is not in memory yet.
> +             * NOTE: Counterintuitive, but a WRITE must be able to LOAD from
> +             * the input address. The contents of that buffer will be
> +             * directed to the SWI interface.
> +             */
> +            do_preload(env, swi_info, (what_swi == HEX_SYS_WRITE));
> +        }
> +        /*
> +         * ARM-compat semihosting SWI numbers are all <= 0x31.
> +         * If R0 holds a value outside that range (e.g. guest code
> +         * executing trap0(#0) with an arbitrary R0), treat it as an
> +         * unrecognized request rather than forwarding to
> +         * do_common_semihosting() which would abort.
> +         */
> +        if (what_swi > 0x31) {
> +            qemu_log_mask(LOG_UNIMP,
> +                          "trap0(#0): unrecognized request in r0: "
> +                          "0x" TARGET_FMT_lx "\n", what_swi);
> +            return;
> +        }
> +        do_common_semihosting(cs);
> +        return;
> +    }
> +
> +    switch (what_swi) {
> +
> +    case HEX_SYS_EXCEPTION:
> +    {
> +        uint32_t ret = arch_get_thread_reg(env, HEX_REG_R02);
> +        arch_set_system_reg(env, HEX_SREG_MODECTL, 0);
> +        gdb_exit(ret);
> +        exit(ret);
> +    }
> +    break;
> +
> +    /* TODO: implement other hexagon-specific semihosting calls */
> +
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,

LOG_UNIMP

> +                      "unknown swi request: 0x%" PRIx32 "\n",
> +                      (uint32_t)what_swi);
> +        common_semi_cb(cs, -1, ENOSYS);
> +    }
> +}
> +
>   static void set_addresses(CPUHexagonState *env, uint32_t pc_offset,
>                             uint32_t exception_index)
> -
>   {
>       HexagonCPU *cpu = env_archcpu(env);
>       uint32_t evb = cpu->globalregs ?
> @@ -95,8 +257,7 @@ void hexagon_cpu_do_interrupt(CPUState *cs)
>       switch (cs->exception_index) {
>       case HEX_EVENT_TRAP0:
>           if (env->cause_code == 0) {
> -            qemu_log_mask(LOG_UNIMP,
> -                          "trap0 is unhandled, no semihosting available\n");
> +            sim_handle_trap0(env);
>           }
>   
>           hexagon_ssr_set_cause(env, env->cause_code);
> diff --git a/hw/hexagon/Kconfig b/hw/hexagon/Kconfig
> index 52065ab3b22..3a8ff17812b 100644
> --- a/hw/hexagon/Kconfig
> +++ b/hw/hexagon/Kconfig
> @@ -2,6 +2,7 @@ config HEX_DSP
>       bool
>       default y
>       depends on HEXAGON
> +    select ARM_COMPATIBLE_SEMIHOSTING

Here we select explicitly ...

>   
>   config HEX_VIRT
>       bool


> diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
> index 59cb09c1070..69f01bd2f70 100644
> --- a/target/hexagon/meson.build
> +++ b/target/hexagon/meson.build
> @@ -262,6 +262,9 @@ hexagon_softmmu_ss.add(files(
>       'machine.c',
>   ))
>   
> +hexagon_softmmu_ss.add(when: 'CONFIG_ARM_COMPATIBLE_SEMIHOSTING',
> +                       if_true: files('common-semi-target.c'))

... so optionally including this file is odd. If we still want it
optional, then we need stubs for symbols with external linkage.