Re: [PATCH v17 08/14] gdbstub: Infer number of core registers from XML
On Wed, Dec 13, 2023 at 4:44 PM Akihiko Odaki wrote:
>
> GDBFeature has the num_regs member so use it where applicable to
> remove magic numbers.
>
> Signed-off-by: Akihiko Odaki
Acked-by: Alistair Francis
Alistair
> ---
> include/hw/core/cpu.h | 3 ++-
> target/s390x/cpu.h | 2 --
> gdbstub/gdbstub.c | 5 -
> target/arm/cpu.c| 1 -
> target/arm/cpu64.c | 1 -
> target/avr/cpu.c| 1 -
> target/hexagon/cpu.c| 1 -
> target/i386/cpu.c | 2 --
> target/loongarch/cpu.c | 2 --
> target/m68k/cpu.c | 1 -
> target/microblaze/cpu.c | 1 -
> target/riscv/cpu.c | 1 -
> target/rx/cpu.c | 1 -
> target/s390x/cpu.c | 1 -
> 14 files changed, 6 insertions(+), 17 deletions(-)
>
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index c0c8320413e5..a6214610603f 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -127,7 +127,8 @@ struct SysemuCPUOps;
> * @gdb_adjust_breakpoint: Callback for adjusting the address of a
> * 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_num_core_regs: Number of core registers accessible to GDB or 0 to
> infer
> + * from @gdb_core_xml_file.
> * @gdb_core_xml_file: File name for core registers GDB XML description.
> * @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
> * before the insn which triggers a watchpoint rather than after
> it.
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index fa3aac4f9739..2d81fbfea5cb 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -491,8 +491,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 1d5c1da1b243..801eba9a0b0b 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -546,9 +546,12 @@ void gdb_init_cpu(CPUState *cpu)
> gdb_register_feature(cpu, 0,
> cc->gdb_read_register, cc->gdb_write_register,
> feature);
> +cpu->gdb_num_regs = cpu->gdb_num_g_regs = feature->num_regs;
> }
>
> -cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
> +if (cc->gdb_num_core_regs) {
> +cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
> +}
> }
>
> void gdb_register_coprocessor(CPUState *cpu,
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index efb22a87f9ed..efcc22b1446c 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -2498,7 +2498,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void
> *data)
> #ifndef CONFIG_USER_ONLY
> cc->sysemu_ops = _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;
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 1e9c6c85aece..8a5bad54cf70 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -793,7 +793,6 @@ 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_arch_name = aarch64_gdb_arch_name;
>
> diff --git a/target/avr/cpu.c b/target/avr/cpu.c
> index 999c010dedb8..4bab9e227286 100644
> --- a/target/avr/cpu.c
> +++ b/target/avr/cpu.c
> @@ -251,7 +251,6 @@ 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->tcg_ops = _tcg_ops;
> }
> diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
> index 65ac9c75ad08..71678ef9c674 100644
> --- a/target/hexagon/cpu.c
> +++ b/target/hexagon/cpu.c
> @@ -384,7 +384,6 @@ static void hexagon_cpu_class_init(ObjectClass *c, void
> *data)
> cc->get_pc = hexagon_cpu_get_pc;
> cc->gdb_read_register = hexagon_gdb_read_register;
> 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->disas_set_info = hexagon_cpu_disas_set_info;
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index cd16cb893daf..7c2276b90656 100644
>
[PATCH v17 08/14] gdbstub: Infer number of core registers from XML
GDBFeature has the num_regs member so use it where applicable to
remove magic numbers.
Signed-off-by: Akihiko Odaki
---
include/hw/core/cpu.h | 3 ++-
target/s390x/cpu.h | 2 --
gdbstub/gdbstub.c | 5 -
target/arm/cpu.c| 1 -
target/arm/cpu64.c | 1 -
target/avr/cpu.c| 1 -
target/hexagon/cpu.c| 1 -
target/i386/cpu.c | 2 --
target/loongarch/cpu.c | 2 --
target/m68k/cpu.c | 1 -
target/microblaze/cpu.c | 1 -
target/riscv/cpu.c | 1 -
target/rx/cpu.c | 1 -
target/s390x/cpu.c | 1 -
14 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index c0c8320413e5..a6214610603f 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -127,7 +127,8 @@ struct SysemuCPUOps;
* @gdb_adjust_breakpoint: Callback for adjusting the address of a
* 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_num_core_regs: Number of core registers accessible to GDB or 0 to infer
+ * from @gdb_core_xml_file.
* @gdb_core_xml_file: File name for core registers GDB XML description.
* @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
* before the insn which triggers a watchpoint rather than after it.
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index fa3aac4f9739..2d81fbfea5cb 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -491,8 +491,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 1d5c1da1b243..801eba9a0b0b 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -546,9 +546,12 @@ void gdb_init_cpu(CPUState *cpu)
gdb_register_feature(cpu, 0,
cc->gdb_read_register, cc->gdb_write_register,
feature);
+cpu->gdb_num_regs = cpu->gdb_num_g_regs = feature->num_regs;
}
-cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
+if (cc->gdb_num_core_regs) {
+cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
+}
}
void gdb_register_coprocessor(CPUState *cpu,
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index efb22a87f9ed..efcc22b1446c 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2498,7 +2498,6 @@ static void arm_cpu_class_init(ObjectClass *oc, void
*data)
#ifndef CONFIG_USER_ONLY
cc->sysemu_ops = _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;
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 1e9c6c85aece..8a5bad54cf70 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -793,7 +793,6 @@ 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_arch_name = aarch64_gdb_arch_name;
diff --git a/target/avr/cpu.c b/target/avr/cpu.c
index 999c010dedb8..4bab9e227286 100644
--- a/target/avr/cpu.c
+++ b/target/avr/cpu.c
@@ -251,7 +251,6 @@ 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->tcg_ops = _tcg_ops;
}
diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 65ac9c75ad08..71678ef9c674 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -384,7 +384,6 @@ static void hexagon_cpu_class_init(ObjectClass *c, void
*data)
cc->get_pc = hexagon_cpu_get_pc;
cc->gdb_read_register = hexagon_gdb_read_register;
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->disas_set_info = hexagon_cpu_disas_set_info;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index cd16cb893daf..7c2276b90656 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7969,10 +7969,8 @@ 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;
#else
cc->gdb_core_xml_file =
