From: Brian Cain <bcain@quicinc.com>

Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
---
 target/hexagon/cpu.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 36a93cc22f..2b6a707fca 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -26,6 +26,7 @@
 #include "fpu/softfloat-helpers.h"
 #include "tcg/tcg.h"
 #include "exec/gdbstub.h"
+#include "cpu_helper.h"
 
 static void hexagon_v66_cpu_init(Object *obj) { }
 static void hexagon_v67_cpu_init(Object *obj) { }
@@ -290,11 +291,18 @@ static void hexagon_cpu_reset_hold(Object *obj, ResetType type)
     set_float_default_nan_pattern(0b11111111, &env->fp_status);
 
 #ifndef CONFIG_USER_ONLY
+    HexagonCPU *cpu = HEXAGON_CPU(cs);
+
     if (cs->cpu_index == 0) {
         memset(env->g_sreg, 0, sizeof(target_ulong) * NUM_SREGS);
     }
     memset(env->t_sreg, 0, sizeof(target_ulong) * NUM_SREGS);
     memset(env->greg, 0, sizeof(target_ulong) * NUM_GREGS);
+
+    if (cs->cpu_index == 0) {
+        arch_set_system_reg(env, HEX_SREG_MODECTL, 0x1);
+    }
+    arch_set_system_reg(env, HEX_SREG_HTID, cs->cpu_index);
 #endif
 }
 
-- 
2.34.1

On 1/3/25 06:26, Brian Cain wrote:
> From: Brian Cain <bcain@quicinc.com>
> 
> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> ---
>   target/hexagon/cpu.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
> index 36a93cc22f..2b6a707fca 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -26,6 +26,7 @@
>   #include "fpu/softfloat-helpers.h"
>   #include "tcg/tcg.h"
>   #include "exec/gdbstub.h"
> +#include "cpu_helper.h"
>   
>   static void hexagon_v66_cpu_init(Object *obj) { }
>   static void hexagon_v67_cpu_init(Object *obj) { }
> @@ -290,11 +291,18 @@ static void hexagon_cpu_reset_hold(Object *obj, ResetType type)
>       set_float_default_nan_pattern(0b11111111, &env->fp_status);
>   
>   #ifndef CONFIG_USER_ONLY
> +    HexagonCPU *cpu = HEXAGON_CPU(cs);
> +
>       if (cs->cpu_index == 0) {

This doesn't scale to heterogeneous emulation.

>           memset(env->g_sreg, 0, sizeof(target_ulong) * NUM_SREGS);
>       }
>       memset(env->t_sreg, 0, sizeof(target_ulong) * NUM_SREGS);
>       memset(env->greg, 0, sizeof(target_ulong) * NUM_GREGS);
> +
> +    if (cs->cpu_index == 0) {

Ditto.

> +        arch_set_system_reg(env, HEX_SREG_MODECTL, 0x1);
> +    }
> +    arch_set_system_reg(env, HEX_SREG_HTID, cs->cpu_index);
>   #endif
>   }
>

On 3/12/2025 2:19 PM, Philippe Mathieu-Daudé wrote:
> On 1/3/25 06:26, Brian Cain wrote:
>> From: Brian Cain <bcain@quicinc.com>
>>
>> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
>> ---
>>   target/hexagon/cpu.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
>> index 36a93cc22f..2b6a707fca 100644
>> --- a/target/hexagon/cpu.c
>> +++ b/target/hexagon/cpu.c
>> @@ -26,6 +26,7 @@
>>   #include "fpu/softfloat-helpers.h"
>>   #include "tcg/tcg.h"
>>   #include "exec/gdbstub.h"
>> +#include "cpu_helper.h"
>>     static void hexagon_v66_cpu_init(Object *obj) { }
>>   static void hexagon_v67_cpu_init(Object *obj) { }
>> @@ -290,11 +291,18 @@ static void hexagon_cpu_reset_hold(Object *obj, 
>> ResetType type)
>>       set_float_default_nan_pattern(0b11111111, &env->fp_status);
>>     #ifndef CONFIG_USER_ONLY
>> +    HexagonCPU *cpu = HEXAGON_CPU(cs);
>> +
>>       if (cs->cpu_index == 0) {
>
> This doesn't scale to heterogeneous emulation.
>

If we have a target-specific index here (instead of cpu_index) guarding 
the "g_sreg" allocation shared by these Hexagon vCPUs, does that 
suffice?  Or is the problem the shared allocation itself?


Could a heterogeneous emulation configuration consist of multiple 
instances of (same-architecture) vCPU-groups?


>>           memset(env->g_sreg, 0, sizeof(target_ulong) * NUM_SREGS);
>>       }
>>       memset(env->t_sreg, 0, sizeof(target_ulong) * NUM_SREGS);
>>       memset(env->greg, 0, sizeof(target_ulong) * NUM_GREGS);
>> +
>> +    if (cs->cpu_index == 0) {
>
> Ditto.
>
>> +        arch_set_system_reg(env, HEX_SREG_MODECTL, 0x1);
>> +    }
>> +    arch_set_system_reg(env, HEX_SREG_HTID, cs->cpu_index);
>>   #endif
>>   }
>

