Re: [PATCH 17/39] target/hexagon: Implement software interrupt

[Brian Cain <brian.cain@oss.qualcomm.com>]

On 3/19/2025 4:28 PM, ltaylorsimpson@gmail.com wrote:
>
>> -----Original Message-----
>> From: Brian Cain <brian.cain@oss.qualcomm.com>
>> Sent: Friday, February 28, 2025 11:28 PM
>> To: qemu-devel@nongnu.org
>> Cc: brian.cain@oss.qualcomm.com; richard.henderson@linaro.org;
>> philmd@linaro.org; quic_mathbern@quicinc.com; ale@rev.ng; anjo@rev.ng;
>> quic_mliebel@quicinc.com; ltaylorsimpson@gmail.com;
>> alex.bennee@linaro.org; quic_mburton@quicinc.com;
>> sidneym@quicinc.com; Brian Cain <bcain@quicinc.com>; Mike Lambert
>> <mlambert@quicinc.com>
>> Subject: [PATCH 17/39] target/hexagon: Implement software interrupt
>>
>> From: Brian Cain <bcain@quicinc.com>
>>
>> Co-authored-by: Mike Lambert <mlambert@quicinc.com>
>> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
>> ---
>>   target/hexagon/cpu.h       |   1 -
>>   target/hexagon/hexswi.h    |  17 +++
>>   target/hexagon/cpu.c       |   2 +
>>   target/hexagon/hexswi.c    | 258
>> +++++++++++++++++++++++++++++++++++++
>>   target/hexagon/op_helper.c |   1 +
>>   5 files changed, 278 insertions(+), 1 deletion(-)  create mode 100644
>> target/hexagon/hexswi.h  create mode 100644 target/hexagon/hexswi.c
>>
>> diff --git a/target/hexagon/cpu.h b/target/hexagon/cpu.h index
>> dabee310c5..045581d7be 100644
>> --- a/target/hexagon/cpu.h
>> +++ b/target/hexagon/cpu.h
>> @@ -256,5 +256,4 @@ typedef HexagonCPU ArchCPU;  void
>> hexagon_translate_init(void);  void hexagon_translate_code(CPUState *cs,
>> TranslationBlock *tb,
>>                               int *max_insns, vaddr pc, void *host_pc);
>> -
> Gratuitous change
Fixed in v2.
>>   #endif /* HEXAGON_CPU_H */
>> diff --git a/target/hexagon/hexswi.h b/target/hexagon/hexswi.h new file
>> mode 100644 index 0000000000..5d232cb06c
>> --- /dev/null
>> +++ b/target/hexagon/hexswi.h
>> @@ -0,0 +1,17 @@
>> +/*
>> + * Copyright(c) 2025 Qualcomm Innovation Center, Inc. All Rights Reserved.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later  */
>> +
>> +#ifndef HEXSWI_H
>> +#define HEXSWI_H
>> +
>> +
>> +#include "cpu.h"
>> +
>> +void hexagon_cpu_do_interrupt(CPUState *cpu); void
>> +register_trap_exception(CPUHexagonState *env, int type, int imm,
>> +                             target_ulong PC);
>> +
>> +#endif /* HEXSWI_H */
>> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c index
>> 89a051b41d..843be8221f 100644
>> --- a/target/hexagon/cpu.c
>> +++ b/target/hexagon/cpu.c
>> @@ -33,6 +33,8 @@
>>   #ifndef CONFIG_USER_ONLY
>>   #include "sys_macros.h"
>>   #include "qemu/main-loop.h"
>> +#include "hex_interrupts.h"
>> +#include "hexswi.h"
> Move these added include to a different patch where the contents are needed.

I didn't fix this in v2 because I convinced myself that these were 
appropriate as-is.  But on reconsideration I think I'm mistaken and will 
try to fix this for v3.


>>   #endif
>>
>>   static void hexagon_v66_cpu_init(Object *obj) { } diff --git
>> a/target/hexagon/hexswi.c b/target/hexagon/hexswi.c new file mode
>> 100644 index 0000000000..5fcf9b2be9
>> --- /dev/null
>> +++ b/target/hexagon/hexswi.c
>> @@ -0,0 +1,258 @@
>> +/*
>> + * Copyright(c) 2019-2025 Qualcomm Innovation Center, Inc. All Rights
>> Reserved.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later  */
>> +
>> +#include "qemu/osdep.h"
>> +#include "cpu.h"
>> +#ifdef CONFIG_USER_ONLY
> This file is only included in the system-mode build, so we don't need these guards.  Several in this file.

