This is a tree-wide change to replace gdb_core_xml_file, the path to
GDB XML file with gdb_core_feature, the pointer to GDBFeature. This
also replaces the values assigned to gdb_num_core_regs with the
num_regs member of GDBFeature where applicable to remove magic numbers.

A following change will utilize additional information provided by
GDBFeature to simplify XML file lookup.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 include/hw/core/cpu.h   | 5 +++--
 target/s390x/cpu.h      | 2 --
 gdbstub/gdbstub.c       | 6 +++---
 target/arm/cpu.c        | 4 ++--
 target/arm/cpu64.c      | 4 ++--
 target/arm/tcg/cpu32.c  | 3 ++-
 target/avr/cpu.c        | 4 ++--
 target/hexagon/cpu.c    | 2 +-
 target/i386/cpu.c       | 7 +++----
 target/loongarch/cpu.c  | 4 ++--
 target/m68k/cpu.c       | 7 ++++---
 target/microblaze/cpu.c | 4 ++--
 target/ppc/cpu_init.c   | 4 ++--
 target/riscv/cpu.c      | 7 ++++---
 target/rx/cpu.c         | 4 ++--
 target/s390x/cpu.c      | 4 ++--
 16 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index fdcbe87352..84219c1885 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -23,6 +23,7 @@
 #include "hw/qdev-core.h"
 #include "disas/dis-asm.h"
 #include "exec/cpu-common.h"
+#include "exec/gdbstub.h"
 #include "exec/hwaddr.h"
 #include "exec/memattrs.h"
 #include "qapi/qapi-types-run-state.h"
@@ -127,7 +128,7 @@ struct SysemuCPUOps;
  *       breakpoint.  Used by AVR to handle a gdb mis-feature with
  *       its Harvard architecture split code and data.
  * @gdb_num_core_regs: Number of core registers accessible to GDB.
- * @gdb_core_xml_file: File name for core registers GDB XML description.
+ * @gdb_core_feature: GDB core feature description.
  * @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
  *           before the insn which triggers a watchpoint rather than after it.
  * @gdb_arch_name: Optional callback that returns the architecture name known
