[PATCH 39/39] target/hexagon: Add pcycle setting functionality

Brian Cain posted 39 patches 11 months, 2 weeks ago
Only 37 patches received!
[PATCH 39/39] target/hexagon: Add pcycle setting functionality
Posted by Brian Cain 11 months, 2 weeks ago
Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
---
 target/hexagon/cpu.c        | 10 +++++++---
 target/hexagon/cpu_helper.c | 17 ++++++++++++++---
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 80f5e23794..4ca6add834 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -440,19 +440,23 @@ static void hexagon_cpu_realize(DeviceState *dev, Error **errp)
 #endif
 
     qemu_init_vcpu(cs);
-#ifndef CONFIG_USER_ONLY
     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);
-        env->g_pcycle_base = g_malloc0(sizeof(*env->g_pcycle_base));
     } else {
         CPUState *cpu0 = qemu_get_cpu(0);
         CPUHexagonState *env0 = cpu_env(cpu0);
         env->g_sreg = env0->g_sreg;
-        env->g_pcycle_base = env0->g_pcycle_base;
     }
 #endif
+    if (cs->cpu_index == 0) {
+        env->g_pcycle_base = g_malloc0(sizeof(*env->g_pcycle_base));
+    } else {
+        CPUState *cpu0 = qemu_get_cpu(0);
+        env->g_pcycle_base = cpu_env(cpu0)->g_pcycle_base;
+    }
 
     mcc->parent_realize(dev, errp);
 }
diff --git a/target/hexagon/cpu_helper.c b/target/hexagon/cpu_helper.c
index 9c44cb7950..08c749e9fa 100644
--- a/target/hexagon/cpu_helper.c
+++ b/target/hexagon/cpu_helper.c
@@ -70,18 +70,29 @@ uint32_t hexagon_get_sys_pcycle_count_low(CPUHexagonState *env)
 void hexagon_set_sys_pcycle_count_high(CPUHexagonState *env,
         uint32_t cycles_hi)
 {
-    g_assert_not_reached();
+    uint64_t cur_cycles = hexagon_get_sys_pcycle_count(env);
+    uint64_t cycles =
+        ((uint64_t)cycles_hi << 32) | extract64(cur_cycles, 0, 32);
+    hexagon_set_sys_pcycle_count(env, cycles);
 }
 
 void hexagon_set_sys_pcycle_count_low(CPUHexagonState *env,
         uint32_t cycles_lo)
 {
-    g_assert_not_reached();
+    uint64_t cur_cycles = hexagon_get_sys_pcycle_count(env);
+    uint64_t cycles = extract64(cur_cycles, 32, 32) | cycles_lo;
+    hexagon_set_sys_pcycle_count(env, cycles);
 }
 
 void hexagon_set_sys_pcycle_count(CPUHexagonState *env, uint64_t cycles)
 {
-    g_assert_not_reached();
+    *(env->g_pcycle_base) = cycles;
+
+    CPUState *cs;
+    CPU_FOREACH(cs) {
+        CPUHexagonState *env_ = cpu_env(cs);
+        env_->t_cycle_count = 0;
+    }
 }
 
 static void set_wait_mode(CPUHexagonState *env)
-- 
2.34.1

RE: [PATCH 39/39] target/hexagon: Add pcycle setting functionality
Posted by ltaylorsimpson@gmail.com 10 months, 3 weeks ago