On 13/3/25 00:10, Brian Cain wrote:
> 
> On 3/12/2025 2:19 PM, Philippe Mathieu-Daudé wrote:
>> On 1/3/25 06:26, Brian Cain wrote:
>>> From: Brian Cain <bcain@quicinc.com>
>>>
>>> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
>>> ---
>>>   target/hexagon/cpu.c | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
>>> index 36a93cc22f..2b6a707fca 100644
>>> --- a/target/hexagon/cpu.c
>>> +++ b/target/hexagon/cpu.c
>>> @@ -26,6 +26,7 @@
>>>   #include "fpu/softfloat-helpers.h"
>>>   #include "tcg/tcg.h"
>>>   #include "exec/gdbstub.h"
>>> +#include "cpu_helper.h"
>>>     static void hexagon_v66_cpu_init(Object *obj) { }
>>>   static void hexagon_v67_cpu_init(Object *obj) { }
>>> @@ -290,11 +291,18 @@ static void hexagon_cpu_reset_hold(Object *obj, 
>>> ResetType type)
>>>       set_float_default_nan_pattern(0b11111111, &env->fp_status);
>>>     #ifndef CONFIG_USER_ONLY
>>> +    HexagonCPU *cpu = HEXAGON_CPU(cs);
>>> +
>>>       if (cs->cpu_index == 0) {
>>
>> This doesn't scale to heterogeneous emulation.
>>
> 
> If we have a target-specific index here (instead of cpu_index) guarding 
> the "g_sreg" allocation shared by these Hexagon vCPUs, does that 
> suffice?  Or is the problem the shared allocation itself?

I'm not sure that suffices, but it is still better from my PoV.

Let's assume we instantiate 4 ARM cores, then 2 HEX ones. The first
Hexagon core has cpu_index=5. Now for the same example machine we
instantiate first the Hexagon cores, then the ARM ones. The first
Hexagon core has cpu_index=0.

> Could a heterogeneous emulation configuration consist of multiple 
> instances of (same-architecture) vCPU-groups?

Up to you if you want to model multiple hexagon SoCs in the same
machine ;) Note in that case you could use a CPUClusterState to
group.

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Sent: Wednesday, March 12, 2025 6:40 PM
> To: Brian Cain <brian.cain@oss.qualcomm.com>; qemu-devel@nongnu.org
> Cc: richard.henderson@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 28/38] target/hexagon: Initialize htid, modectl regs
> 
> On 13/3/25 00:10, Brian Cain wrote:
> >
> > On 3/12/2025 2:19 PM, Philippe Mathieu-Daudé wrote:
> >> On 1/3/25 06:26, Brian Cain wrote:
> >>> From: Brian Cain <bcain@quicinc.com>
> >>>
> >>> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> >>> ---
> >>>   target/hexagon/cpu.c | 8 ++++++++
> >>>   1 file changed, 8 insertions(+)
> >>>
> >>> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c index
> >>> 36a93cc22f..2b6a707fca 100644
> >>> --- a/target/hexagon/cpu.c
> >>> +++ b/target/hexagon/cpu.c
> >>> @@ -26,6 +26,7 @@
> >>>   #include "fpu/softfloat-helpers.h"
> >>>   #include "tcg/tcg.h"
> >>>   #include "exec/gdbstub.h"
> >>> +#include "cpu_helper.h"
> >>>     static void hexagon_v66_cpu_init(Object *obj) { }
> >>>   static void hexagon_v67_cpu_init(Object *obj) { } @@ -290,11
> >>> +291,18 @@ static void hexagon_cpu_reset_hold(Object *obj,
> ResetType
> >>> type)
> >>>       set_float_default_nan_pattern(0b11111111, &env->fp_status);
> >>>     #ifndef CONFIG_USER_ONLY
> >>> +    HexagonCPU *cpu = HEXAGON_CPU(cs);
> >>> +
> >>>       if (cs->cpu_index == 0) {
> >>
> >> This doesn't scale to heterogeneous emulation.
> >>
> >
> > If we have a target-specific index here (instead of cpu_index)
> > guarding the "g_sreg" allocation shared by these Hexagon vCPUs, does
> > that suffice?  Or is the problem the shared allocation itself?
> 
> I'm not sure that suffices, but it is still better from my PoV.
> 
> Let's assume we instantiate 4 ARM cores, then 2 HEX ones. The first Hexagon
> core has cpu_index=5. Now for the same example machine we instantiate
> first the Hexagon cores, then the ARM ones. The first Hexagon core has
> cpu_index=0.
> 
> > Could a heterogeneous emulation configuration consist of multiple
> > instances of (same-architecture) vCPU-groups?
> 
> Up to you if you want to model multiple hexagon SoCs in the same machine
> ;) Note in that case you could use a CPUClusterState to group.

What we are trying to model is an instance of a Hexagon that has a number of threads and some resources that are shared.  The shared resources include the TLB and global S registers.  The initial thought was to tie the shared resources to the thread with cpu_index == 0.  If we were to model a Qualcomm SoC, there would be multiple ARM cores and multiple Hexagon instances.  Each Hexagon instance would have distinct shared resources.  So, you are correct that using cpu_index is not going to scale.

What is the recommended way to model this?  I see a "nr_threads" field in CPUCore but no clear way to find the threads.  PPC has some cores that add a "threads" field.  Should we follow this approach?

HTH,
Taylor

On 3/13/25 11:47, ltaylorsimpson@gmail.com wrote:
> What we are trying to model is an instance of a Hexagon that has a number of threads and some resources that are shared.  The shared resources include the TLB and global S registers.  The initial thought was to tie the shared resources to the thread with cpu_index == 0.  If we were to model a Qualcomm SoC, there would be multiple ARM cores and multiple Hexagon instances.  Each Hexagon instance would have distinct shared resources.  So, you are correct that using cpu_index is not going to scale.
> 
> What is the recommended way to model this?  I see a "nr_threads" field in CPUCore but no clear way to find the threads.  PPC has some cores that add a "threads" field.  Should we follow this approach?

I recommend that the shared resources be modeled as a separate Object,
which is linked via object_property_add_link to all of the cpus that use it.

The linking would be under the control of the board model and/or the SoC model.


r~

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Thursday, March 13, 2025 2:07 PM
> To: ltaylorsimpson@gmail.com; 'Philippe Mathieu-Daudé'
> <philmd@linaro.org>; 'Brian Cain' <brian.cain@oss.qualcomm.com>; qemu-
> devel@nongnu.org
> Cc: Matheus Bernardino (QUIC) <quic_mathbern@quicinc.com>;
> ale@rev.ng; anjo@rev.ng; Marco Liebel (QUIC)
> <quic_mliebel@quicinc.com>; alex.bennee@linaro.org; Mark Burton (QUIC)
> <quic_mburton@quicinc.com>; Sid Manning <sidneym@quicinc.com>; Brian
> Cain <bcain@quicinc.com>
> Subject: Re: [PATCH 28/38] target/hexagon: Initialize htid, modectl regs
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> 
> On 3/13/25 11:47, ltaylorsimpson@gmail.com wrote:
> > What we are trying to model is an instance of a Hexagon that has a number
> of threads and some resources that are shared.  The shared resources
> include the TLB and global S registers.  The initial thought was to tie the
> shared resources to the thread with cpu_index == 0.  If we were to model a
> Qualcomm SoC, there would be multiple ARM cores and multiple Hexagon
> instances.  Each Hexagon instance would have distinct shared resources.  So,
> you are correct that using cpu_index is not going to scale.
> >
> > What is the recommended way to model this?  I see a "nr_threads" field in
> CPUCore but no clear way to find the threads.  PPC has some cores that add a
> "threads" field.  Should we follow this approach?
> 
> I recommend that the shared resources be modeled as a separate Object,
> which is linked via object_property_add_link to all of the cpus that use it.
> 
[Sid Manning] 
Hi Richard,
An example of shared resources would be the system registers.  They are broken down into 2 regions.  Each thread has its own copy of system registers 0-15 while registers 16-63 are global.  Right now CPUHexagonState contains a pointer, g_sreg which points back to cpu[0]'s state thus keeping one copy of the global registers, accesses are done with BQL held to avoid race conditions.

