Re: [PATCH 01/11] target/hexagon: align exceptions for user/sysemu

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

On 8/21/2026 11:37 AM, Pierrick Bouvier wrote:
> On 8/20/2026 12:02 PM, Brian Cain wrote:
>> On 8/20/2026 1:40 PM, Pierrick Bouvier wrote:
>>> On 8/18/2026 6:31 PM, Brian Cain wrote:
>>>> System mode reports an exception as cs->exception_index = HEX_EVENT_*
>>>> plus
>>>> env->cause_code = HEX_CAUSE_*, but translated code in user mode put
>>>> the cause
>>>> code straight into exception_index, so cpu_loop() was decoding both
>>>> forms.
>>>> gen_exception_decode_fail() and the misaligned-PC check used the raw
>>>> form
>>>> unconditionally, so in system mode the cause code was misread as an
>>>> event
>>>> number.
>>>>
>>>> Use the {event, cause} everywhere and drop the duplicated cases
>>>> from cpu_loop(), which fixes HEX_CAUSE_PRIV_USER_NO_SINSN and
>>>> HEX_CAUSE_PRIV_USER_NO_GINSN.  The misaligned PC is no longer zeroed
>>>> on its way out either, so it reaches the signal frame as si_addr instead
>>>> of whatever r31 held.
>>>>
>>>> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
>>>> ---
>>>>    target/hexagon/translate.h    |  2 +-
>>>>    linux-user/hexagon/cpu_loop.c | 30 +++++++++++-------------------
>>>>    target/hexagon/cpu.c          |  3 ++-
>>>>    target/hexagon/translate.c    | 27 +++++++++------------------
>>>>    4 files changed, 23 insertions(+), 39 deletions(-)
>>>>
>>>> diff --git a/target/hexagon/translate.h b/target/hexagon/translate.h
>>>> index 3c5773e2c73..00de2b0d2ec 100644
>>>> --- a/target/hexagon/translate.h
>>>> +++ b/target/hexagon/translate.h
>>>> @@ -330,7 +330,7 @@ extern TCGv_i32 hex_t_sreg[NUM_SREGS];
>>>>    #endif
>>>>      -void hex_gen_exception_end_tb(DisasContext *ctx, int excp);
>>>> +void hex_gen_exception_end_tb(DisasContext *ctx, int cause);
>>>>      void process_store(DisasContext *ctx, int slot_num);
>>>>    diff --git a/linux-user/hexagon/cpu_loop.c b/linux-user/hexagon/
>>>> cpu_loop.c
>>>> index d7f73439dbc..e4ef97a1184 100644
>>>> --- a/linux-user/hexagon/cpu_loop.c
>>>> +++ b/linux-user/hexagon/cpu_loop.c
>>>> @@ -66,21 +66,22 @@ void cpu_loop(CPUHexagonState *env)
>>>>                case HEX_CAUSE_FETCH_NO_UPAGE:
>>>>                case HEX_CAUSE_PRIV_NO_UREAD:
>>>>                case HEX_CAUSE_PRIV_NO_UWRITE:
>>>> -            force_sig_fault(TARGET_SIGSEGV, TARGET_SEGV_MAPERR,
>>>> -                    env->gpr[HEX_REG_PC]);
>>>> -
>>>> -            break;
>>>> +                force_sig_fault(TARGET_SIGSEGV, TARGET_SEGV_MAPERR,
>>>> +                                env->gpr[HEX_REG_PC]);
>>>> +                break;
>>>>                case HEX_CAUSE_PRIV_USER_NO_GINSN:
>>>>                case HEX_CAUSE_PRIV_USER_NO_SINSN:
>>>>                case HEX_CAUSE_INVALID_PACKET:
>>>> -            force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPC,
>>>> -                    env->gpr[HEX_REG_PC]);
>>>> -            break;
>>>> +            case HEX_CAUSE_REG_WRITE_CONFLICT:
>>>> +                force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPC,
>>>> +                                env->gpr[HEX_REG_PC]);
>>>> +                break;
>>>>                case HEX_CAUSE_MISALIGNED_LOAD:
>>>>                case HEX_CAUSE_MISALIGNED_STORE:
>>>> -            force_sig_fault(TARGET_SIGBUS, TARGET_BUS_ADRALN,
>>>> -                    env->gpr[HEX_REG_PC]);
>>>> -            break;
>>>> +            case HEX_CAUSE_PC_NOT_ALIGNED:
>>>> +                force_sig_fault(TARGET_SIGBUS, TARGET_BUS_ADRALN,
>>>> +                                env->gpr[HEX_REG_PC]);
>>>> +                break;
>>>>                default:
>>>>                    EXCP_DUMP(env, "\nqemu: unhandled CPU precise
>>>> exception "
>>>>                        "cause code 0x%x - aborting\n",
>>>> @@ -88,15 +89,6 @@ void cpu_loop(CPUHexagonState *env)
>>>>                    exit(EXIT_FAILURE);
>>>>                }
>>>>                break;
>>>> -        case HEX_CAUSE_PC_NOT_ALIGNED:
>>>> -            force_sig_fault(TARGET_SIGBUS, TARGET_BUS_ADRALN,
>>>> -                            env->gpr[HEX_REG_R31]);
>>>> -            break;
>>>> -        case HEX_CAUSE_INVALID_PACKET:
>>>> -        case HEX_CAUSE_REG_WRITE_CONFLICT:
>>>> -            force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPC,
>>>> -                            env->gpr[HEX_REG_PC]);
>>>> -            break;
>>>>            case EXCP_ATOMIC:
>>>>                cpu_exec_step_atomic(cs);
>>>>                break;
>>>> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
>>>> index 7067e5b70f7..0bbefc2fb87 100644
>>>> --- a/target/hexagon/cpu.c
>>>> +++ b/target/hexagon/cpu.c
>>>> @@ -323,7 +323,8 @@ static TCGTBCPUState
>>>> hexagon_get_tb_cpu_state(CPUState *cs)
>>>>            hex_flags = FIELD_DP32(hex_flags, TB_FLAGS, IS_TIGHT_LOOP, 1);
>>>>        }
>>>>        if (pc & PCALIGN_MASK) {
>>>> -        hexagon_raise_exception_err(env, HEX_CAUSE_PC_NOT_ALIGNED, 0);
>>>> +        env->cause_code = HEX_CAUSE_PC_NOT_ALIGNED;
>>>> +        hexagon_raise_exception_err(env, HEX_EVENT_PRECISE, pc);
>>>>        }
>>>>      #ifndef CONFIG_USER_ONLY
>>>> diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
>>>> index 06a8159d283..5cfa60ca302 100644
>>>> --- a/target/hexagon/translate.c
>>>> +++ b/target/hexagon/translate.c
>>>> @@ -73,8 +73,8 @@ TCGv hex_vstore_pending[VSTORES_MAX];
>>>>    #ifndef CONFIG_USER_ONLY
>>>>    TCGv_i32 hex_greg[NUM_GREGS];
>>>>    TCGv_i32 hex_t_sreg[NUM_SREGS];
>>>> -TCGv_i32 hex_cause_code;
>>>>    #endif
>>>> +static TCGv_i32 hex_cause_code;
>>>>
>>> Shouldn't this be part of CPUState?
>>> What if multiple cpus trigger an exception at the same time?
>> The cause_code is part of CPUState.  This TCGv is a reference to that
>> state member for use with translation. We take advantage of the single-
>> threaded nature of translation with all of these file-global TCGv values.
>>
> Thinking twice about it, please note that this is true only for
> qemu-user (when tb_gen_code runs with mmap_lock), or qemu-system without
> MTTCG, which is what we have at the moment.


I had intended to claim that it was the case that translation was 
single-threaded even in MTTCG.  But I see now that I was mistaken about 
that.

I arrived at this belief somewhat backwards, I guess - the file-scope 
TCGv's used by other architectures who do support MTTCG already, and our 
results using file-scope TCGv's on qemu-system-hexagon with MTTCG in a 
downstream fork.

But after digging a bit I think it's still safe/appropriate to have 
file-scope TCGv's concurrently accessed by multiple translation 
threads.  Because they are merely a reference to the state data and not 
the data itself.  The state data is modified by the TCG instructions 
generated by translation.  The TCGv - the reference itself - is probably 
immutable, meaning it couldn't be made to refer to anything else.  Or 
certainly shouldn't.


> However, using qemu-system with MTTCG, translation can happen
> concurrently, and those variables should be part of DisasContext instead.
>
> Regards,
> Pierrick