Fixed in v2.


>> +#include "exec/helper-proto.h"
>> +#include "qemu.h"
>> +#endif
>> +#include "exec/cpu_ldst.h"
>> +#include "exec/exec-all.h"
>> +#include "qemu/log.h"
>> +#include "qemu/main-loop.h"
>> +#include "arch.h"
>> +#include "internal.h"
>> +#include "macros.h"
>> +#include "sys_macros.h"
>> +#include "tcg/tcg-op.h"
>> +#ifndef CONFIG_USER_ONLY
>> +#include "hex_mmu.h"
>> +#include "hexswi.h"
>> +#endif
>> +
>> +#ifndef CONFIG_USER_ONLY
>> +
>> +
>> +static void set_addresses(CPUHexagonState *env, target_ulong pc_offset,
>> +                          target_ulong exception_index)
>> +
>> +{
>> +    arch_set_system_reg(env, HEX_SREG_ELR,
>> +                        arch_get_thread_reg(env, HEX_REG_PC) + pc_offset);
>> +    arch_set_thread_reg(env, HEX_REG_PC,
>> +                        arch_get_system_reg(env, HEX_SREG_EVB) |
>> +                            (exception_index << 2)); }
>> +
>> +static const char *event_name[] = {
>> +    [HEX_EVENT_RESET] = "HEX_EVENT_RESET",
>> +    [HEX_EVENT_IMPRECISE] = "HEX_EVENT_IMPRECISE",
>> +    [HEX_EVENT_TLB_MISS_X] = "HEX_EVENT_TLB_MISS_X",
>> +    [HEX_EVENT_TLB_MISS_RW] = "HEX_EVENT_TLB_MISS_RW",
>> +    [HEX_EVENT_TRAP0] = "HEX_EVENT_TRAP0",
>> +    [HEX_EVENT_TRAP1] = "HEX_EVENT_TRAP1",
>> +    [HEX_EVENT_FPTRAP] = "HEX_EVENT_FPTRAP",
>> +    [HEX_EVENT_DEBUG] = "HEX_EVENT_DEBUG",
>> +    [HEX_EVENT_INT0] = "HEX_EVENT_INT0",
>> +    [HEX_EVENT_INT1] = "HEX_EVENT_INT1",
>> +    [HEX_EVENT_INT2] = "HEX_EVENT_INT2",
>> +    [HEX_EVENT_INT3] = "HEX_EVENT_INT3",
>> +    [HEX_EVENT_INT4] = "HEX_EVENT_INT4",
>> +    [HEX_EVENT_INT5] = "HEX_EVENT_INT5",
>> +    [HEX_EVENT_INT6] = "HEX_EVENT_INT6",
>> +    [HEX_EVENT_INT7] = "HEX_EVENT_INT7",
>> +    [HEX_EVENT_INT8] = "HEX_EVENT_INT8",
>> +    [HEX_EVENT_INT9] = "HEX_EVENT_INT9",
>> +    [HEX_EVENT_INTA] = "HEX_EVENT_INTA",
>> +    [HEX_EVENT_INTB] = "HEX_EVENT_INTB",
>> +    [HEX_EVENT_INTC] = "HEX_EVENT_INTC",
>> +    [HEX_EVENT_INTD] = "HEX_EVENT_INTD",
>> +    [HEX_EVENT_INTE] = "HEX_EVENT_INTE",
>> +    [HEX_EVENT_INTF] = "HEX_EVENT_INTF"
>> +};
>> +
>> +void hexagon_cpu_do_interrupt(CPUState *cs)
>> +
>> +{
>> +    CPUHexagonState *env = cpu_env(cs);
>> +    BQL_LOCK_GUARD();
>> +
>> +    qemu_log_mask(CPU_LOG_INT, "\t%s: event 0x%x:%s, cause
>> 0x%x(%d)\n",
>> +                  __func__, cs->exception_index,
>> +                  event_name[cs->exception_index], env->cause_code,
>> +                  env->cause_code);
>> +
>> +    env->llsc_addr = ~0;
>> +
>> +    uint32_t ssr = arch_get_system_reg(env, HEX_SREG_SSR);
> Declarations at the beginning of the function.

Fixed in v2.


>> +    if (GET_SSR_FIELD(SSR_EX, ssr) == 1) {
>> +        arch_set_system_reg(env, HEX_SREG_DIAG, env->cause_code);
>> +        env->cause_code = HEX_CAUSE_DOUBLE_EXCEPT;
>> +        cs->exception_index = HEX_EVENT_PRECISE;
>> +    }
>> +
>> +    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");
>> +        }
>> +
>> +        hexagon_ssr_set_cause(env, env->cause_code);
>> +        set_addresses(env, 4, cs->exception_index);
>> +        break;
>> +
>> +    case HEX_EVENT_TRAP1:
>> +        hexagon_ssr_set_cause(env, env->cause_code);
>> +        set_addresses(env, 4, cs->exception_index);
>> +        break;
>> +
>> +    case HEX_EVENT_TLB_MISS_X:
>> +        switch (env->cause_code) {
>> +        case HEX_CAUSE_TLBMISSX_CAUSE_NORMAL:
>> +        case HEX_CAUSE_TLBMISSX_CAUSE_NEXTPAGE:
>> +            qemu_log_mask(CPU_LOG_MMU,
>> +                          "TLB miss EX exception (0x%x) caught: "
>> +                          "Cause code (0x%x) "
>> +                          "TID = 0x%" PRIx32 ", PC = 0x%" PRIx32
>> +                          ", BADVA = 0x%" PRIx32 "\n",
>> +                          cs->exception_index, env->cause_code, env->threadId,
>> +                          arch_get_thread_reg(env, HEX_REG_PC),
>> +                          arch_get_system_reg(env, HEX_SREG_BADVA));
>> +
>> +            hexagon_ssr_set_cause(env, env->cause_code);
>> +            set_addresses(env, 0, cs->exception_index);
>> +            break;
>> +
>> +        default:
>> +            cpu_abort(cs,
>> +                      "1:Hexagon exception %d/0x%x: "
>> +                      "Unknown cause code %d/0x%x\n",
>> +                      cs->exception_index, cs->exception_index, env->cause_code,
>> +                      env->cause_code);
>> +            break;
>> +        }
>> +        break;
>> +
>> +    case HEX_EVENT_TLB_MISS_RW:
>> +        switch (env->cause_code) {
>> +        case HEX_CAUSE_TLBMISSRW_CAUSE_READ:
>> +        case HEX_CAUSE_TLBMISSRW_CAUSE_WRITE:
>> +            qemu_log_mask(CPU_LOG_MMU,
>> +                          "TLB miss RW exception (0x%x) caught: "
>> +                          "Cause code (0x%x) "
>> +                          "TID = 0x%" PRIx32 ", PC = 0x%" PRIx32
>> +                          ", BADVA = 0x%" PRIx32 "\n",
>> +                          cs->exception_index, env->cause_code, env->threadId,
>> +                          env->gpr[HEX_REG_PC],
>> +                          arch_get_system_reg(env, HEX_SREG_BADVA));
>> +
>> +            hexagon_ssr_set_cause(env, env->cause_code);
>> +            set_addresses(env, 0, cs->exception_index);
>> +            /* env->sreg[HEX_SREG_BADVA] is set when the exception is raised
>> */
>> +            break;
>> +
>> +        default:
>> +            cpu_abort(cs,
>> +                      "2:Hexagon exception %d/0x%x: "
>> +                      "Unknown cause code %d/0x%x\n",
>> +                      cs->exception_index, cs->exception_index, env->cause_code,
>> +                      env->cause_code);
>> +            break;
>> +        }
>> +        break;
>> +
>> +    case HEX_EVENT_FPTRAP:
>> +        hexagon_ssr_set_cause(env, env->cause_code);
>> +        arch_set_thread_reg(env, HEX_REG_PC,
>> +                            arch_get_system_reg(env, HEX_SREG_EVB) |
>> +                                (cs->exception_index << 2));
> Why not use set_addresses here?  How is ELR set?
>
>> +        break;
>> +
>> +    case HEX_EVENT_DEBUG:
>> +        hexagon_ssr_set_cause(env, env->cause_code);
>> +        set_addresses(env, 0, cs->exception_index);
>> +        qemu_log_mask(LOG_UNIMP, "single-step exception is not
>> handled\n");
>> +        break;
>> +
>> +    case HEX_EVENT_PRECISE:
>> +        switch (env->cause_code) {
>> +        case HEX_CAUSE_FETCH_NO_XPAGE:
>> +        case HEX_CAUSE_FETCH_NO_UPAGE:
>> +        case HEX_CAUSE_PRIV_NO_READ:
>> +        case HEX_CAUSE_PRIV_NO_UREAD:
>> +        case HEX_CAUSE_PRIV_NO_WRITE:
>> +        case HEX_CAUSE_PRIV_NO_UWRITE:
>> +        case HEX_CAUSE_MISALIGNED_LOAD:
>> +        case HEX_CAUSE_MISALIGNED_STORE:
>> +        case HEX_CAUSE_PC_NOT_ALIGNED:
>> +            qemu_log_mask(CPU_LOG_MMU,
>> +                          "MMU permission exception (0x%x) caught: "
>> +                          "Cause code (0x%x) "
>> +                          "TID = 0x%" PRIx32 ", PC = 0x%" PRIx32
>> +                          ", BADVA = 0x%" PRIx32 "\n",
>> +                          cs->exception_index, env->cause_code, env->threadId,
>> +                          env->gpr[HEX_REG_PC],
>> +                          arch_get_system_reg(env, HEX_SREG_BADVA));
>> +
>> +
>> +            hexagon_ssr_set_cause(env, env->cause_code);
>> +            set_addresses(env, 0, cs->exception_index);
>> +            /* env->sreg[HEX_SREG_BADVA] is set when the exception is raised
>> */
>> +            break;
>> +
>> +        case HEX_CAUSE_DOUBLE_EXCEPT:
>> +        case HEX_CAUSE_PRIV_USER_NO_SINSN:
>> +        case HEX_CAUSE_PRIV_USER_NO_GINSN:
>> +        case HEX_CAUSE_INVALID_OPCODE:
>> +        case HEX_CAUSE_NO_COPROC_ENABLE:
>> +        case HEX_CAUSE_NO_COPROC2_ENABLE:
>> +        case HEX_CAUSE_UNSUPORTED_HVX_64B:
>> +        case HEX_CAUSE_REG_WRITE_CONFLICT:
>> +        case HEX_CAUSE_VWCTRL_WINDOW_MISS:
>> +            hexagon_ssr_set_cause(env, env->cause_code);
>> +            set_addresses(env, 0, cs->exception_index);
>> +            break;
>> +
>> +        case HEX_CAUSE_COPROC_LDST:
>> +            hexagon_ssr_set_cause(env, env->cause_code);
>> +            set_addresses(env, 0, cs->exception_index);
>> +            break;
>> +
>> +        case HEX_CAUSE_STACK_LIMIT:
>> +            hexagon_ssr_set_cause(env, env->cause_code);
>> +            set_addresses(env, 0, cs->exception_index);
>> +            break;
>> +
>> +        default:
>> +            cpu_abort(cs,
>> +                      "3:Hexagon exception %d/0x%x: "
>> +                      "Unknown cause code %d/0x%x\n",
>> +                      cs->exception_index, cs->exception_index, env->cause_code,
>> +                      env->cause_code);
>> +            break;
>> +        }
>> +        break;
>> +
>> +    case HEX_EVENT_IMPRECISE:
>> +        qemu_log_mask(LOG_UNIMP,
>> +                "Imprecise exception: this case is not yet handled");
>> +        break;
>> +
>> +    default:
>> +        qemu_log_mask(LOG_UNIMP,
>> +                "Hexagon Unsupported exception 0x%x/0x%x\n",
>> +                  cs->exception_index, env->cause_code);
>> +        break;
>> +    }
>> +
>> +    cs->exception_index = HEX_EVENT_NONE; }
>> +
>> +void register_trap_exception(CPUHexagonState *env, int traptype, int
>> imm,
>> +                             target_ulong PC) {
>> +    CPUState *cs = env_cpu(env);
>> +
>> +    cs->exception_index = (traptype == 0) ? HEX_EVENT_TRAP0 :
>> HEX_EVENT_TRAP1;
>> +    ASSERT_DIRECT_TO_GUEST_UNSET(env, cs->exception_index);
>> +
>> +    env->cause_code = imm;
>> +    env->gpr[HEX_REG_PC] = PC;
>> +    cpu_loop_exit(cs);
>> +}
>> +#endif
>> diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
>> index 42805d0f1d..687e7f45c2 100644
>> --- a/target/hexagon/op_helper.c
>> +++ b/target/hexagon/op_helper.c
>> @@ -38,6 +38,7 @@
>>   #include "hex_mmu.h"
>>   #include "hw/intc/l2vic.h"
>>   #include "hex_interrupts.h"
>> +#include "hexswi.h"
> Move this do a different patch where the contents are needed

I didn't fix this in v2 because I convinced myself that these were 
appropriate as-is.  But on reconsideration I think I'm mistaken and will 
try to fix this for v3.


>>   #endif
>>
>>   #define SF_BIAS        127
>> --
>> 2.34.1
>