Your suggestion is to create a new object to represent the set of global system registers, I tried this:

#define TYPE_HEXAGON_G_SREG "hexagon.global_sreg"
OBJECT_DECLARE_SIMPLE_TYPE(HexagonGlobalSREGState, HEXAGON_G_SREG)
struct HexagonGlobalSREGState {
    SysBusDevice parent_obj;
    uint32_t regs[64];
};

In our virtual machine init:
vms->g_sreg = HEXAGON_G_SREG(qdev_new(TYPE_HEXAGON_G_SREG));

and 
 object_property_set_link(OBJECT(cpu), "global-sreg", OBJECT(vms->g_sreg), &error_abort);

to attach the global regs to the cpu, but the above doesn't update cpu elements the same way calls to qdev_prop_set_uint32 will do, object_property_set_link doesn’t error out and returns true.  A straight assignment would work, cpu->global_sreg = vms->g_sreg; but that isn't what you are suggesting.  Can you help me understand what I'm doing wrong.

Thanks,


> The linking would be under the control of the board model and/or the SoC
> model.
> 
> 
> r~

On 3/19/25 09:08, Sid Manning wrote:
> 
> 
>> -----Original Message-----
>> From: Richard Henderson <richard.henderson@linaro.org>
>> Sent: Thursday, March 13, 2025 2:07 PM
>> To: ltaylorsimpson@gmail.com; 'Philippe Mathieu-Daudé'
>> <philmd@linaro.org>; 'Brian Cain' <brian.cain@oss.qualcomm.com>; qemu-
>> devel@nongnu.org
>> Cc: Matheus Bernardino (QUIC) <quic_mathbern@quicinc.com>;
>> ale@rev.ng; anjo@rev.ng; Marco Liebel (QUIC)
>> <quic_mliebel@quicinc.com>; alex.bennee@linaro.org; Mark Burton (QUIC)
>> <quic_mburton@quicinc.com>; Sid Manning <sidneym@quicinc.com>; Brian
>> Cain <bcain@quicinc.com>
>> Subject: Re: [PATCH 28/38] target/hexagon: Initialize htid, modectl regs
>>
>> WARNING: This email originated from outside of Qualcomm. Please be wary
>> of any links or attachments, and do not enable macros.
>>
>> On 3/13/25 11:47, ltaylorsimpson@gmail.com wrote:
>>> What we are trying to model is an instance of a Hexagon that has a number
>> of threads and some resources that are shared.  The shared resources
>> include the TLB and global S registers.  The initial thought was to tie the
>> shared resources to the thread with cpu_index == 0.  If we were to model a
>> Qualcomm SoC, there would be multiple ARM cores and multiple Hexagon
>> instances.  Each Hexagon instance would have distinct shared resources.  So,
>> you are correct that using cpu_index is not going to scale.
>>>
>>> What is the recommended way to model this?  I see a "nr_threads" field in
>> CPUCore but no clear way to find the threads.  PPC has some cores that add a
>> "threads" field.  Should we follow this approach?
>>
>> I recommend that the shared resources be modeled as a separate Object,
>> which is linked via object_property_add_link to all of the cpus that use it.
>>
> [Sid Manning]
> Hi Richard,
> An example of shared resources would be the system registers.  They are broken down into 2 regions.  Each thread has its own copy of system registers 0-15 while registers 16-63 are global.  Right now CPUHexagonState contains a pointer, g_sreg which points back to cpu[0]'s state thus keeping one copy of the global registers, accesses are done with BQL held to avoid race conditions.
> 
> Your suggestion is to create a new object to represent the set of global system registers, I tried this:
> 
> #define TYPE_HEXAGON_G_SREG "hexagon.global_sreg"
> OBJECT_DECLARE_SIMPLE_TYPE(HexagonGlobalSREGState, HEXAGON_G_SREG)
> struct HexagonGlobalSREGState {
>      SysBusDevice parent_obj;

SysBusDevice is more than you need -- Object is sufficient here.

>      uint32_t regs[64];
> };
> 
> In our virtual machine init:
> vms->g_sreg = HEXAGON_G_SREG(qdev_new(TYPE_HEXAGON_G_SREG));
> 
> and
>   object_property_set_link(OBJECT(cpu), "global-sreg", OBJECT(vms->g_sreg), &error_abort);
> 
> to attach the global regs to the cpu, but the above doesn't update cpu elements the same way calls to qdev_prop_set_uint32 will do, object_property_set_link doesn’t error out and returns true. 

Did you add the DEFINE_PROP_LINK to match?  I'd expect something like

     DEFINE_PROP_LINK("global-sreg", HexagonCPU, g_sreg,
                      TYPE_HEXAGON_G_SREG, HexagonGlobalSREGState *),

Beyond that, I guess I'd have to see an actual patch to work out what's wrong.


