RE: [PATCH 34/39] target/hexagon: Add TLB, k0 {un,}lock

[<ltaylorsimpson@gmail.com>]

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Sent: Tuesday, March 4, 2025 5:58 PM
> To: richard.henderson@linaro.org; ltaylorsimpson@gmail.com; 'Brian Cain'
> <brian.cain@oss.qualcomm.com>; qemu-devel@nongnu.org
> Cc: quic_mathbern@quicinc.com; ale@rev.ng; anjo@rev.ng;
> quic_mliebel@quicinc.com; alex.bennee@linaro.org;
> quic_mburton@quicinc.com; sidneym@quicinc.com; 'Brian Cain'
> <bcain@quicinc.com>
> Subject: Re: [PATCH 34/39] target/hexagon: Add TLB, k0 {un,}lock
> 
> Hi Taylor,
> 
> On 5/3/25 00:09, ltaylorsimpson@gmail.com wrote:
> >
> >
> >> -----Original Message-----
> >> From: Brian Cain <brian.cain@oss.qualcomm.com>
> >> Sent: Monday, March 3, 2025 10:24 AM
> >> To: qemu-devel@nongnu.org
> >> Cc: 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: Re: [PATCH 34/39] target/hexagon: Add TLB, k0 {un,}lock
> >>
> >>
> >> On 2/28/2025 11:28 PM, Brian Cain wrote:
> >>> From: Brian Cain <bcain@quicinc.com>
> >>>
> >>> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> >>> ---
> >>>    target/hexagon/sys_macros.h |   8 +--
> >>>    target/hexagon/op_helper.c  | 104
> >> ++++++++++++++++++++++++++++++++++++
> >>>    2 files changed, 108 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/target/hexagon/sys_macros.h
> >>> b/target/hexagon/sys_macros.h index 3c4c3c7aa5..e5dc1ce0ab 100644
> >>> --- a/target/hexagon/sys_macros.h
> >>> +++ b/target/hexagon/sys_macros.h
> >>> @@ -143,11 +143,11 @@
> >>>    #define fDCINVIDX(REG)
> >>>    #define fDCINVA(REG) do { REG = REG; } while (0) /* Nothing to do
> >>> in qemu */
> >>>
> >>> -#define fSET_TLB_LOCK()       g_assert_not_reached()
> >>> -#define fCLEAR_TLB_LOCK()     g_assert_not_reached()
> >>> +#define fSET_TLB_LOCK()       hex_tlb_lock(env);
> >>> +#define fCLEAR_TLB_LOCK()     hex_tlb_unlock(env);
> >>>
> >>> -#define fSET_K0_LOCK()        g_assert_not_reached()
> >>> -#define fCLEAR_K0_LOCK()      g_assert_not_reached()
> >>> +#define fSET_K0_LOCK()        hex_k0_lock(env);
> >>> +#define fCLEAR_K0_LOCK()      hex_k0_unlock(env);
> >>>
> >>>    #define fTLB_IDXMASK(INDEX) \
> >>>        ((INDEX) & (fPOW2_ROUNDUP(fCAST4u(env_archcpu(env)-
> >>> num_tlbs)) -
> >>> 1)) diff --git a/target/hexagon/op_helper.c
> >>> b/target/hexagon/op_helper.c index 702c3dd3c6..f3b14fbf58 100644
> >>> --- a/target/hexagon/op_helper.c
> >>> +++ b/target/hexagon/op_helper.c
> >>> @@ -1184,6 +1184,110 @@ void HELPER(modify_ssr)(CPUHexagonState
> >> *env, uint32_t new, uint32_t old)
> >>>        BQL_LOCK_GUARD();
> >>>        hexagon_modify_ssr(env, new, old);
> >>>    }
> >>> +
> >>> +static void hex_k0_lock(CPUHexagonState *env) {
> >>> +    BQL_LOCK_GUARD();
> >>> +    g_assert((env->k0_lock_count == 0) || (env->k0_lock_count ==
> >>> +1));
> >>> +
> >>> +    uint32_t syscfg = arch_get_system_reg(env, HEX_SREG_SYSCFG);
> >
> > Minor nit - registers should be target_ulong type.
> 
> Since Hexagon is only implemented using 32-bit registers, is it worth using
> target_ulong? (I'm trying to foresee heterogeneous emulation).
> 
> Richard, any thought on this (whether a target implementing only 32
> *or* 64 bits should use target_[u]long).

It's just a hedge against the future in case Qualcomm ever builds a 64-bit Hexagon.