@@ -163,7 +164,7 @@ struct CPUClass {
     int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
     vaddr (*gdb_adjust_breakpoint)(CPUState *cpu, vaddr addr);
 
-    const char *gdb_core_xml_file;
+    const GDBFeature *gdb_core_feature;
     gchar * (*gdb_arch_name)(CPUState *cpu);
     const char * (*gdb_get_dynamic_xml)(CPUState *cpu, const char *xmlname);
 
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index eb5b65b7d3..c5bac3230c 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -451,8 +451,6 @@ static inline void cpu_get_tb_cpu_state(CPUS390XState *env, vaddr *pc,
 #define S390_R13_REGNUM 15
 #define S390_R14_REGNUM 16
 #define S390_R15_REGNUM 17
-/* Total Core Registers. */
-#define S390_NUM_CORE_REGS 18
 
 static inline void setcc(S390CPU *cpu, uint64_t cc)
 {
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 6d9cef5b95..6f2e0cb06f 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -386,7 +386,7 @@ static const char *get_feature_xml(const char *p, const char **newp,
                 g_free(arch);
             }
             pstrcat(buf, buf_sz, "<xi:include href=\"");
-            pstrcat(buf, buf_sz, cc->gdb_core_xml_file);
+            pstrcat(buf, buf_sz, cc->gdb_core_feature->xmlname);
             pstrcat(buf, buf_sz, "\"/>");
             for (r = cpu->gdb_regs; r; r = r->next) {
                 pstrcat(buf, buf_sz, "<xi:include href=\"");
@@ -1506,7 +1506,7 @@ static void handle_query_supported(GArray *params, void *user_ctx)
 
     g_string_printf(gdbserver_state.str_buf, "PacketSize=%x", MAX_PACKET_LENGTH);
     cc = CPU_GET_CLASS(first_cpu);
-    if (cc->gdb_core_xml_file) {
+    if (cc->gdb_core_feature) {
         g_string_append(gdbserver_state.str_buf, ";qXfer:features:read+");
     }
 
@@ -1548,7 +1548,7 @@ static void handle_query_xfer_features(GArray *params, void *user_ctx)
 
     process = gdb_get_cpu_process(gdbserver_state.g_cpu);
     cc = CPU_GET_CLASS(gdbserver_state.g_cpu);
-    if (!cc->gdb_core_xml_file) {
+    if (!cc->gdb_core_feature) {
         gdb_put_packet("");
         return;
     }
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index d71a162070..a206ab6b1b 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2353,7 +2353,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
 #ifndef CONFIG_USER_ONLY
     cc->sysemu_ops = &arm_sysemu_ops;
 #endif
-    cc->gdb_num_core_regs = 26;
     cc->gdb_arch_name = arm_gdb_arch_name;
     cc->gdb_get_dynamic_xml = arm_gdb_get_dynamic_xml;
     cc->gdb_stop_before_watchpoint = true;
@@ -2378,7 +2377,8 @@ static void cpu_register_class_init(ObjectClass *oc, void *data)
     CPUClass *cc = CPU_CLASS(acc);
 
     acc->info = data;
-    cc->gdb_core_xml_file = "arm-core.xml";
+    cc->gdb_core_feature = gdb_find_static_feature("arm-core.xml");
+    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
 }
 
 void arm_cpu_register(const ARMCPUInfo *info)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 96158093cc..9c2a226159 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -754,8 +754,8 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
 
     cc->gdb_read_register = aarch64_cpu_gdb_read_register;
     cc->gdb_write_register = aarch64_cpu_gdb_write_register;
-    cc->gdb_num_core_regs = 34;
-    cc->gdb_core_xml_file = "aarch64-core.xml";
+    cc->gdb_core_feature = gdb_find_static_feature("aarch64-core.xml");
+    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
     cc->gdb_arch_name = aarch64_gdb_arch_name;
 
     object_class_property_add_bool(oc, "aarch64", aarch64_cpu_get_aarch64,
diff --git a/target/arm/tcg/cpu32.c b/target/arm/tcg/cpu32.c
index 47d2e8e781..49a823ad58 100644
--- a/target/arm/tcg/cpu32.c
+++ b/target/arm/tcg/cpu32.c
@@ -1040,7 +1040,8 @@ static void arm_v7m_class_init(ObjectClass *oc, void *data)
 
     acc->info = data;
     cc->tcg_ops = &arm_v7m_tcg_ops;
-    cc->gdb_core_xml_file = "arm-m-profile.xml";
+    cc->gdb_core_feature = gdb_find_static_feature("arm-m-profile.xml");
+    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
 }
 
 #ifndef TARGET_AARCH64
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 8f741f258c..217adc64cb 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -246,8 +246,8 @@ static void avr_cpu_class_init(ObjectClass *oc, void *data)
     cc->gdb_read_register = avr_cpu_gdb_read_register;
     cc->gdb_write_register = avr_cpu_gdb_write_register;
     cc->gdb_adjust_breakpoint = avr_cpu_gdb_adjust_breakpoint;
-    cc->gdb_num_core_regs = 35;
-    cc->gdb_core_xml_file = "avr-cpu.xml";
+    cc->gdb_core_feature = gdb_find_static_feature("avr-cpu.xml");
+    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
     cc->tcg_ops = &avr_tcg_ops;
 }
 
diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index f155936289..b54162cbeb 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -391,7 +391,7 @@ static void hexagon_cpu_class_init(ObjectClass *c, void *data)
     cc->gdb_write_register = hexagon_gdb_write_register;
     cc->gdb_num_core_regs = TOTAL_PER_THREAD_REGS;
     cc->gdb_stop_before_watchpoint = true;
-    cc->gdb_core_xml_file = "hexagon-core.xml";
+    cc->gdb_core_feature = gdb_find_static_feature("hexagon-core.xml");
     cc->disas_set_info = hexagon_cpu_disas_set_info;
     cc->tcg_ops = &hexagon_tcg_ops;
 }
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 97ad229d8b..069410985f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7963,12 +7963,11 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 
     cc->gdb_arch_name = x86_gdb_arch_name;
 #ifdef TARGET_X86_64
-    cc->gdb_core_xml_file = "i386-64bit.xml";
-    cc->gdb_num_core_regs = 66;
+    cc->gdb_core_feature = gdb_find_static_feature("i386-64bit.xml");
 #else
-    cc->gdb_core_xml_file = "i386-32bit.xml";
-    cc->gdb_num_core_regs = 50;
+    cc->gdb_core_feature = gdb_find_static_feature("i386-32bit.xml");
 #endif
+    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
     cc->disas_set_info = x86_disas_set_info;
 
     dc->user_creatable = true;
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index ad93ecac92..b204cb279d 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -722,8 +722,8 @@ static void loongarch_cpu_class_init(ObjectClass *c, void *data)
     cc->gdb_read_register = loongarch_cpu_gdb_read_register;
     cc->gdb_write_register = loongarch_cpu_gdb_write_register;
     cc->disas_set_info = loongarch_cpu_disas_set_info;
-    cc->gdb_num_core_regs = 35;
-    cc->gdb_core_xml_file = "loongarch-base64.xml";
+    cc->gdb_core_feature = gdb_find_static_feature("loongarch-base64.xml");
+    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
     cc->gdb_stop_before_watchpoint = true;
     cc->gdb_arch_name = loongarch_gdb_arch_name;
 
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index 70d58471dc..2bd7238aa8 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -572,7 +572,6 @@ static void m68k_cpu_class_init(ObjectClass *c, void *data)
 #endif
     cc->disas_set_info = m68k_cpu_disas_set_info;
 
-    cc->gdb_num_core_regs = 18;
     cc->tcg_ops = &m68k_tcg_ops;
 }
 
@@ -580,7 +579,8 @@ static void m68k_cpu_class_init_cf_core(ObjectClass *c, void *data)
 {
     CPUClass *cc = CPU_CLASS(c);
 
-    cc->gdb_core_xml_file = "cf-core.xml";
+    cc->gdb_core_feature = gdb_find_static_feature("cf-core.xml");
+    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
 }
 
 #define DEFINE_M68K_CPU_TYPE_CF(model)               \
@@ -595,7 +595,8 @@ static void m68k_cpu_class_init_m68k_core(ObjectClass *c, void *data)
 {
     CPUClass *cc = CPU_CLASS(c);
 
-    cc->gdb_core_xml_file = "m68k-core.xml";
+    cc->gdb_core_feature = gdb_find_static_feature("m68k-core.xml");
+    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
 }
 
 #define DEFINE_M68K_CPU_TYPE_M68K(model)             \
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 03c2c4db1f..47f37c2519 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -428,8 +428,8 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
     cc->sysemu_ops = &mb_sysemu_ops;
 #endif
     device_class_set_props(dc, mb_properties);
-    cc->gdb_num_core_regs = 32 + 25;
-    cc->gdb_core_xml_file = "microblaze-core.xml";
+    cc->gdb_core_feature = gdb_find_static_feature("microblaze-core.xml");
+    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
 
     cc->disas_set_info = mb_disas_set_info;
     cc->tcg_ops = &mb_tcg_ops;
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 02b7aad9b0..eb56226865 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7381,9 +7381,9 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
 
     cc->gdb_arch_name = ppc_gdb_arch_name;
 #if defined(TARGET_PPC64)
-    cc->gdb_core_xml_file = "power64-core.xml";
+    cc->gdb_core_feature = gdb_find_static_feature("power64-core.xml");
 #else
-    cc->gdb_core_xml_file = "power-core.xml";
+    cc->gdb_core_feature = gdb_find_static_feature("power-core.xml");
 #endif
     cc->disas_set_info = ppc_disas_set_info;
 
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 6b93b04453..36de35270d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1031,11 +1031,11 @@ 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";
+        cc->gdb_core_feature = gdb_find_static_feature("riscv-64bit-cpu.xml");
         break;
 #endif
     case MXL_RV32:
-        cc->gdb_core_xml_file = "riscv-32bit-cpu.xml";
+        cc->gdb_core_feature = gdb_find_static_feature("riscv-32bit-cpu.xml");
         break;
     default:
         g_assert_not_reached();
@@ -1045,6 +1045,8 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
         error_setg(errp, "misa_mxl_max must be equal to misa_mxl");
         return;
     }
+
+    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
 }
 
 /*
@@ -2138,7 +2140,6 @@ 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;
-    cc->gdb_num_core_regs = 33;
     cc->gdb_stop_before_watchpoint = true;
     cc->disas_set_info = riscv_cpu_disas_set_info;
 #ifndef CONFIG_USER_ONLY
diff --git a/target/rx/cpu.c b/target/rx/cpu.c
index 157e57da0f..b139265728 100644
--- a/target/rx/cpu.c
+++ b/target/rx/cpu.c
@@ -239,8 +239,8 @@ static void rx_cpu_class_init(ObjectClass *klass, void *data)
     cc->gdb_write_register = rx_cpu_gdb_write_register;
     cc->disas_set_info = rx_cpu_disas_set_info;
 
-    cc->gdb_num_core_regs = 26;
-    cc->gdb_core_xml_file = "rx-core.xml";
+    cc->gdb_core_feature = gdb_find_static_feature("rx-core.xml");
+    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
     cc->tcg_ops = &rx_tcg_ops;
 }
 
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index df167493c3..2a2ff8cbdc 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -348,8 +348,8 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
     s390_cpu_class_init_sysemu(cc);
 #endif
     cc->disas_set_info = s390_cpu_disas_set_info;
-    cc->gdb_num_core_regs = S390_NUM_CORE_REGS;
-    cc->gdb_core_xml_file = "s390x-core64.xml";
+    cc->gdb_core_feature = gdb_find_static_feature("s390x-core64.xml");
+    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
     cc->gdb_arch_name = s390_gdb_arch_name;
 
     s390_cpu_model_class_register_props(oc);
-- 
2.41.0

Akihiko Odaki <akihiko.odaki@daynix.com> writes:

> This is a tree-wide change to replace gdb_core_xml_file, the path to
> GDB XML file with gdb_core_feature, the pointer to GDBFeature. This
> also replaces the values assigned to gdb_num_core_regs with the
> num_regs member of GDBFeature where applicable to remove magic numbers.
>
> A following change will utilize additional information provided by
> GDBFeature to simplify XML file lookup.

re: other comment about assert(). Maybe gdb_find_static_feature() needs to assert
success because:

Thread 1 "qemu-loongarch6" received signal SIGSEGV, Segmentation fault.
loongarch_cpu_class_init (c=<optimized out>, data=<optimized out>) at ../../target/loongarch/cpu.c:726
726         cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
(gdb) p/x cc->gdb_core_feature 
$1 = 0x0
(gdb) l
721         cc->disas_set_info = loongarch_cpu_disas_set_info;
722         cc->gdb_read_register = loongarch_cpu_gdb_read_register;
723         cc->gdb_write_register = loongarch_cpu_gdb_write_register;
724         cc->disas_set_info = loongarch_cpu_disas_set_info;
725         cc->gdb_core_feature = gdb_find_static_feature("loongarch-base64.xml");
726         cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
727         cc->gdb_stop_before_watchpoint = true;
728         cc->gdb_arch_name = loongarch_gdb_arch_name;
729
730     #ifdef CONFIG_TCG

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Akihiko Odaki <akihiko.odaki@daynix.com> writes:

> This is a tree-wide change to replace gdb_core_xml_file, the path to
> GDB XML file with gdb_core_feature, the pointer to GDBFeature. This
> also replaces the values assigned to gdb_num_core_regs with the
> num_regs member of GDBFeature where applicable to remove magic numbers.
>
> A following change will utilize additional information provided by
> GDBFeature to simplify XML file lookup.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  include/hw/core/cpu.h   | 5 +++--
>  target/s390x/cpu.h      | 2 --
>  gdbstub/gdbstub.c       | 6 +++---
>  target/arm/cpu.c        | 4 ++--
>  target/arm/cpu64.c      | 4 ++--
>  target/arm/tcg/cpu32.c  | 3 ++-
>  target/avr/cpu.c        | 4 ++--
>  target/hexagon/cpu.c    | 2 +-
>  target/i386/cpu.c       | 7 +++----
>  target/loongarch/cpu.c  | 4 ++--
>  target/m68k/cpu.c       | 7 ++++---
>  target/microblaze/cpu.c | 4 ++--
>  target/ppc/cpu_init.c   | 4 ++--
>  target/riscv/cpu.c      | 7 ++++---
>  target/rx/cpu.c         | 4 ++--
>  target/s390x/cpu.c      | 4 ++--
>  16 files changed, 36 insertions(+), 35 deletions(-)
>
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index fdcbe87352..84219c1885 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -23,6 +23,7 @@
>  #include "hw/qdev-core.h"
>  #include "disas/dis-asm.h"
>  #include "exec/cpu-common.h"
> +#include "exec/gdbstub.h"
>  #include "exec/hwaddr.h"
>  #include "exec/memattrs.h"
>  #include "qapi/qapi-types-run-state.h"
> @@ -127,7 +128,7 @@ struct SysemuCPUOps;
>   *       breakpoint.  Used by AVR to handle a gdb mis-feature with
>   *       its Harvard architecture split code and data.
>   * @gdb_num_core_regs: Number of core registers accessible to GDB.

It seems redundant to have this when gdb_core_features already
encapsulates this, especially since...

> - * @gdb_core_xml_file: File name for core registers GDB XML description.
> + * @gdb_core_feature: GDB core feature description.
>   * @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
>   *           before the insn which triggers a watchpoint rather than after it.
>   * @gdb_arch_name: Optional callback that returns the architecture name known
> @@ -163,7 +164,7 @@ struct CPUClass {
>      int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
>      vaddr (*gdb_adjust_breakpoint)(CPUState *cpu, vaddr addr);
>  
> -    const char *gdb_core_xml_file;
> +    const GDBFeature *gdb_core_feature;
>      gchar * (*gdb_arch_name)(CPUState *cpu);
>      const char * (*gdb_get_dynamic_xml)(CPUState *cpu, const char *xmlname);
>  
<snip>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index d71a162070..a206ab6b1b 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -2353,7 +2353,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
>  #ifndef CONFIG_USER_ONLY
>      cc->sysemu_ops = &arm_sysemu_ops;
>  #endif
> -    cc->gdb_num_core_regs = 26;
>      cc->gdb_arch_name = arm_gdb_arch_name;
>      cc->gdb_get_dynamic_xml = arm_gdb_get_dynamic_xml;
>      cc->gdb_stop_before_watchpoint = true;
> @@ -2378,7 +2377,8 @@ static void cpu_register_class_init(ObjectClass *oc, void *data)
>      CPUClass *cc = CPU_CLASS(acc);
>  
>      acc->info = data;
> -    cc->gdb_core_xml_file = "arm-core.xml";
> +    cc->gdb_core_feature = gdb_find_static_feature("arm-core.xml");
> +    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;

You are doing assignments like this. I think something like this in
gdbstub:

modified   gdbstub/gdbstub.c
@@ -440,7 +440,7 @@ int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg, bool has_xml)
     CPUArchState *env = cpu->env_ptr;
     GDBRegisterState *r;
 
-    if (reg < cc->gdb_num_core_regs) {
+    if (reg < cc->gdb_core_feature->num_regs) {
         return cc->gdb_read_register(cpu, buf, reg, has_xml);
     }
 
@@ -459,7 +459,7 @@ static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg,
     CPUArchState *env = cpu->env_ptr;
     GDBRegisterState *r;
 
-    if (reg < cc->gdb_num_core_regs) {
+    if (reg < cc->gdb_core_feature->num_regs) {
         return cc->gdb_write_register(cpu, mem_buf, reg, has_xml);
     }

makes most of the uses go away. Some of the other arches might need
target specific tweaks.

<snip>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

On 2023/08/14 20:59, Alex Bennée wrote:
> 
> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
> 
>> This is a tree-wide change to replace gdb_core_xml_file, the path to
>> GDB XML file with gdb_core_feature, the pointer to GDBFeature. This
>> also replaces the values assigned to gdb_num_core_regs with the
>> num_regs member of GDBFeature where applicable to remove magic numbers.
>>
>> A following change will utilize additional information provided by
>> GDBFeature to simplify XML file lookup.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   include/hw/core/cpu.h   | 5 +++--
>>   target/s390x/cpu.h      | 2 --
>>   gdbstub/gdbstub.c       | 6 +++---
>>   target/arm/cpu.c        | 4 ++--
>>   target/arm/cpu64.c      | 4 ++--
>>   target/arm/tcg/cpu32.c  | 3 ++-
>>   target/avr/cpu.c        | 4 ++--
>>   target/hexagon/cpu.c    | 2 +-
>>   target/i386/cpu.c       | 7 +++----
>>   target/loongarch/cpu.c  | 4 ++--
>>   target/m68k/cpu.c       | 7 ++++---
>>   target/microblaze/cpu.c | 4 ++--
>>   target/ppc/cpu_init.c   | 4 ++--
>>   target/riscv/cpu.c      | 7 ++++---
>>   target/rx/cpu.c         | 4 ++--
>>   target/s390x/cpu.c      | 4 ++--
>>   16 files changed, 36 insertions(+), 35 deletions(-)
>>
>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>> index fdcbe87352..84219c1885 100644
>> --- a/include/hw/core/cpu.h
>> +++ b/include/hw/core/cpu.h
>> @@ -23,6 +23,7 @@
>>   #include "hw/qdev-core.h"
>>   #include "disas/dis-asm.h"
>>   #include "exec/cpu-common.h"
>> +#include "exec/gdbstub.h"
>>   #include "exec/hwaddr.h"
>>   #include "exec/memattrs.h"
>>   #include "qapi/qapi-types-run-state.h"
>> @@ -127,7 +128,7 @@ struct SysemuCPUOps;
>>    *       breakpoint.  Used by AVR to handle a gdb mis-feature with
>>    *       its Harvard architecture split code and data.
>>    * @gdb_num_core_regs: Number of core registers accessible to GDB.
> 
> It seems redundant to have this when gdb_core_features already
> encapsulates this, especially since...
> 
>> - * @gdb_core_xml_file: File name for core registers GDB XML description.
>> + * @gdb_core_feature: GDB core feature description.
>>    * @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
>>    *           before the insn which triggers a watchpoint rather than after it.
>>    * @gdb_arch_name: Optional callback that returns the architecture name known
>> @@ -163,7 +164,7 @@ struct CPUClass {
>>       int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
>>       vaddr (*gdb_adjust_breakpoint)(CPUState *cpu, vaddr addr);
>>   
>> -    const char *gdb_core_xml_file;
>> +    const GDBFeature *gdb_core_feature;
>>       gchar * (*gdb_arch_name)(CPUState *cpu);
>>       const char * (*gdb_get_dynamic_xml)(CPUState *cpu, const char *xmlname);
>>   
> <snip>
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index d71a162070..a206ab6b1b 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -2353,7 +2353,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
>>   #ifndef CONFIG_USER_ONLY
>>       cc->sysemu_ops = &arm_sysemu_ops;
>>   #endif
>> -    cc->gdb_num_core_regs = 26;
>>       cc->gdb_arch_name = arm_gdb_arch_name;
>>       cc->gdb_get_dynamic_xml = arm_gdb_get_dynamic_xml;
>>       cc->gdb_stop_before_watchpoint = true;
>> @@ -2378,7 +2377,8 @@ static void cpu_register_class_init(ObjectClass *oc, void *data)
>>       CPUClass *cc = CPU_CLASS(acc);
>>   
>>       acc->info = data;
>> -    cc->gdb_core_xml_file = "arm-core.xml";
>> +    cc->gdb_core_feature = gdb_find_static_feature("arm-core.xml");
>> +    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
> 
> You are doing assignments like this. I think something like this in
> gdbstub:
> 
> modified   gdbstub/gdbstub.c
> @@ -440,7 +440,7 @@ int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg, bool has_xml)
>       CPUArchState *env = cpu->env_ptr;
>       GDBRegisterState *r;
>   
> -    if (reg < cc->gdb_num_core_regs) {
> +    if (reg < cc->gdb_core_feature->num_regs) {
>           return cc->gdb_read_register(cpu, buf, reg, has_xml);
>       }
>   
> @@ -459,7 +459,7 @@ static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg,
>       CPUArchState *env = cpu->env_ptr;
>       GDBRegisterState *r;
>   
> -    if (reg < cc->gdb_num_core_regs) {
> +    if (reg < cc->gdb_core_feature->num_regs) {
>           return cc->gdb_write_register(cpu, mem_buf, reg, has_xml);
>       }
> 
> makes most of the uses go away. Some of the other arches might need
> target specific tweaks.

The problem is how to deal with the target specific tweaks. ppc requires 
gdb_num_core_regs to have some value greater than 
cc->gdb_core_feature->num_regs for compatibility with legacy GDB. Other 
architectures simply do not have XMLs. Simply replacing 
cc->gdb_num_core_regs with cc->gdb_core_feature->num_regs will break 
those architectures.

> 
> <snip>
> 

Akihiko Odaki <akihiko.odaki@daynix.com> writes:

> On 2023/08/14 20:59, Alex Bennée wrote:
>> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
>> 
>>> This is a tree-wide change to replace gdb_core_xml_file, the path to
>>> GDB XML file with gdb_core_feature, the pointer to GDBFeature. This
>>> also replaces the values assigned to gdb_num_core_regs with the
>>> num_regs member of GDBFeature where applicable to remove magic numbers.
>>>
>>> A following change will utilize additional information provided by
>>> GDBFeature to simplify XML file lookup.
>>>
>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>> ---
>>>   include/hw/core/cpu.h   | 5 +++--
>>>   target/s390x/cpu.h      | 2 --
>>>   gdbstub/gdbstub.c       | 6 +++---
>>>   target/arm/cpu.c        | 4 ++--
>>>   target/arm/cpu64.c      | 4 ++--
>>>   target/arm/tcg/cpu32.c  | 3 ++-
>>>   target/avr/cpu.c        | 4 ++--
>>>   target/hexagon/cpu.c    | 2 +-
>>>   target/i386/cpu.c       | 7 +++----
>>>   target/loongarch/cpu.c  | 4 ++--
>>>   target/m68k/cpu.c       | 7 ++++---
>>>   target/microblaze/cpu.c | 4 ++--
>>>   target/ppc/cpu_init.c   | 4 ++--
>>>   target/riscv/cpu.c      | 7 ++++---
>>>   target/rx/cpu.c         | 4 ++--
>>>   target/s390x/cpu.c      | 4 ++--
>>>   16 files changed, 36 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>>> index fdcbe87352..84219c1885 100644
>>> --- a/include/hw/core/cpu.h
>>> +++ b/include/hw/core/cpu.h
>>> @@ -23,6 +23,7 @@
>>>   #include "hw/qdev-core.h"
>>>   #include "disas/dis-asm.h"
>>>   #include "exec/cpu-common.h"
>>> +#include "exec/gdbstub.h"
>>>   #include "exec/hwaddr.h"
>>>   #include "exec/memattrs.h"
>>>   #include "qapi/qapi-types-run-state.h"
>>> @@ -127,7 +128,7 @@ struct SysemuCPUOps;
>>>    *       breakpoint.  Used by AVR to handle a gdb mis-feature with
>>>    *       its Harvard architecture split code and data.
>>>    * @gdb_num_core_regs: Number of core registers accessible to GDB.
>> It seems redundant to have this when gdb_core_features already
>> encapsulates this, especially since...
>> 
>>> - * @gdb_core_xml_file: File name for core registers GDB XML description.
>>> + * @gdb_core_feature: GDB core feature description.
>>>    * @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
>>>    *           before the insn which triggers a watchpoint rather than after it.
>>>    * @gdb_arch_name: Optional callback that returns the architecture name known
>>> @@ -163,7 +164,7 @@ struct CPUClass {
>>>       int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
>>>       vaddr (*gdb_adjust_breakpoint)(CPUState *cpu, vaddr addr);
>>>   -    const char *gdb_core_xml_file;
>>> +    const GDBFeature *gdb_core_feature;
>>>       gchar * (*gdb_arch_name)(CPUState *cpu);
>>>       const char * (*gdb_get_dynamic_xml)(CPUState *cpu, const char *xmlname);
>>>   
>> <snip>
>>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>>> index d71a162070..a206ab6b1b 100644
>>> --- a/target/arm/cpu.c
>>> +++ b/target/arm/cpu.c
>>> @@ -2353,7 +2353,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
>>>   #ifndef CONFIG_USER_ONLY
>>>       cc->sysemu_ops = &arm_sysemu_ops;
>>>   #endif
>>> -    cc->gdb_num_core_regs = 26;
>>>       cc->gdb_arch_name = arm_gdb_arch_name;
>>>       cc->gdb_get_dynamic_xml = arm_gdb_get_dynamic_xml;
>>>       cc->gdb_stop_before_watchpoint = true;
>>> @@ -2378,7 +2377,8 @@ static void cpu_register_class_init(ObjectClass *oc, void *data)
>>>       CPUClass *cc = CPU_CLASS(acc);
>>>         acc->info = data;
>>> -    cc->gdb_core_xml_file = "arm-core.xml";
>>> +    cc->gdb_core_feature = gdb_find_static_feature("arm-core.xml");
>>> +    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
>> You are doing assignments like this. I think something like this in
>> gdbstub:
>> modified   gdbstub/gdbstub.c
>> @@ -440,7 +440,7 @@ int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg, bool has_xml)
>>       CPUArchState *env = cpu->env_ptr;
>>       GDBRegisterState *r;
>>   -    if (reg < cc->gdb_num_core_regs) {
>> +    if (reg < cc->gdb_core_feature->num_regs) {
>>           return cc->gdb_read_register(cpu, buf, reg, has_xml);
>>       }
>>   @@ -459,7 +459,7 @@ static int gdb_write_register(CPUState *cpu,
>> uint8_t *mem_buf, int reg,
>>       CPUArchState *env = cpu->env_ptr;
>>       GDBRegisterState *r;
>>   -    if (reg < cc->gdb_num_core_regs) {
>> +    if (reg < cc->gdb_core_feature->num_regs) {
>>           return cc->gdb_write_register(cpu, mem_buf, reg, has_xml);
>>       }
>> makes most of the uses go away. Some of the other arches might need
>> target specific tweaks.
>
> The problem is how to deal with the target specific tweaks. ppc
> requires gdb_num_core_regs to have some value greater than
> cc->gdb_core_feature->num_regs for compatibility with legacy GDB.
> Other architectures simply do not have XMLs. Simply replacing
> cc->gdb_num_core_regs with cc->gdb_core_feature->num_regs will break
> those architectures.

How about:

  int core_regs = cc->gdb_core_feature ? cc->gdb_core_feature->num_regs
  : cc->gdb_num_core_regs

And document the field as for legacy gdb use only?

>
>> <snip>
>> 


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

On 2023/08/17 0:00, Alex Bennée wrote:
> 
> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
> 
>> On 2023/08/14 20:59, Alex Bennée wrote:
>>> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
>>>
>>>> This is a tree-wide change to replace gdb_core_xml_file, the path to
>>>> GDB XML file with gdb_core_feature, the pointer to GDBFeature. This
>>>> also replaces the values assigned to gdb_num_core_regs with the
>>>> num_regs member of GDBFeature where applicable to remove magic numbers.
>>>>
>>>> A following change will utilize additional information provided by
>>>> GDBFeature to simplify XML file lookup.
>>>>
>>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>>> ---
>>>>    include/hw/core/cpu.h   | 5 +++--
>>>>    target/s390x/cpu.h      | 2 --
>>>>    gdbstub/gdbstub.c       | 6 +++---
>>>>    target/arm/cpu.c        | 4 ++--
>>>>    target/arm/cpu64.c      | 4 ++--
>>>>    target/arm/tcg/cpu32.c  | 3 ++-
>>>>    target/avr/cpu.c        | 4 ++--
>>>>    target/hexagon/cpu.c    | 2 +-
>>>>    target/i386/cpu.c       | 7 +++----
>>>>    target/loongarch/cpu.c  | 4 ++--
>>>>    target/m68k/cpu.c       | 7 ++++---
>>>>    target/microblaze/cpu.c | 4 ++--
>>>>    target/ppc/cpu_init.c   | 4 ++--
>>>>    target/riscv/cpu.c      | 7 ++++---
>>>>    target/rx/cpu.c         | 4 ++--
>>>>    target/s390x/cpu.c      | 4 ++--
>>>>    16 files changed, 36 insertions(+), 35 deletions(-)
>>>>
>>>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>>>> index fdcbe87352..84219c1885 100644
>>>> --- a/include/hw/core/cpu.h
>>>> +++ b/include/hw/core/cpu.h
>>>> @@ -23,6 +23,7 @@
>>>>    #include "hw/qdev-core.h"
>>>>    #include "disas/dis-asm.h"
>>>>    #include "exec/cpu-common.h"
>>>> +#include "exec/gdbstub.h"
>>>>    #include "exec/hwaddr.h"
>>>>    #include "exec/memattrs.h"
>>>>    #include "qapi/qapi-types-run-state.h"
>>>> @@ -127,7 +128,7 @@ struct SysemuCPUOps;
>>>>     *       breakpoint.  Used by AVR to handle a gdb mis-feature with
>>>>     *       its Harvard architecture split code and data.
>>>>     * @gdb_num_core_regs: Number of core registers accessible to GDB.
>>> It seems redundant to have this when gdb_core_features already
>>> encapsulates this, especially since...
>>>
>>>> - * @gdb_core_xml_file: File name for core registers GDB XML description.
>>>> + * @gdb_core_feature: GDB core feature description.
>>>>     * @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
>>>>     *           before the insn which triggers a watchpoint rather than after it.
>>>>     * @gdb_arch_name: Optional callback that returns the architecture name known
>>>> @@ -163,7 +164,7 @@ struct CPUClass {
>>>>        int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
>>>>        vaddr (*gdb_adjust_breakpoint)(CPUState *cpu, vaddr addr);
>>>>    -    const char *gdb_core_xml_file;
>>>> +    const GDBFeature *gdb_core_feature;
>>>>        gchar * (*gdb_arch_name)(CPUState *cpu);
>>>>        const char * (*gdb_get_dynamic_xml)(CPUState *cpu, const char *xmlname);
>>>>    
>>> <snip>
>>>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>>>> index d71a162070..a206ab6b1b 100644
>>>> --- a/target/arm/cpu.c
>>>> +++ b/target/arm/cpu.c
>>>> @@ -2353,7 +2353,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
>>>>    #ifndef CONFIG_USER_ONLY
>>>>        cc->sysemu_ops = &arm_sysemu_ops;
>>>>    #endif
>>>> -    cc->gdb_num_core_regs = 26;
>>>>        cc->gdb_arch_name = arm_gdb_arch_name;
>>>>        cc->gdb_get_dynamic_xml = arm_gdb_get_dynamic_xml;
>>>>        cc->gdb_stop_before_watchpoint = true;
>>>> @@ -2378,7 +2377,8 @@ static void cpu_register_class_init(ObjectClass *oc, void *data)
>>>>        CPUClass *cc = CPU_CLASS(acc);
>>>>          acc->info = data;
>>>> -    cc->gdb_core_xml_file = "arm-core.xml";
>>>> +    cc->gdb_core_feature = gdb_find_static_feature("arm-core.xml");
>>>> +    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
>>> You are doing assignments like this. I think something like this in
>>> gdbstub:
>>> modified   gdbstub/gdbstub.c
>>> @@ -440,7 +440,7 @@ int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg, bool has_xml)
>>>        CPUArchState *env = cpu->env_ptr;
>>>        GDBRegisterState *r;
>>>    -    if (reg < cc->gdb_num_core_regs) {
>>> +    if (reg < cc->gdb_core_feature->num_regs) {
>>>            return cc->gdb_read_register(cpu, buf, reg, has_xml);
>>>        }
>>>    @@ -459,7 +459,7 @@ static int gdb_write_register(CPUState *cpu,
>>> uint8_t *mem_buf, int reg,
>>>        CPUArchState *env = cpu->env_ptr;
>>>        GDBRegisterState *r;
>>>    -    if (reg < cc->gdb_num_core_regs) {
>>> +    if (reg < cc->gdb_core_feature->num_regs) {
>>>            return cc->gdb_write_register(cpu, mem_buf, reg, has_xml);
>>>        }
>>> makes most of the uses go away. Some of the other arches might need
>>> target specific tweaks.
>>
>> The problem is how to deal with the target specific tweaks. ppc
>> requires gdb_num_core_regs to have some value greater than
>> cc->gdb_core_feature->num_regs for compatibility with legacy GDB.
>> Other architectures simply do not have XMLs. Simply replacing
>> cc->gdb_num_core_regs with cc->gdb_core_feature->num_regs will break
>> those architectures.
> 
> How about:
> 
>    int core_regs = cc->gdb_core_feature ? cc->gdb_core_feature->num_regs
>    : cc->gdb_num_core_regs
> 
> And document the field as for legacy gdb use only?

I'm not sure. It looks a more error-prone way to derive the core 
register number.

> 
>>
>>> <snip>
>>>
> 
> 

On 31/7/23 10:43, Akihiko Odaki wrote:
> This is a tree-wide change to replace gdb_core_xml_file, the path to
> GDB XML file with gdb_core_feature, the pointer to GDBFeature. This
> also replaces the values assigned to gdb_num_core_regs with the
> num_regs member of GDBFeature where applicable to remove magic numbers.
> 
> A following change will utilize additional information provided by
> GDBFeature to simplify XML file lookup.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   include/hw/core/cpu.h   | 5 +++--
>   target/s390x/cpu.h      | 2 --
>   gdbstub/gdbstub.c       | 6 +++---
>   target/arm/cpu.c        | 4 ++--
>   target/arm/cpu64.c      | 4 ++--
>   target/arm/tcg/cpu32.c  | 3 ++-
>   target/avr/cpu.c        | 4 ++--
>   target/hexagon/cpu.c    | 2 +-
>   target/i386/cpu.c       | 7 +++----
>   target/loongarch/cpu.c  | 4 ++--
>   target/m68k/cpu.c       | 7 ++++---
>   target/microblaze/cpu.c | 4 ++--
>   target/ppc/cpu_init.c   | 4 ++--
>   target/riscv/cpu.c      | 7 ++++---
>   target/rx/cpu.c         | 4 ++--
>   target/s390x/cpu.c      | 4 ++--
>   16 files changed, 36 insertions(+), 35 deletions(-)


> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index d71a162070..a206ab6b1b 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -2353,7 +2353,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
>   #ifndef CONFIG_USER_ONLY
>       cc->sysemu_ops = &arm_sysemu_ops;
>   #endif
> -    cc->gdb_num_core_regs = 26;
>       cc->gdb_arch_name = arm_gdb_arch_name;
>       cc->gdb_get_dynamic_xml = arm_gdb_get_dynamic_xml;
>       cc->gdb_stop_before_watchpoint = true;
> @@ -2378,7 +2377,8 @@ static void cpu_register_class_init(ObjectClass *oc, void *data)
>       CPUClass *cc = CPU_CLASS(acc);
>   
>       acc->info = data;
> -    cc->gdb_core_xml_file = "arm-core.xml";
> +    cc->gdb_core_feature = gdb_find_static_feature("arm-core.xml");

Can we have:

   cc->gdb_core_feature = gdb_find_static_feature(cc->gdb_core_xml_file);

once in hw/core/cpu-common.c::cpu_class_init()?

(haven't verified, just wondering)

> +    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
>   }

On 2023/07/31 22:27, Philippe Mathieu-Daudé wrote:
> On 31/7/23 10:43, Akihiko Odaki wrote:
>> This is a tree-wide change to replace gdb_core_xml_file, the path to
>> GDB XML file with gdb_core_feature, the pointer to GDBFeature. This
>> also replaces the values assigned to gdb_num_core_regs with the
>> num_regs member of GDBFeature where applicable to remove magic numbers.
>>
>> A following change will utilize additional information provided by
>> GDBFeature to simplify XML file lookup.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   include/hw/core/cpu.h   | 5 +++--
>>   target/s390x/cpu.h      | 2 --
>>   gdbstub/gdbstub.c       | 6 +++---
>>   target/arm/cpu.c        | 4 ++--
>>   target/arm/cpu64.c      | 4 ++--
>>   target/arm/tcg/cpu32.c  | 3 ++-
>>   target/avr/cpu.c        | 4 ++--
>>   target/hexagon/cpu.c    | 2 +-
>>   target/i386/cpu.c       | 7 +++----
>>   target/loongarch/cpu.c  | 4 ++--
>>   target/m68k/cpu.c       | 7 ++++---
>>   target/microblaze/cpu.c | 4 ++--
>>   target/ppc/cpu_init.c   | 4 ++--
>>   target/riscv/cpu.c      | 7 ++++---
>>   target/rx/cpu.c         | 4 ++--
>>   target/s390x/cpu.c      | 4 ++--
>>   16 files changed, 36 insertions(+), 35 deletions(-)
> 
> 
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index d71a162070..a206ab6b1b 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -2353,7 +2353,6 @@ static void arm_cpu_class_init(ObjectClass *oc, 
>> void *data)
>>   #ifndef CONFIG_USER_ONLY
>>       cc->sysemu_ops = &arm_sysemu_ops;
>>   #endif
>> -    cc->gdb_num_core_regs = 26;
>>       cc->gdb_arch_name = arm_gdb_arch_name;
>>       cc->gdb_get_dynamic_xml = arm_gdb_get_dynamic_xml;
>>       cc->gdb_stop_before_watchpoint = true;
>> @@ -2378,7 +2377,8 @@ static void cpu_register_class_init(ObjectClass 
>> *oc, void *data)
>>       CPUClass *cc = CPU_CLASS(acc);
>>       acc->info = data;
>> -    cc->gdb_core_xml_file = "arm-core.xml";
>> +    cc->gdb_core_feature = gdb_find_static_feature("arm-core.xml");
> 
> Can we have:
> 
>    cc->gdb_core_feature = gdb_find_static_feature(cc->gdb_core_xml_file);
> 
> once in hw/core/cpu-common.c::cpu_class_init()?
> 
> (haven't verified, just wondering)

Unfortunately no, cpu_class_init() earlier than this line. Also the 
feature needs to be resolved here because the next line refers to it.

> 
>> +    cc->gdb_num_core_regs = cc->gdb_core_feature->num_regs;
>>   }
> 
>