r~

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Thursday, March 20, 2025 10:34 AM
> To: Sid Manning <sidneym@quicinc.com>; ltaylorsimpson@gmail.com;
> 'Philippe Mathieu-Daudé' <philmd@linaro.org>; 'Brian Cain'
> <brian.cain@oss.qualcomm.com>; qemu-devel@nongnu.org
> Cc: Matheus Bernardino (QUIC) <quic_mathbern@quicinc.com>;
> ale@rev.ng; anjo@rev.ng; Marco Liebel (QUIC)
> <quic_mliebel@quicinc.com>; alex.bennee@linaro.org; Mark Burton (QUIC)
> <quic_mburton@quicinc.com>; Brian Cain <bcain@quicinc.com>
> Subject: Re: [PATCH 28/38] target/hexagon: Initialize htid, modectl regs
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> 
> On 3/19/25 09:08, Sid Manning wrote:
> >
> >
> >> -----Original Message-----
> >> From: Richard Henderson <richard.henderson@linaro.org>
> >> Sent: Thursday, March 13, 2025 2:07 PM
> >> To: ltaylorsimpson@gmail.com; 'Philippe Mathieu-Daudé'
> >> <philmd@linaro.org>; 'Brian Cain' <brian.cain@oss.qualcomm.com>;
> >> qemu- devel@nongnu.org
> >> Cc: Matheus Bernardino (QUIC) <quic_mathbern@quicinc.com>;
> >> ale@rev.ng; anjo@rev.ng; Marco Liebel (QUIC)
> >> <quic_mliebel@quicinc.com>; alex.bennee@linaro.org; Mark Burton
> >> (QUIC) <quic_mburton@quicinc.com>; Sid Manning
> <sidneym@quicinc.com>;
> >> Brian Cain <bcain@quicinc.com>
> >> Subject: Re: [PATCH 28/38] target/hexagon: Initialize htid, modectl
> >> regs
> >>
> >> WARNING: This email originated from outside of Qualcomm. Please be
> >> wary of any links or attachments, and do not enable macros.
> >>
> >> On 3/13/25 11:47, ltaylorsimpson@gmail.com wrote:
> >>> What we are trying to model is an instance of a Hexagon that has a
> >>> number
> >> of threads and some resources that are shared.  The shared resources
> >> include the TLB and global S registers.  The initial thought was to
> >> tie the shared resources to the thread with cpu_index == 0.  If we
> >> were to model a Qualcomm SoC, there would be multiple ARM cores and
> >> multiple Hexagon instances.  Each Hexagon instance would have
> >> distinct shared resources.  So, you are correct that using cpu_index is not
> going to scale.
> >>>
> >>> What is the recommended way to model this?  I see a "nr_threads"
> >>> field in
> >> CPUCore but no clear way to find the threads.  PPC has some cores
> >> that add a "threads" field.  Should we follow this approach?
> >>
> >> I recommend that the shared resources be modeled as a separate
> >> Object, which is linked via object_property_add_link to all of the cpus that
> use it.
> >>
> > [Sid Manning]
> > Hi Richard,
> > An example of shared resources would be the system registers.  They are
> broken down into 2 regions.  Each thread has its own copy of system
> registers 0-15 while registers 16-63 are global.  Right now CPUHexagonState
> contains a pointer, g_sreg which points back to cpu[0]'s state thus keeping
> one copy of the global registers, accesses are done with BQL held to avoid
> race conditions.
> >
> > Your suggestion is to create a new object to represent the set of global
> system registers, I tried this:
> >
> > #define TYPE_HEXAGON_G_SREG "hexagon.global_sreg"
> > OBJECT_DECLARE_SIMPLE_TYPE(HexagonGlobalSREGState,
> HEXAGON_G_SREG)
> > struct HexagonGlobalSREGState {
> >      SysBusDevice parent_obj;
> 
> SysBusDevice is more than you need -- Object is sufficient here.
[Sid Manning] 
Thanks!  Will change that to Object.

> 
> >      uint32_t regs[64];
> > };
> >
> > In our virtual machine init:
> > vms->g_sreg =
> HEXAGON_G_SREG(qdev_new(TYPE_HEXAGON_G_SREG));
> >
> > and
> >   object_property_set_link(OBJECT(cpu), "global-sreg",
> > OBJECT(vms->g_sreg), &error_abort);
> >
> > to attach the global regs to the cpu, but the above doesn't update cpu
> elements the same way calls to qdev_prop_set_uint32 will do,
> object_property_set_link doesn’t error out and returns true.
> 
> Did you add the DEFINE_PROP_LINK to match?  I'd expect something like
> 
>      DEFINE_PROP_LINK("global-sreg", HexagonCPU, g_sreg,
>                       TYPE_HEXAGON_G_SREG, HexagonGlobalSREGState *),
> 
> Beyond that, I guess I'd have to see an actual patch to work out what's
> wrong.
[Sid Manning] 
Yes, PROP_LINK above is almost exactly what I added.

Below is a patch representing what I tried.  I hoped that cpu->global_sreg would be updated after the call to object_property_set_link but it was not, in the patch below I manually set it.

diff --git a/hw/hexagon/sysreg.h b/hw/hexagon/sysreg.h
new file mode 100644
index 0000000000..d7204896cf
--- /dev/null
+++ b/hw/hexagon/sysreg.h
@@ -0,0 +1,47 @@
+/*
+ * Hexagon system reg
+ * FIXME
+ */
+
+#ifndef HW_HEXAGON_HART_H
+#define HW_HEXAGON_HART_H
+#if !defined(CONFIG_USER_ONLY)
+#include "hw/sysbus.h"
+#include "qom/object.h"
+
+#define NUM_SREGS 64
+struct HexagonGlobalSREGState {
+    struct Object parent_obj;
+    uint32_t regs[NUM_SREGS];
+};
+
+#define TYPE_HEXAGON_G_SREG "hexagon.global_sreg"
+OBJECT_DECLARE_SIMPLE_TYPE(HexagonGlobalSREGState, HEXAGON_G_SREG)
+
+static void hexagon_global_sreg_init(Object *obj)
+{
+    HexagonGlobalSREGState *s = HEXAGON_G_SREG(obj);
+    /*
+     * The first 16 registers are thread local and should not come from
+     * this structure
+     */
+    for (int i = 0; i < 16; i++) {
+        s->regs[i] = 0xffffffff;
+    }
+}
+
+static const TypeInfo hexagon_sreg_info = {
+    .name          = TYPE_HEXAGON_G_SREG,
+    .parent        = TYPE_DEVICE,
+    .instance_size = sizeof(struct HexagonGlobalSREGState),
+    .instance_init = hexagon_global_sreg_init,
+};
+
+__attribute__ ((unused))
+static void hexagon_sreg_register_types(void)
+{
+    type_register_static(&hexagon_sreg_info);
+}
+#endif
+#endif
+
diff --git a/hw/hexagon/virt.c b/hw/hexagon/virt.c
index 1e7ac4e5b7..d2d599ac1d 100644
--- a/hw/hexagon/virt.c
+++ b/hw/hexagon/virt.c
@@ -10,12 +10,14 @@
 #include "hw/char/pl011.h"
 #include "hw/core/sysbus-fdt.h"
 #include "hw/hexagon/hexagon.h"
+#include "hw/hexagon/sysreg.h"
 #include "hw/hexagon/virt.h"
 #include "hw/loader.h"
 #include "hw/qdev-properties.h"
 #include "hw/register.h"
 #include "hw/timer/qct-qtimer.h"
 #include "qemu/error-report.h"
