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

[Alex Bennée]

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.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro