> -----Original Message-----
> From: Philippe Mathieu-Daudé <phi...@linaro.org>
> Sent: Tuesday, March 4, 2025 6:19 PM
> To: ltaylorsimp...@gmail.com; richard.hender...@linaro.org; 'Brian Cain'
> <brian.c...@oss.qualcomm.com>; qemu-devel@nongnu.org
> Cc: quic_mathb...@quicinc.com; a...@rev.ng; a...@rev.ng;
> quic_mlie...@quicinc.com; alex.ben...@linaro.org;
> quic_mbur...@quicinc.com; sidn...@quicinc.com; 'Brian Cain'
> <bc...@quicinc.com>
> Subject: Re: [PATCH 34/39] target/hexagon: Add TLB, k0 {un,}lock
>
> On 5/3/25 01:05, ltaylorsimp...@gmail.com wrote:
> >
> >
> >> -----Original Message-----
> >> From: Philippe Mathieu-Daudé <phi...@linaro.org>
> >> Sent: Tuesday, March 4, 2025 5:58 PM
> >> To: richard.hender...@linaro.org; ltaylorsimp...@gmail.com; 'Brian Cain'
> >> <brian.c...@oss.qualcomm.com>; qemu-devel@nongnu.org
> >> Cc: quic_mathb...@quicinc.com; a...@rev.ng; a...@rev.ng;
> >> quic_mlie...@quicinc.com; alex.ben...@linaro.org;
> >> quic_mbur...@quicinc.com; sidn...@quicinc.com; 'Brian Cain'
> >> <bc...@quicinc.com>
> >> Subject: Re: [PATCH 34/39] target/hexagon: Add TLB, k0 {un,}lock
> >>
> >> Hi Taylor,
> >>
> >> On 5/3/25 00:09, ltaylorsimp...@gmail.com wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Brian Cain <brian.c...@oss.qualcomm.com>
> >>>> Sent: Monday, March 3, 2025 10:24 AM
> >>>> To: qemu-devel@nongnu.org
> >>>> Cc: richard.hender...@linaro.org; phi...@linaro.org;
> >>>> quic_mathb...@quicinc.com; a...@rev.ng; a...@rev.ng;
> >>>> quic_mlie...@quicinc.com; ltaylorsimp...@gmail.com;
> >>>> alex.ben...@linaro.org; quic_mbur...@quicinc.com;
> >>>> sidn...@quicinc.com; Brian Cain <bc...@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 <bc...@quicinc.com>
> >>>>>
> >>>>> Signed-off-by: Brian Cain <brian.c...@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.
>
> If there are plan for such future, then this is fine.
> We are worried by maintenance burden, see for microblaze:
> https://lore.kernel.org/qemu-devel/ad364fce-f73d-4dde-b890-
> 0ea86d9c4...@linaro.org/
As I said "minor nit". I did a search through the code and the registers are
declared in cpu.h as target_ulong but arch_get_system_reg is defined to return
uint32_t. The users of this function have a mix of uint32_t and target_ulong.
>From a quick read of the thread you pointed out, the problem for microblaze is
>changing the TARGET_LONG_BITS from 64 to 32. Hexagon wouldn't have this
>problem.
I'm not aware of any plans for a 64-bit Hexagon, so I'll leave it to Brian to
decide if he thinks the change is worthwhile.
Taylor
