Re: [RFC PATCH 15/24] target/arm: Fill new members of GDBFeature

[Akihiko Odaki]

On 2023/08/17 0:03, Alex Bennée wrote:
> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
>
> > On 2023/08/14 23:56, Alex Bennée wrote:
> > > Akihiko Odaki <akihiko.odaki@daynix.com> writes:
> > >
> > > > These members will be used to help plugins to identify registers.
> > > >
> > > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > > > ---
> > > >    target/arm/gdbstub.c   | 46 +++++++++++++++++++++++++++---------------
> > > >    target/arm/gdbstub64.c | 42 +++++++++++++++++++++++++-------------
> > > >    2 files changed, 58 insertions(+), 30 deletions(-)
> > > >
> > > > diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
> > > > index 100a6eed15..56d24028f6 100644
> > > > --- a/target/arm/gdbstub.c
> > > > +++ b/target/arm/gdbstub.c
> > > > @@ -270,6 +270,7 @@ static void arm_gen_one_feature_sysreg(GString *s,
> > > >        g_string_append_printf(s, " regnum=\"%d\"", regnum);
> > > >        g_string_append_printf(s, " group=\"cp_regs\"/>");
> > > >        dyn_feature->data.cpregs.keys[dyn_feature->desc.num_regs] = ri_key;
> > > > +    ((const char **)dyn_feature->desc.regs)[dyn_feature->desc.num_regs] = 
> > > > ri->name;
> > > >        dyn_feature->desc.num_regs++;
> > > >    }
> > > >    @@ -316,6 +317,8 @@ static GDBFeature
> > > > *arm_gen_dynamic_sysreg_feature(CPUState *cs, int base_reg)
> > > >        DynamicGDBFeatureInfo *dyn_feature = &cpu->dyn_sysreg_feature;
> > > >        gsize num_regs = g_hash_table_size(cpu->cp_regs);
> > > >    +    dyn_feature->desc.name = "org.qemu.gdb.arm.sys.regs";
> > > > +    dyn_feature->desc.regs = g_new(const char *, num_regs);
> > > AIUI this means we now have an array of register names which mirrors
> > > the
> > > names embedded in the XML. This smells like a few steps away from just
> > > abstracting the whole XML away from the targets and generating them
> > > inside gdbstub when we need them. As per my stalled attempt I referenced
> > > earlier.
> >
> > The abstraction is strictly limited for identifiers. Most plugin
> > should already have some knowledge of how registers are used. For
> > example, a plugin that tracks stack frame for RISC-V should know sp is
> > the stack pointer register. Similarly, a cycle simulator plugin should
> > know how registers are used in a program. Only identifiers matter in
> > such cases.
> >
> > I'm definitely *not* in favor of abstracting the whole XML for
> > plugins. It will be too hard to maintain ABI compatibility when a new
> > attribute emerges, for example.
>
> No I agree the XML shouldn't go near the plugins. I was just looking to
> avoid having an XML builder for every target.

Oh, I see. It's done in v4 with patch "gdbstub: Introduce 
GDBFeatureBuilder".

>