Re: [PATCH v2 2/3] target/riscv: Initialize gdb_core_xml_file only once

[Daniel Henrique Barboza]

On 10/14/23 00:35, Akihiko Odaki wrote:

gdb_core_xml_file was assigned each time a CPU is instantiated before
this change.

Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
---
  target/riscv/cpu.c         | 5 +++++
  target/riscv/tcg/tcg-cpu.c | 4 ----
  2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ac4a6c7eec..a811215150 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1575,6 +1575,11 @@ static void riscv_cpu_class_init(ObjectClass *c, void 
*data)
      cc->get_pc = riscv_cpu_get_pc;
      cc->gdb_read_register = riscv_cpu_gdb_read_register;
      cc->gdb_write_register = riscv_cpu_gdb_write_register;
+#ifdef TARGET_RISCV64
+    cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
+#elif defined(TARGET_RISCV32)
+    cc->gdb_core_xml_file = "riscv-32bit-cpu.xml";
+#endif
      cc->gdb_num_core_regs = 33;
      cc->gdb_stop_before_watchpoint = true;
      cc->disas_set_info = riscv_cpu_disas_set_info;
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index e0cbc56320..626fb2acea 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -150,8 +150,6 @@ static void riscv_cpu_validate_misa_priv(CPURISCVState 
*env, Error **errp)

static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)

  {
-    RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
-    CPUClass *cc = CPU_CLASS(mcc);
      CPURISCVState *env = &cpu->env;

/* Validate that MISA_MXL is set properly. */

@@ -159,11 +157,9 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, 
Error **errp)
  #ifdef TARGET_RISCV64
      case MXL_RV64:
      case MXL_RV128:
-        cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
          break;
  #elif defined(TARGET_RISCV32)
      case MXL_RV32:
-        cc->gdb_core_xml_file = "riscv-32bit-cpu.xml";
          break;

Hmmm the issue here is that, in patch 1, you added an "elif 
defined(TARGET_RISCV32)"
based on an assumption that you changed here since there's no more gdb_core 
files being
set.

My suggestion is to use patch 1 from v1, where you removed the misa_mxl_max == 
misa_mxl
check at the end of this function. And then in this patch you can remove this 
function
altogether since you're assigning gdb_core in riscv_cpu_class_init() and the 
function will
be left doing nothing of note.


Thanks,

Daniel

  #endif
      default: