Re: [PATCH v4 03/11] target/loongarch: Add GDB support for loongarch32 mode

2023-08-08 Thread Richard Henderson 

On 8/7/23 18:54, Jiajie Chen wrote:

GPRs and PC are 32-bit wide in loongarch32 mode.

Signed-off-by: Jiajie Chen 
---
  configs/targets/loongarch64-softmmu.mak |  2 +-
  gdb-xml/loongarch-base32.xml| 45 +
  target/loongarch/cpu.c  | 10 +-
  target/loongarch/gdbstub.c  | 32 ++
  4 files changed, 80 insertions(+), 9 deletions(-)
  create mode 100644 gdb-xml/loongarch-base32.xml

diff --git a/configs/targets/loongarch64-softmmu.mak 
b/configs/targets/loongarch64-softmmu.mak
index 9abc99056f..f23780fdd8 100644
--- a/configs/targets/loongarch64-softmmu.mak
+++ b/configs/targets/loongarch64-softmmu.mak
@@ -1,5 +1,5 @@
  TARGET_ARCH=loongarch64
  TARGET_BASE_ARCH=loongarch
  TARGET_SUPPORTS_MTTCG=y
-TARGET_XML_FILES= gdb-xml/loongarch-base64.xml gdb-xml/loongarch-fpu.xml
+TARGET_XML_FILES= gdb-xml/loongarch-base32.xml gdb-xml/loongarch-base64.xml 
gdb-xml/loongarch-fpu.xml
  TARGET_NEED_FDT=y
diff --git a/gdb-xml/loongarch-base32.xml b/gdb-xml/loongarch-base32.xml
new file mode 100644
index 00..af47bbd3da
--- /dev/null
+++ b/gdb-xml/loongarch-base32.xml
@@ -0,0 +1,45 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 3bd293d00a..13d4fccbd3 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -694,7 +694,13 @@ static const struct SysemuCPUOps loongarch_sysemu_ops = {
  
  static gchar *loongarch_gdb_arch_name(CPUState *cs)

  {
-return g_strdup("loongarch64");
+LoongArchCPU *cpu = LOONGARCH_CPU(cs);
+CPULoongArchState *env = &cpu->env;
+if (LOONGARCH_CPUCFG_ARCH(env, LA64)) {
+return g_strdup("loongarch64");
+} else {
+return g_strdup("loongarch32");
+}
  }
  
  static void loongarch_cpu_class_init(ObjectClass *c, void *data)

@@ -734,6 +740,8 @@ static void loongarch_cpu_class_init(ObjectClass *c, void 
*data)
  
  static void loongarch32_cpu_class_init(ObjectClass *c, void *data)

  {
+CPUClass *cc = CPU_CLASS(c);
+cc->gdb_core_xml_file = "loongarch-base32.xml";
  }
  
  #define DEFINE_LOONGARCH_CPU_TYPE(model, initfn) \

diff --git a/target/loongarch/gdbstub.c b/target/loongarch/gdbstub.c
index 0752fff924..0dfd1c8bb9 100644
--- a/target/loongarch/gdbstub.c
+++ b/target/loongarch/gdbstub.c
@@ -34,16 +34,25 @@ int loongarch_cpu_gdb_read_register(CPUState *cs, 
GByteArray *mem_buf, int n)
  {
  LoongArchCPU *cpu = LOONGARCH_CPU(cs);
  CPULoongArchState *env = &cpu->env;
+uint64_t val;
  
  if (0 <= n && n < 32) {

-return gdb_get_regl(mem_buf, env->gpr[n]);
+val = env->gpr[n];
  } else if (n == 32) {
  /* orig_a0 */
-return gdb_get_regl(mem_buf, 0);
+val = 0;
  } else if (n == 33) {
-return gdb_get_regl(mem_buf, env->pc);
+val = env->pc;
  } else if (n == 34) {
-return gdb_get_regl(mem_buf, env->CSR_BADV);
+val = env->CSR_BADV;
+}
+
+if (0 <= n && n <= 34) {
+if (LOONGARCH_CPUCFG_ARCH(env, LA64)) {
+return gdb_get_reg64(mem_buf, val);
+} else {
+return gdb_get_reg32(mem_buf, val);
+}


Is it an existing bug that BADV is readable...



@@ -52,15 +61,24 @@ int loongarch_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
  {
  LoongArchCPU *cpu = LOONGARCH_CPU(cs);
  CPULoongArchState *env = &cpu->env;
-target_ulong tmp = ldtul_p(mem_buf);
+target_ulong tmp;
+int read_length;
  int length = 0;
  
+if (LOONGARCH_CPUCFG_ARCH(env, LA64)) {

+tmp = ldq_p(mem_buf);
+read_length = 8;
+} else {
+tmp = ldl_p(mem_buf);
+read_length = 4;
+}
+
  if (0 <= n && n < 32) {
  env->gpr[n] = tmp;
-length = sizeof(target_ulong);
+length = read_length;
  } else if (n == 33) {
  env->pc = tmp;
-length = sizeof(target_ulong);
+length = read_length;
  }


... but not writable?  I can't immediately see any reason why gdbstub should reject writes 
to BADV.


But for this patch:
Reviewed-by: Richard Henderson 


r~

[PATCH v4 03/11] target/loongarch: Add GDB support for loongarch32 mode

2023-08-07 Thread Jiajie Chen 
GPRs and PC are 32-bit wide in loongarch32 mode.

Signed-off-by: Jiajie Chen 
---
 configs/targets/loongarch64-softmmu.mak |  2 +-
 gdb-xml/loongarch-base32.xml| 45 +
 target/loongarch/cpu.c  | 10 +-
 target/loongarch/gdbstub.c  | 32 ++
 4 files changed, 80 insertions(+), 9 deletions(-)
 create mode 100644 gdb-xml/loongarch-base32.xml

diff --git a/configs/targets/loongarch64-softmmu.mak 
b/configs/targets/loongarch64-softmmu.mak
index 9abc99056f..f23780fdd8 100644
--- a/configs/targets/loongarch64-softmmu.mak
+++ b/configs/targets/loongarch64-softmmu.mak
@@ -1,5 +1,5 @@
 TARGET_ARCH=loongarch64
 TARGET_BASE_ARCH=loongarch
 TARGET_SUPPORTS_MTTCG=y
-TARGET_XML_FILES= gdb-xml/loongarch-base64.xml gdb-xml/loongarch-fpu.xml
+TARGET_XML_FILES= gdb-xml/loongarch-base32.xml gdb-xml/loongarch-base64.xml 
gdb-xml/loongarch-fpu.xml
 TARGET_NEED_FDT=y
diff --git a/gdb-xml/loongarch-base32.xml b/gdb-xml/loongarch-base32.xml
new file mode 100644
index 00..af47bbd3da
--- /dev/null
+++ b/gdb-xml/loongarch-base32.xml
@@ -0,0 +1,45 @@
+
+
+
+
+
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+  
+
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 3bd293d00a..13d4fccbd3 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -694,7 +694,13 @@ static const struct SysemuCPUOps loongarch_sysemu_ops = {
 
 static gchar *loongarch_gdb_arch_name(CPUState *cs)
 {
-return g_strdup("loongarch64");
+LoongArchCPU *cpu = LOONGARCH_CPU(cs);
+CPULoongArchState *env = &cpu->env;
+if (LOONGARCH_CPUCFG_ARCH(env, LA64)) {
+return g_strdup("loongarch64");
+} else {
+return g_strdup("loongarch32");
+}
 }
 
 static void loongarch_cpu_class_init(ObjectClass *c, void *data)
@@ -734,6 +740,8 @@ static void loongarch_cpu_class_init(ObjectClass *c, void 
*data)
 
 static void loongarch32_cpu_class_init(ObjectClass *c, void *data)
 {
+CPUClass *cc = CPU_CLASS(c);
+cc->gdb_core_xml_file = "loongarch-base32.xml";
 }
 
 #define DEFINE_LOONGARCH_CPU_TYPE(model, initfn) \
diff --git a/target/loongarch/gdbstub.c b/target/loongarch/gdbstub.c
index 0752fff924..0dfd1c8bb9 100644
--- a/target/loongarch/gdbstub.c
+++ b/target/loongarch/gdbstub.c
@@ -34,16 +34,25 @@ int loongarch_cpu_gdb_read_register(CPUState *cs, 
GByteArray *mem_buf, int n)
 {
 LoongArchCPU *cpu = LOONGARCH_CPU(cs);
 CPULoongArchState *env = &cpu->env;
+uint64_t val;
 
 if (0 <= n && n < 32) {
-return gdb_get_regl(mem_buf, env->gpr[n]);
+val = env->gpr[n];
 } else if (n == 32) {
 /* orig_a0 */
-return gdb_get_regl(mem_buf, 0);
+val = 0;
 } else if (n == 33) {
-return gdb_get_regl(mem_buf, env->pc);
+val = env->pc;
 } else if (n == 34) {
-return gdb_get_regl(mem_buf, env->CSR_BADV);
+val = env->CSR_BADV;
+}
+
+if (0 <= n && n <= 34) {
+if (LOONGARCH_CPUCFG_ARCH(env, LA64)) {
+return gdb_get_reg64(mem_buf, val);
+} else {
+return gdb_get_reg32(mem_buf, val);
+}
 }
 return 0;
 }
@@ -52,15 +61,24 @@ int loongarch_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 {
 LoongArchCPU *cpu = LOONGARCH_CPU(cs);
 CPULoongArchState *env = &cpu->env;
-target_ulong tmp = ldtul_p(mem_buf);
+target_ulong tmp;
+int read_length;
 int length = 0;
 
+if (LOONGARCH_CPUCFG_ARCH(env, LA64)) {
+tmp = ldq_p(mem_buf);
+read_length = 8;
+} else {
+tmp = ldl_p(mem_buf);
+read_length = 4;
+}
+
 if (0 <= n && n < 32) {
 env->gpr[n] = tmp;
-length = sizeof(target_ulong);
+length = read_length;
 } else if (n == 33) {
 env->pc = tmp;
-length = sizeof(target_ulong);
+length = read_length;
 }
 return length;
 }
-- 
2.41.0