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

[Brian Cain]

On 3/19/2025 1:49 PM, ltaylorsimp...@gmail.com wrote:

-----Original Message-----
From: Brian Cain <brian.c...@oss.qualcomm.com>
Sent: Friday, February 28, 2025 11:29 PM
To: qemu-devel@nongnu.org
Cc: brian.c...@oss.qualcomm.com; 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
Subject: [PATCH 39/39] target/hexagon: Add pcycle setting functionality

Signed-off-by: Brian Cain <brian.c...@oss.qualcomm.com>
Signed-off-by: Matheus Tavares Bernardino <quic_mathb...@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;