Re: [PATCH v3 04/13] hexagon: cpu_helper: add reg reading/writing helpers

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

On 7/20/2026 12:41 PM, Matheus Tavares Bernardino wrote:
> And adjust op_helper to use those. They will also be used on upcoming
> semihosting commits.
>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
> Signed-off-by: Matheus Tavares Bernardino <matheus.bernardino@oss.qualcomm.com>
> ---


I got rid of arch_{s,g}et_system_reg() from a previous series under 
advice from Taylor.  I think it's probably for the best. In our 
downstream fork we had a nest of functions and macros for accessing 
system registers and if we can avoid those we'll probably be better off.


https://lore.kernel.org/qemu-devel/017c01db9772$3f31b610$bd952230$@gmail.com/


>   target/hexagon/cpu_helper.h | 18 +++++++++++++++++
>   target/hexagon/cpu_helper.c | 40 +++++++++++++++++++++++++++++++++++++
>   target/hexagon/op_helper.c  | 18 ++---------------
>   3 files changed, 60 insertions(+), 16 deletions(-)
>
> diff --git a/target/hexagon/cpu_helper.h b/target/hexagon/cpu_helper.h
> index ca2e13ab1d1..757a49fc4d4 100644
> --- a/target/hexagon/cpu_helper.h
> +++ b/target/hexagon/cpu_helper.h
> @@ -7,6 +7,24 @@
>   #ifndef HEXAGON_CPU_HELPER_H
>   #define HEXAGON_CPU_HELPER_H
>   
> +static inline void arch_set_thread_reg(CPUHexagonState *env, uint32_t reg,
> +                                       uint32_t val)
> +{
> +    g_assert(reg < TOTAL_PER_THREAD_REGS);
> +    env->gpr[reg] = val;
> +}
> +
> +static inline uint32_t arch_get_thread_reg(CPUHexagonState *env, uint32_t reg)
> +{
> +    g_assert(reg < TOTAL_PER_THREAD_REGS);
> +    return env->gpr[reg];
> +}
> +
> +void arch_set_system_reg(CPUHexagonState *env, uint32_t reg, uint32_t val);
> +void arch_set_system_reg_masked(CPUHexagonState *env, uint32_t reg,
> +                                uint32_t val);
> +uint32_t arch_get_system_reg(CPUHexagonState *env, uint32_t reg);
> +
>   void hexagon_read_memory(CPUHexagonState *env, target_ulong vaddr, int size,
>                            void *retptr, uintptr_t retaddr);
>   void hexagon_write_memory(CPUHexagonState *env, target_ulong vaddr,
> diff --git a/target/hexagon/cpu_helper.c b/target/hexagon/cpu_helper.c
> index e981e11a35d..98ce599571e 100644
> --- a/target/hexagon/cpu_helper.c
> +++ b/target/hexagon/cpu_helper.c
> @@ -27,6 +27,46 @@
>   
>   #ifndef CONFIG_USER_ONLY
>   
> +uint32_t arch_get_system_reg(CPUHexagonState *env, uint32_t reg)
> +{
> +    if (reg == HEX_SREG_PCYCLELO) {
> +        return hexagon_get_sys_pcycle_count_low(env);
> +    } else if (reg == HEX_SREG_PCYCLEHI) {
> +        return hexagon_get_sys_pcycle_count_high(env);
> +    }
> +
> +    g_assert(reg < NUM_SREGS);
> +    if (reg < HEX_SREG_GLB_START) {
> +        return env->t_sreg[reg];
> +    } else {
> +        HexagonCPU *cpu = env_archcpu(env);
> +        return hexagon_globalreg_read(cpu->globalregs, reg, env->threadId);
> +    }
> +}
> +
> +void arch_set_system_reg(CPUHexagonState *env, uint32_t reg, uint32_t val)
> +{
> +    g_assert(reg < NUM_SREGS);
> +    if (reg < HEX_SREG_GLB_START) {
> +        env->t_sreg[reg] = val;
> +    } else {
> +        HexagonCPU *cpu = env_archcpu(env);
> +        hexagon_globalreg_write(cpu->globalregs, reg, val, env->threadId);
> +    }
> +}
> +
> +void arch_set_system_reg_masked(CPUHexagonState *env, uint32_t reg,
> +                                uint32_t val)
> +{
> +    g_assert(reg < NUM_SREGS);
> +    if (reg < HEX_SREG_GLB_START) {
> +        env->t_sreg[reg] = val;
> +    } else {
> +        HexagonCPU *cpu = env_archcpu(env);
> +        hexagon_globalreg_write_masked(cpu->globalregs, reg, val);
> +    }
> +}
> +
>   static bool hexagon_read_memory_small(CPUHexagonState *env, target_ulong addr,
>                                         int byte_count, unsigned char *dstbuf,
>                                         int mmu_idx, uintptr_t retaddr)
> diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
> index 3ce223caba3..324a9632dd4 100644
> --- a/target/hexagon/op_helper.c
> +++ b/target/hexagon/op_helper.c
> @@ -1846,28 +1846,14 @@ void HELPER(setimask)(CPUHexagonState *env, uint32_t tid, uint32_t imask)
>   void HELPER(sreg_write_masked)(CPUHexagonState *env, uint32_t reg, uint32_t val)
>   {
>       BQL_LOCK_GUARD();
> -    if (reg < HEX_SREG_GLB_START) {
> -        env->t_sreg[reg] = val;
> -    } else {
> -        HexagonCPU *cpu = env_archcpu(env);
> -        if (cpu->globalregs) {
> -            hexagon_globalreg_write_masked(cpu->globalregs, reg, val);
> -        }
> -    }
> +    arch_set_system_reg_masked(env, reg, val);
>   }
>   
>   static inline QEMU_ALWAYS_INLINE uint32_t sreg_read(CPUHexagonState *env,
>                                                       uint32_t reg)
>   {
> -    HexagonCPU *cpu;
> -
>       g_assert(bql_locked());
> -    if (reg < HEX_SREG_GLB_START) {
> -        return env->t_sreg[reg];
> -    }
> -    cpu = env_archcpu(env);
> -    return cpu->globalregs ?
> -        hexagon_globalreg_read(cpu->globalregs, reg, env->threadId) : 0;
> +    return arch_get_system_reg(env, reg);
>   }
>   
>   uint32_t HELPER(sreg_read)(CPUHexagonState *env, uint32_t reg)