Re: [PATCH v2 13/13] target/loongarch: Add new structure CPUSysState

[Philippe Mathieu-Daudé]

On 2/6/26 05:00, Bibo Mao wrote:
New structure CPUSysState is added here, it contains CSR registers
now, in future TLB and timer can be moved to this structure also.
It is only code movement, no function change.

Signed-off-by: Bibo Mao <maob...@loongson.cn>
---
  target/loongarch/cpu.c                        |   4 +-
  target/loongarch/cpu.h                        |  35 +++---
  target/loongarch/csr.c                        |   4 +-
  target/loongarch/csr.h                        |   2 +-
  target/loongarch/machine.c                    | 118 +++++++++---------
  .../tcg/insn_trans/trans_extra.c.inc          |   4 +-
  6 files changed, 86 insertions(+), 81 deletions(-)


diff --git a/target/loongarch/csr.h b/target/loongarch/csr.h
index ed7c603a0b..ef71cdf30f 100644
--- a/target/loongarch/csr.h
+++ b/target/loongarch/csr.h
@@ -29,6 +29,6 @@ CSRInfo *get_csr(unsigned int csr_num);
  bool set_csr_flag(unsigned int csr_num, int flag);
  static inline int get_csr_offset(const CSRInfo *csr, int vm_level)
  {
-    return csr->offset;
+    return csr->offset + offsetof(CPULoongArchState, sys_states[vm_level]);
  }
  #endif /* TARGET_LOONGARCH_CSR_H */
diff --git a/target/loongarch/machine.c b/target/loongarch/machine.c
index 4db53fec26..931a5ca5ba 100644
--- a/target/loongarch/machine.c
+++ b/target/loongarch/machine.c
@@ -58,9 +58,9 @@ static const VMStateDescription vmstate_msgint = {
      .minimum_version_id = 1,
      .needed = msgint_needed,
      .fields = (const VMStateField[]) {
-        VMSTATE_UINT64_ARRAY(env.CSR_MSGIS, LoongArchCPU, N_MSGIS),
-        VMSTATE_UINT64(env.CSR_MSGIR, LoongArchCPU),
-        VMSTATE_UINT64(env.CSR_MSGIE, LoongArchCPU),
+        VMSTATE_UINT64_ARRAY(env.sys_states[0].CSR_MSGIS, LoongArchCPU, 
N_MSGIS),
+        VMSTATE_UINT64(env.sys_states[0].CSR_MSGIR, LoongArchCPU),
+        VMSTATE_UINT64(env.sys_states[0].CSR_MSGIE, LoongArchCPU),

No impact right now, but all the migration stream will be broken when
you add more sys_states.

Will sys_states number be static or dynamic?

          VMSTATE_END_OF_LIST()
      },
  };


diff --git a/target/loongarch/tcg/insn_trans/trans_extra.c.inc 
b/target/loongarch/tcg/insn_trans/trans_extra.c.inc
index 298a80cff5..838ac7e6b4 100644
--- a/target/loongarch/tcg/insn_trans/trans_extra.c.inc
+++ b/target/loongarch/tcg/insn_trans/trans_extra.c.inc
@@ -46,13 +46,15 @@ static bool gen_rdtime(DisasContext *ctx, arg_rr *a,
  {
      TCGv dst1 = gpr_dst(ctx, a->rd, EXT_NONE);
      TCGv dst2 = gpr_dst(ctx, a->rj, EXT_NONE);
+    tcg_target_long offset;
  
      translator_io_start(&ctx->base);
      gen_helper_rdtime_d(dst1, tcg_env);
      if (word) {
          tcg_gen_sextract_tl(dst1, dst1, high ? 32 : 0, 32);
      }
-    tcg_gen_ld_i64(dst2, tcg_env, offsetof(CPULoongArchState, CSR_TID));
+    offset = offsetof(CPUSysState, CSR_TID) + offsetof(CPULoongArchState, 
sys_states[0]);

This seems fragile. Could you move as a preliminary patch change?
Also write it reversed:

  offsetof(CPULoongArchState, sys_states[0]) + offsetof(CPUSysState, 
CSR_TID)

+    tcg_gen_ld_i64(dst2, tcg_env, offset);
  
      return true;
  }