| Message ID | 20250301052845.1012069-11-brian.cain@oss.qualcomm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hexagon system emu, part 2/3 | expand |
> -----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> > Subject: [PATCH 10/39] target/hexagon: Implement > arch_{s,g}et_{thread,system}_reg() > > From: Brian Cain <bcain@quicinc.com> > > Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com> > --- > target/hexagon/cpu_helper.h | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/target/hexagon/cpu_helper.h b/target/hexagon/cpu_helper.h > index e8d89d8526..1cdf4f8dd0 100644 > --- a/target/hexagon/cpu_helper.h > +++ b/target/hexagon/cpu_helper.h > @@ -26,20 +26,27 @@ static inline void > arch_set_thread_reg(CPUHexagonState *env, uint32_t reg, > uint32_t val) { > g_assert(reg < TOTAL_PER_THREAD_REGS); > - g_assert_not_reached(); > + env->gpr[reg] = val; Gotta be careful here. Not all registers can be assigned directly. Look at gen_write_ctrl_reg in genptr.c. > } > > static inline uint32_t arch_get_thread_reg(CPUHexagonState *env, uint32_t > reg) { > g_assert(reg < TOTAL_PER_THREAD_REGS); > - g_assert_not_reached(); > + return env->gpr[reg]; Ditto - look at gen_read_ctrl_reg in genptr.c > } > > +#ifndef CONFIG_USER_ONLY > static inline void arch_set_system_reg(CPUHexagonState *env, uint32_t > reg, > uint32_t val) { > - g_assert_not_reached(); > + g_assert(reg < NUM_SREGS); > + if (reg < HEX_SREG_GLB_START) { > + env->t_sreg[reg] = val; > + } else { > + env->g_sreg[reg] = val; > + } Be careful here too. Look at gen_log_sreg_write. > } > +#endif > > uint32_t arch_get_system_reg(CPUHexagonState *env, uint32_t reg); Honestly, consider getting rid of these. All the uses are a specific register number, so there's no benefit of things like g_assert(reg < TOTAL_PER_THREAD_REGS) or reg < HEX_SREG_GLB_START. Also, keeping them runs the risk of not having all the proper checks inside.
diff --git a/target/hexagon/cpu_helper.h b/target/hexagon/cpu_helper.h index e8d89d8526..1cdf4f8dd0 100644 --- a/target/hexagon/cpu_helper.h +++ b/target/hexagon/cpu_helper.h @@ -26,20 +26,27 @@ static inline void arch_set_thread_reg(CPUHexagonState *env, uint32_t reg, uint32_t val) { g_assert(reg < TOTAL_PER_THREAD_REGS); - g_assert_not_reached(); + env->gpr[reg] = val; } static inline uint32_t arch_get_thread_reg(CPUHexagonState *env, uint32_t reg) { g_assert(reg < TOTAL_PER_THREAD_REGS); - g_assert_not_reached(); + return env->gpr[reg]; } +#ifndef CONFIG_USER_ONLY static inline void arch_set_system_reg(CPUHexagonState *env, uint32_t reg, uint32_t val) { - g_assert_not_reached(); + g_assert(reg < NUM_SREGS); + if (reg < HEX_SREG_GLB_START) { + env->t_sreg[reg] = val; + } else { + env->g_sreg[reg] = val; + } } +#endif uint32_t arch_get_system_reg(CPUHexagonState *env, uint32_t reg);