+#include "qapi/error.h"
 #include "qemu/guest-random.h"
 #include "qemu/units.h"
 #include "elf.h"
@@ -335,6 +337,7 @@ static void virt_init(MachineState *ms)
         cpu_model = HEXAGON_CPU_TYPE_NAME("v73");
     }
 
+    vms->g_sreg = HEXAGON_G_SREG(qdev_new(TYPE_HEXAGON_G_SREG));
     HexagonCPU *cpu_0 = NULL;
     for (int i = 0; i < ms->smp.cpus; i++) {
         HexagonCPU *cpu = HEXAGON_CPU(object_new(ms->cpu_type));
@@ -356,6 +359,14 @@ static void virt_init(MachineState *ms)
         qdev_prop_set_uint32(DEVICE(cpu), "qtimer-base-addr", m_cfg->qtmr_region);
         qdev_prop_set_uint32(DEVICE(cpu), "jtlb-entries",
                              m_cfg->cfgtable.jtlb_size_entries);
+        bool rc = object_property_set_link(OBJECT(cpu), "global-sreg",
+                                           OBJECT(vms->g_sreg), &error_abort);
+        g_assert(rc == true);
+
+        /* This is doing what I think object_property_set_link should do.*/
+        cpu->global_sreg = vms->g_sreg;
+
+
 
         if (!qdev_realize_and_unref(DEVICE(cpu), NULL, errp)) {
             return;
@@ -413,3 +424,5 @@ static const TypeInfo virt_machine_types[] = { {
 } };
 
 DEFINE_TYPES(virt_machine_types)
+
+type_init(hexagon_sreg_register_types)
diff --git a/include/hw/hexagon/virt.h b/include/hw/hexagon/virt.h
index 0c165a786d..dcd09d50b1 100644
--- a/include/hw/hexagon/virt.h
+++ b/include/hw/hexagon/virt.h
@@ -9,6 +9,7 @@
 #define HW_HEXAGONVIRT_H
 
 #include "hw/boards.h"
+#include "hw/hexagon/sysreg.h"
 #include "target/hexagon/cpu.h"
 
 struct HexagonVirtMachineState {
@@ -22,6 +23,7 @@ struct HexagonVirtMachineState {
     MemoryRegion tcm;
     MemoryRegion vtcm;
     DeviceState *l2vic;
+    HexagonGlobalSREGState *g_sreg;
 };
 
 void hexagon_load_fdt(const struct HexagonVirtMachineState *vms);
diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index c649aef99e..9773ee0be8 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -80,6 +80,8 @@ static const Property hexagon_cpu_properties[] = {
     DEFINE_PROP_UINT32("exec-start-addr", HexagonCPU, boot_addr, 0xffffffffULL),
     DEFINE_PROP_UINT64("config-table-addr", HexagonCPU, config_table_addr,
                        0xffffffffULL),
+    DEFINE_PROP_LINK("global-sreg", HexagonCPU, global_sreg,
+                     TYPE_HEXAGON_G_SREG, HexagonGlobalSREGState *),
 #endif
     DEFINE_PROP_UINT32("dsp-rev", HexagonCPU, rev_reg, 0),
     DEFINE_PROP_BOOL("lldb-compat", HexagonCPU, lldb_compat, false),
@@ -378,6 +380,11 @@ static void hexagon_cpu_reset_hold(Object *obj, ResetType type)
     CPUState *cs = CPU(obj);
     HexagonCPUClass *mcc = HEXAGON_CPU_GET_CLASS(obj);
     CPUHexagonState *env = cpu_env(cs);
+#ifndef CONFIG_USER_ONLY
+    HexagonCPU *cpu = HEXAGON_CPU(cs);
+    env->g_sreg = cpu->global_sreg->regs;
+#endif
+
 
     if (mcc->parent_phases.hold) {
         mcc->parent_phases.hold(obj, type);
@@ -389,11 +396,6 @@ static void hexagon_cpu_reset_hold(Object *obj, ResetType type)
     set_float_default_nan_pattern(0b11111111, &env->fp_status);
 
 #ifndef CONFIG_USER_ONLY
-    HexagonCPU *cpu = HEXAGON_CPU(cs);
-
-    if (cs->cpu_index == 0) {
-        memset(env->g_sreg, 0, sizeof(target_ulong) * NUM_SREGS);
-    }
     memset(env->t_sreg, 0, sizeof(target_ulong) * NUM_SREGS);
     memset(env->greg, 0, sizeof(target_ulong) * NUM_GREGS);
 
@@ -468,13 +470,6 @@ static void hexagon_cpu_realize(DeviceState *dev, Error **errp)
     CPUHexagonState *env = cpu_env(cs);
 #ifndef CONFIG_USER_ONLY
     hex_mmu_realize(env);
-    if (cs->cpu_index == 0) {
-        env->g_sreg = g_new0(target_ulong, NUM_SREGS);
-    } else {
-        CPUState *cpu0 = qemu_get_cpu(0);
-        CPUHexagonState *env0 = cpu_env(cpu0);
-        env->g_sreg = env0->g_sreg;
-    }
 #endif
     if (cs->cpu_index == 0) {
         env->g_pcycle_base = g_malloc0(sizeof(*env->g_pcycle_base));
diff --git a/target/hexagon/cpu.h b/target/hexagon/cpu.h
index 8b334068e2..716dd8253b 100644
--- a/target/hexagon/cpu.h
+++ b/target/hexagon/cpu.h
@@ -19,10 +19,10 @@
 #define HEXAGON_CPU_H
 
 #include "fpu/softfloat-types.h"
+#include "hw/hexagon/sysreg.h"
 
 #define NUM_GREGS 32
 #define GREG_WRITES_MAX 32
-#define NUM_SREGS 64
 #define SREG_WRITES_MAX 64
 
 #include "cpu-qom.h"
@@ -199,6 +199,7 @@ struct ArchCPU {
     uint32_t hvx_contexts;
     uint32_t boot_addr;
     uint64_t config_table_addr;
+    HexagonGlobalSREGState *global_sreg;
 #endif
 };



> 
> 
> r~

On 3/20/2025 12:38 PM, Sid Manning wrote:
>
>> -----Original Message-----
>> From: Richard Henderson <richard.henderson@linaro.org>
>> Sent: Thursday, March 20, 2025 10:34 AM
>> To: Sid Manning <sidneym@quicinc.com>; ltaylorsimpson@gmail.com;
>> 'Philippe Mathieu-Daudé' <philmd@linaro.org>; 'Brian Cain'
>> <brian.cain@oss.qualcomm.com>; qemu-devel@nongnu.org
>> Cc: Matheus Bernardino (QUIC) <quic_mathbern@quicinc.com>;
>> ale@rev.ng; anjo@rev.ng; Marco Liebel (QUIC)
>> <quic_mliebel@quicinc.com>; alex.bennee@linaro.org; Mark Burton (QUIC)
>> <quic_mburton@quicinc.com>; Brian Cain <bcain@quicinc.com>
>> Subject: Re: [PATCH 28/38] target/hexagon: Initialize htid, modectl regs
>>
>> WARNING: This email originated from outside of Qualcomm. Please be wary
>> of any links or attachments, and do not enable macros.
>>
>> On 3/19/25 09:08, Sid Manning wrote:
>>>
>>>> -----Original Message-----
>>>> From: Richard Henderson <richard.henderson@linaro.org>
>>>> Sent: Thursday, March 13, 2025 2:07 PM
>>>> To: ltaylorsimpson@gmail.com; 'Philippe Mathieu-Daudé'
>>>> <philmd@linaro.org>; 'Brian Cain' <brian.cain@oss.qualcomm.com>;
>>>> qemu- devel@nongnu.org
>>>> Cc: Matheus Bernardino (QUIC) <quic_mathbern@quicinc.com>;
>>>> ale@rev.ng; anjo@rev.ng; Marco Liebel (QUIC)
>>>> <quic_mliebel@quicinc.com>; alex.bennee@linaro.org; Mark Burton
>>>> (QUIC) <quic_mburton@quicinc.com>; Sid Manning
>> <sidneym@quicinc.com>;
>>>> Brian Cain <bcain@quicinc.com>
>>>> Subject: Re: [PATCH 28/38] target/hexagon: Initialize htid, modectl
>>>> regs
>>>>
>>>> WARNING: This email originated from outside of Qualcomm. Please be
>>>> wary of any links or attachments, and do not enable macros.
>>>>
>>>> On 3/13/25 11:47, ltaylorsimpson@gmail.com wrote:
>>>>> What we are trying to model is an instance of a Hexagon that has a
>>>>> number
>>>> of threads and some resources that are shared.  The shared resources
>>>> include the TLB and global S registers.  The initial thought was to
>>>> tie the shared resources to the thread with cpu_index == 0.  If we
>>>> were to model a Qualcomm SoC, there would be multiple ARM cores and
>>>> multiple Hexagon instances.  Each Hexagon instance would have
>>>> distinct shared resources.  So, you are correct that using cpu_index is not
>> going to scale.
>>>>> What is the recommended way to model this?  I see a "nr_threads"
>>>>> field in
>>>> CPUCore but no clear way to find the threads.  PPC has some cores
>>>> that add a "threads" field.  Should we follow this approach?
>>>>
>>>> I recommend that the shared resources be modeled as a separate
>>>> Object, which is linked via object_property_add_link to all of the cpus that
>> use it.
>>> [Sid Manning]
>>> Hi Richard,
>>> An example of shared resources would be the system registers.  They are
>> broken down into 2 regions.  Each thread has its own copy of system
>> registers 0-15 while registers 16-63 are global.  Right now CPUHexagonState
>> contains a pointer, g_sreg which points back to cpu[0]'s state thus keeping
>> one copy of the global registers, accesses are done with BQL held to avoid
>> race conditions.
>>> Your suggestion is to create a new object to represent the set of global
>> system registers, I tried this:
>>> #define TYPE_HEXAGON_G_SREG "hexagon.global_sreg"
>>> OBJECT_DECLARE_SIMPLE_TYPE(HexagonGlobalSREGState,
>> HEXAGON_G_SREG)
>>> struct HexagonGlobalSREGState {
>>>       SysBusDevice parent_obj;
>> SysBusDevice is more than you need -- Object is sufficient here.
> [Sid Manning]
> Thanks!  Will change that to Object.
>
>>>       uint32_t regs[64];
>>> };
>>>
>>> In our virtual machine init:
>>> vms->g_sreg =
>> HEXAGON_G_SREG(qdev_new(TYPE_HEXAGON_G_SREG));
>>> and
>>>    object_property_set_link(OBJECT(cpu), "global-sreg",
>>> OBJECT(vms->g_sreg), &error_abort);
>>>
>>> to attach the global regs to the cpu, but the above doesn't update cpu
>> elements the same way calls to qdev_prop_set_uint32 will do,
>> object_property_set_link doesn’t error out and returns true.
>>
>> Did you add the DEFINE_PROP_LINK to match?  I'd expect something like
>>
>>       DEFINE_PROP_LINK("global-sreg", HexagonCPU, g_sreg,
>>                        TYPE_HEXAGON_G_SREG, HexagonGlobalSREGState *),
>>
>> Beyond that, I guess I'd have to see an actual patch to work out what's
>> wrong.
> [Sid Manning]
> Yes, PROP_LINK above is almost exactly what I added.
>
> Below is a patch representing what I tried.  I hoped that cpu->global_sreg would be updated after the call to object_property_set_link but it was not, in the patch below I manually set it.
>
> diff --git a/hw/hexagon/sysreg.h b/hw/hexagon/sysreg.h
> new file mode 100644
> index 0000000000..d7204896cf
> --- /dev/null
> +++ b/hw/hexagon/sysreg.h
> @@ -0,0 +1,47 @@
> +/*
> + * Hexagon system reg
> + * FIXME
> + */
> +
> +#ifndef HW_HEXAGON_HART_H
> +#define HW_HEXAGON_HART_H
> +#if !defined(CONFIG_USER_ONLY)
> +#include "hw/sysbus.h"
> +#include "qom/object.h"
> +
> +#define NUM_SREGS 64
> +struct HexagonGlobalSREGState {
> +    struct Object parent_obj;
> +    uint32_t regs[NUM_SREGS];
> +};
> +
> +#define TYPE_HEXAGON_G_SREG "hexagon.global_sreg"
> +OBJECT_DECLARE_SIMPLE_TYPE(HexagonGlobalSREGState, HEXAGON_G_SREG)
> +
> +static void hexagon_global_sreg_init(Object *obj)
> +{
> +    HexagonGlobalSREGState *s = HEXAGON_G_SREG(obj);
> +    /*
> +     * The first 16 registers are thread local and should not come from
> +     * this structure
> +     */
> +    for (int i = 0; i < 16; i++) {
> +        s->regs[i] = 0xffffffff;
> +    }
> +}
> +
> +static const TypeInfo hexagon_sreg_info = {
> +    .name          = TYPE_HEXAGON_G_SREG,
> +    .parent        = TYPE_DEVICE,
> +    .instance_size = sizeof(struct HexagonGlobalSREGState),
> +    .instance_init = hexagon_global_sreg_init,
> +};
> +
> +__attribute__ ((unused))
> +static void hexagon_sreg_register_types(void)
> +{
> +    type_register_static(&hexagon_sreg_info);
> +}
> +#endif
> +#endif
> +
> diff --git a/hw/hexagon/virt.c b/hw/hexagon/virt.c
> index 1e7ac4e5b7..d2d599ac1d 100644
> --- a/hw/hexagon/virt.c
> +++ b/hw/hexagon/virt.c
> @@ -10,12 +10,14 @@
>   #include "hw/char/pl011.h"
>   #include "hw/core/sysbus-fdt.h"
>   #include "hw/hexagon/hexagon.h"
> +#include "hw/hexagon/sysreg.h"
>   #include "hw/hexagon/virt.h"
>   #include "hw/loader.h"
>   #include "hw/qdev-properties.h"
>   #include "hw/register.h"
>   #include "hw/timer/qct-qtimer.h"
>   #include "qemu/error-report.h"
> +#include "qapi/error.h"
>   #include "qemu/guest-random.h"
>   #include "qemu/units.h"
>   #include "elf.h"
> @@ -335,6 +337,7 @@ static void virt_init(MachineState *ms)
>           cpu_model = HEXAGON_CPU_TYPE_NAME("v73");
>       }
>   
> +    vms->g_sreg = HEXAGON_G_SREG(qdev_new(TYPE_HEXAGON_G_SREG));
>       HexagonCPU *cpu_0 = NULL;
>       for (int i = 0; i < ms->smp.cpus; i++) {
>           HexagonCPU *cpu = HEXAGON_CPU(object_new(ms->cpu_type));
> @@ -356,6 +359,14 @@ static void virt_init(MachineState *ms)
>           qdev_prop_set_uint32(DEVICE(cpu), "qtimer-base-addr", m_cfg->qtmr_region);
>           qdev_prop_set_uint32(DEVICE(cpu), "jtlb-entries",
>                                m_cfg->cfgtable.jtlb_size_entries);
> +        bool rc = object_property_set_link(OBJECT(cpu), "global-sreg",
> +                                           OBJECT(vms->g_sreg), &error_abort);
> +        g_assert(rc == true);
> +
> +        /* This is doing what I think object_property_set_link should do.*/
> +        cpu->global_sreg = vms->g_sreg;
> +
> +
>   
>           if (!qdev_realize_and_unref(DEVICE(cpu), NULL, errp)) {
>               return;
> @@ -413,3 +424,5 @@ static const TypeInfo virt_machine_types[] = { {
>   } };
>   
>   DEFINE_TYPES(virt_machine_types)
> +
> +type_init(hexagon_sreg_register_types)
> diff --git a/include/hw/hexagon/virt.h b/include/hw/hexagon/virt.h
> index 0c165a786d..dcd09d50b1 100644
> --- a/include/hw/hexagon/virt.h
> +++ b/include/hw/hexagon/virt.h
> @@ -9,6 +9,7 @@
>   #define HW_HEXAGONVIRT_H
>   
>   #include "hw/boards.h"
> +#include "hw/hexagon/sysreg.h"
>   #include "target/hexagon/cpu.h"
>   
>   struct HexagonVirtMachineState {
> @@ -22,6 +23,7 @@ struct HexagonVirtMachineState {
>       MemoryRegion tcm;
>       MemoryRegion vtcm;
>       DeviceState *l2vic;
> +    HexagonGlobalSREGState *g_sreg;
>   };
>   
>   void hexagon_load_fdt(const struct HexagonVirtMachineState *vms);
> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
> index c649aef99e..9773ee0be8 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -80,6 +80,8 @@ static const Property hexagon_cpu_properties[] = {
>       DEFINE_PROP_UINT32("exec-start-addr", HexagonCPU, boot_addr, 0xffffffffULL),
>       DEFINE_PROP_UINT64("config-table-addr", HexagonCPU, config_table_addr,
>                          0xffffffffULL),
> +    DEFINE_PROP_LINK("global-sreg", HexagonCPU, global_sreg,
> +                     TYPE_HEXAGON_G_SREG, HexagonGlobalSREGState *),
>   #endif
>       DEFINE_PROP_UINT32("dsp-rev", HexagonCPU, rev_reg, 0),
>       DEFINE_PROP_BOOL("lldb-compat", HexagonCPU, lldb_compat, false),
> @@ -378,6 +380,11 @@ static void hexagon_cpu_reset_hold(Object *obj, ResetType type)
>       CPUState *cs = CPU(obj);
>       HexagonCPUClass *mcc = HEXAGON_CPU_GET_CLASS(obj);
>       CPUHexagonState *env = cpu_env(cs);
> +#ifndef CONFIG_USER_ONLY
> +    HexagonCPU *cpu = HEXAGON_CPU(cs);
> +    env->g_sreg = cpu->global_sreg->regs;
> +#endif
> +
>   
>       if (mcc->parent_phases.hold) {
>           mcc->parent_phases.hold(obj, type);
> @@ -389,11 +396,6 @@ static void hexagon_cpu_reset_hold(Object *obj, ResetType type)
>       set_float_default_nan_pattern(0b11111111, &env->fp_status);
>   
>   #ifndef CONFIG_USER_ONLY
> -    HexagonCPU *cpu = HEXAGON_CPU(cs);
> -
> -    if (cs->cpu_index == 0) {
> -        memset(env->g_sreg, 0, sizeof(target_ulong) * NUM_SREGS);
> -    }
>       memset(env->t_sreg, 0, sizeof(target_ulong) * NUM_SREGS);
>       memset(env->greg, 0, sizeof(target_ulong) * NUM_GREGS);
>   
> @@ -468,13 +470,6 @@ static void hexagon_cpu_realize(DeviceState *dev, Error **errp)
>       CPUHexagonState *env = cpu_env(cs);
>   #ifndef CONFIG_USER_ONLY
>       hex_mmu_realize(env);
> -    if (cs->cpu_index == 0) {
> -        env->g_sreg = g_new0(target_ulong, NUM_SREGS);
> -    } else {
> -        CPUState *cpu0 = qemu_get_cpu(0);
> -        CPUHexagonState *env0 = cpu_env(cpu0);
> -        env->g_sreg = env0->g_sreg;
> -    }
>   #endif
>       if (cs->cpu_index == 0) {
>           env->g_pcycle_base = g_malloc0(sizeof(*env->g_pcycle_base));
> diff --git a/target/hexagon/cpu.h b/target/hexagon/cpu.h
> index 8b334068e2..716dd8253b 100644
> --- a/target/hexagon/cpu.h
> +++ b/target/hexagon/cpu.h
> @@ -19,10 +19,10 @@
>   #define HEXAGON_CPU_H
>   
>   #include "fpu/softfloat-types.h"
> +#include "hw/hexagon/sysreg.h"
>   
>   #define NUM_GREGS 32
>   #define GREG_WRITES_MAX 32
> -#define NUM_SREGS 64
>   #define SREG_WRITES_MAX 64
>   
>   #include "cpu-qom.h"
> @@ -199,6 +199,7 @@ struct ArchCPU {
>       uint32_t hvx_contexts;
>       uint32_t boot_addr;
>       uint64_t config_table_addr;
> +    HexagonGlobalSREGState *global_sreg;
>   #endif
>   };
>
>

I think the re-design of the global registers present in v2 should be a 
satisfactory resolution to this thread but please let me know if that's 
not the case.

>

> -----Original Message-----
> From: Brian Cain <brian.cain@oss.qualcomm.com>
> Sent: Friday, February 28, 2025 11:26 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 28/38] target/hexagon: Initialize htid, modectl regs
> 
> From: Brian Cain <bcain@quicinc.com>
> 
> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> ---
>  target/hexagon/cpu.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c index
> 36a93cc22f..2b6a707fca 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -26,6 +26,7 @@
>  #include "fpu/softfloat-helpers.h"
>  #include "tcg/tcg.h"
>  #include "exec/gdbstub.h"
> +#include "cpu_helper.h"
> 
>  static void hexagon_v66_cpu_init(Object *obj) { }  static void
> hexagon_v67_cpu_init(Object *obj) { } @@ -290,11 +291,18 @@ static void
> hexagon_cpu_reset_hold(Object *obj, ResetType type)
>      set_float_default_nan_pattern(0b11111111, &env->fp_status);
> 
>  #ifndef CONFIG_USER_ONLY
> +    HexagonCPU *cpu = HEXAGON_CPU(cs);
> +
>      if (cs->cpu_index == 0) {
>          memset(env->g_sreg, 0, sizeof(target_ulong) * NUM_SREGS);
>      }
>      memset(env->t_sreg, 0, sizeof(target_ulong) * NUM_SREGS);
>      memset(env->greg, 0, sizeof(target_ulong) * NUM_GREGS);
> +
> +    if (cs->cpu_index == 0) {
> +        arch_set_system_reg(env, HEX_SREG_MODECTL, 0x1);
> +    }

Combine with previous check cs->cpu_index == 0?

> +    arch_set_system_reg(env, HEX_SREG_HTID, cs->cpu_index);
>  #endif
>  }

Otherwise
Reviewed-by: Taylor Simpson <ltaylorsimpson@gmail.com>

> -----Original Message-----
> From: ltaylorsimpson@gmail.com <ltaylorsimpson@gmail.com>
> Sent: Tuesday, March 11, 2025 6:27 PM
> To: 'Brian Cain' <brian.cain@oss.qualcomm.com>; qemu-devel@nongnu.org
> Cc: richard.henderson@linaro.org; philmd@linaro.org; Matheus Bernardino
> (QUIC) <quic_mathbern@quicinc.com>; ale@rev.ng; anjo@rev.ng; Marco
> Liebel (QUIC) <quic_mliebel@quicinc.com>; alex.bennee@linaro.org; Mark
> Burton (QUIC) <quic_mburton@quicinc.com>; Sid Manning
> <sidneym@quicinc.com>; Brian Cain <bcain@quicinc.com>
> Subject: RE: [PATCH 28/38] target/hexagon: Initialize htid, modectl regs
> 
> WARNING: This email originated from outside of Qualcomm. Please be wary
> of any links or attachments, and do not enable macros.
> 
> > -----Original Message-----
> > From: Brian Cain <brian.cain@oss.qualcomm.com>
> > Sent: Friday, February 28, 2025 11:26 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 28/38] target/hexagon: Initialize htid, modectl regs
> >
> > From: Brian Cain <bcain@quicinc.com>
> >
> > Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> > ---
> >  target/hexagon/cpu.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c index
> > 36a93cc22f..2b6a707fca 100644
> > --- a/target/hexagon/cpu.c
> > +++ b/target/hexagon/cpu.c
> > @@ -26,6 +26,7 @@
> >  #include "fpu/softfloat-helpers.h"
> >  #include "tcg/tcg.h"
> >  #include "exec/gdbstub.h"
> > +#include "cpu_helper.h"
> >
> >  static void hexagon_v66_cpu_init(Object *obj) { }  static void
> > hexagon_v67_cpu_init(Object *obj) { } @@ -290,11 +291,18 @@ static
> > void hexagon_cpu_reset_hold(Object *obj, ResetType type)
> >      set_float_default_nan_pattern(0b11111111, &env->fp_status);
> >
> >  #ifndef CONFIG_USER_ONLY
> > +    HexagonCPU *cpu = HEXAGON_CPU(cs);
> > +
> >      if (cs->cpu_index == 0) {
> >          memset(env->g_sreg, 0, sizeof(target_ulong) * NUM_SREGS);
> >      }
> >      memset(env->t_sreg, 0, sizeof(target_ulong) * NUM_SREGS);
> >      memset(env->greg, 0, sizeof(target_ulong) * NUM_GREGS);
> > +
> > +    if (cs->cpu_index == 0) {
> > +        arch_set_system_reg(env, HEX_SREG_MODECTL, 0x1);
> > +    }
> 
> Combine with previous check cs->cpu_index == 0?
[Sid Manning] 

Yes, will make that change, thanks!

> 
> > +    arch_set_system_reg(env, HEX_SREG_HTID, cs->cpu_index);
> >  #endif
> >  }
> 
> Otherwise
> Reviewed-by: Taylor Simpson <ltaylorsimpson@gmail.com>
>