> -----Original Message-----
> From: Brian Cain <brian.cain@oss.qualcomm.com>
> Sent: Friday, February 28, 2025 11:29 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
> Subject: [PATCH 39/39] target/hexagon: Add pcycle setting functionality
> 
> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> ---
>  target/hexagon/cpu.c        | 10 +++++++---
>  target/hexagon/cpu_helper.c | 17 ++++++++++++++---
>  2 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c index
> 80f5e23794..4ca6add834 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -440,19 +440,23 @@ static void hexagon_cpu_realize(DeviceState *dev,
> Error **errp)  #endif
> 
>      qemu_init_vcpu(cs);
> -#ifndef CONFIG_USER_ONLY
>      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);
> -        env->g_pcycle_base = g_malloc0(sizeof(*env->g_pcycle_base));
>      } else {
>          CPUState *cpu0 = qemu_get_cpu(0);
>          CPUHexagonState *env0 = cpu_env(cpu0);
>          env->g_sreg = env0->g_sreg;
> -        env->g_pcycle_base = env0->g_pcycle_base;
>      }
>  #endif
> +    if (cs->cpu_index == 0) {
> +        env->g_pcycle_base = g_malloc0(sizeof(*env->g_pcycle_base));

Another shared resource ...

> +    } else {
> +        CPUState *cpu0 = qemu_get_cpu(0);
> +        env->g_pcycle_base = cpu_env(cpu0)->g_pcycle_base;
> +    }
> 
>      mcc->parent_realize(dev, errp);
>  }
> diff --git a/target/hexagon/cpu_helper.c b/target/hexagon/cpu_helper.c
> index 9c44cb7950..08c749e9fa 100644
> --- a/target/hexagon/cpu_helper.c
> +++ b/target/hexagon/cpu_helper.c
> @@ -70,18 +70,29 @@ uint32_t
> hexagon_get_sys_pcycle_count_low(CPUHexagonState *env)  void
> hexagon_set_sys_pcycle_count_high(CPUHexagonState *env,
>          uint32_t cycles_hi)
>  {
> -    g_assert_not_reached();
> +    uint64_t cur_cycles = hexagon_get_sys_pcycle_count(env);
> +    uint64_t cycles =
> +        ((uint64_t)cycles_hi << 32) | extract64(cur_cycles, 0, 32);
> +    hexagon_set_sys_pcycle_count(env, cycles);
>  }
> 
>  void hexagon_set_sys_pcycle_count_low(CPUHexagonState *env,
>          uint32_t cycles_lo)
>  {
> -    g_assert_not_reached();
> +    uint64_t cur_cycles = hexagon_get_sys_pcycle_count(env);
> +    uint64_t cycles = extract64(cur_cycles, 32, 32) | cycles_lo;
> +    hexagon_set_sys_pcycle_count(env, cycles);
>  }
> 
>  void hexagon_set_sys_pcycle_count(CPUHexagonState *env, uint64_t
> cycles)  {
> -    g_assert_not_reached();

Do we need a lock here?

> +    *(env->g_pcycle_base) = cycles;
> +
> +    CPUState *cs;
> +    CPU_FOREACH(cs) {
> +        CPUHexagonState *env_ = cpu_env(cs);

This underscore is easy to miss.  Just
    cpu_env(cs)->t_cycle_count = 0;
Re: [PATCH 39/39] target/hexagon: Add pcycle setting functionality
Posted by Brian Cain 5 months, 1 week ago
On 3/19/2025 1:49 PM, ltaylorsimpson@gmail.com wrote:
>
>> -----Original Message-----
>> From: Brian Cain <brian.cain@oss.qualcomm.com>
>> Sent: Friday, February 28, 2025 11:29 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
>> Subject: [PATCH 39/39] target/hexagon: Add pcycle setting functionality
>>
>> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
>> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
>> ---
>>   target/hexagon/cpu.c        | 10 +++++++---
>>   target/hexagon/cpu_helper.c | 17 ++++++++++++++---
>>   2 files changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c index
>> 80f5e23794..4ca6add834 100644
>> --- a/target/hexagon/cpu.c
>> +++ b/target/hexagon/cpu.c
>> @@ -440,19 +440,23 @@ static void hexagon_cpu_realize(DeviceState *dev,
>> Error **errp)  #endif
>>
>>       qemu_init_vcpu(cs);
>> -#ifndef CONFIG_USER_ONLY
>>       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);
>> -        env->g_pcycle_base = g_malloc0(sizeof(*env->g_pcycle_base));
>>       } else {
>>           CPUState *cpu0 = qemu_get_cpu(0);
>>           CPUHexagonState *env0 = cpu_env(cpu0);
>>           env->g_sreg = env0->g_sreg;
>> -        env->g_pcycle_base = env0->g_pcycle_base;
>>       }
>>   #endif
>> +    if (cs->cpu_index == 0) {
>> +        env->g_pcycle_base = g_malloc0(sizeof(*env->g_pcycle_base));
> Another shared resource ...
>
>> +    } else {
>> +        CPUState *cpu0 = qemu_get_cpu(0);
>> +        env->g_pcycle_base = cpu_env(cpu0)->g_pcycle_base;
>> +    }
>>
>>       mcc->parent_realize(dev, errp);
>>   }
>> diff --git a/target/hexagon/cpu_helper.c b/target/hexagon/cpu_helper.c
>> index 9c44cb7950..08c749e9fa 100644
>> --- a/target/hexagon/cpu_helper.c
>> +++ b/target/hexagon/cpu_helper.c
>> @@ -70,18 +70,29 @@ uint32_t
>> hexagon_get_sys_pcycle_count_low(CPUHexagonState *env)  void
>> hexagon_set_sys_pcycle_count_high(CPUHexagonState *env,
>>           uint32_t cycles_hi)
>>   {
>> -    g_assert_not_reached();
>> +    uint64_t cur_cycles = hexagon_get_sys_pcycle_count(env);
>> +    uint64_t cycles =
>> +        ((uint64_t)cycles_hi << 32) | extract64(cur_cycles, 0, 32);
>> +    hexagon_set_sys_pcycle_count(env, cycles);
>>   }
>>
>>   void hexagon_set_sys_pcycle_count_low(CPUHexagonState *env,
>>           uint32_t cycles_lo)
>>   {
>> -    g_assert_not_reached();
>> +    uint64_t cur_cycles = hexagon_get_sys_pcycle_count(env);
>> +    uint64_t cycles = extract64(cur_cycles, 32, 32) | cycles_lo;
>> +    hexagon_set_sys_pcycle_count(env, cycles);
>>   }
>>
>>   void hexagon_set_sys_pcycle_count(CPUHexagonState *env, uint64_t
>> cycles)  {
>> -    g_assert_not_reached();
> Do we need a lock here?
I will address the lack of locking here in v3, seems like an appropriate 
change.
>> +    *(env->g_pcycle_base) = cycles;
>> +
>> +    CPUState *cs;
>> +    CPU_FOREACH(cs) {
>> +        CPUHexagonState *env_ = cpu_env(cs);
> This underscore is easy to miss.  Just
>      cpu_env(cs)->t_cycle_count = 0;
>
>
>