Re: [PATCH v11 11/18] gdbstub: Infer number of core registers from XML

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi Akihiko,

On 12/10/23 15:06, Akihiko Odaki wrote:
> GDBFeature has the num_regs member so use it where applicable to
> remove magic numbers.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   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 3968369554..11d4b5cd0c 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 7bea7075e1..83eafbe4b1 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -452,8 +452,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 49fb23a68a..c19f1785e4 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -541,9 +541,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;

Why not remove SysemuCPUOps::gdb_num_core_regs entirely?

> +    }
>   }