Re: [RFC PATCH 06/24] hw/core/cpu: Replace gdb_core_xml_file with gdb_core_feature

[Akihiko Odaki]

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;
> >   }