Re: [RFC PATCH 07/24] target/arm: Use GDBFeature for dynamic XML

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

On 31/7/23 10:43, Akihiko Odaki wrote:
> In preparation for a change to use GDBFeature as a parameter of
> gdb_register_coprocessor(), convert the internal representation of
> dynamic feature from plain XML to GDBFeature.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   target/arm/cpu.h       | 20 +++++------
>   target/arm/internals.h |  2 +-
>   target/arm/gdbstub.c   | 80 +++++++++++++++++++++++-------------------
>   target/arm/gdbstub64.c | 11 +++---
>   4 files changed, 60 insertions(+), 53 deletions(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 88e5accda6..d6c2378d05 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -136,23 +136,21 @@ enum {
>    */
>   
>   /**
> - * DynamicGDBXMLInfo:
> - * @desc: Contains the XML descriptions.
> - * @num: Number of the registers in this XML seen by GDB.
> + * DynamicGDBFeatureInfo:
> + * @desc: Contains the feature descriptions.
>    * @data: A union with data specific to the set of registers
>    *    @cpregs_keys: Array that contains the corresponding Key of
>    *                  a given cpreg with the same order of the cpreg
>    *                  in the XML description.
>    */
> -typedef struct DynamicGDBXMLInfo {
> -    char *desc;
> -    int num;
> +typedef struct DynamicGDBFeatureInfo {
> +    GDBFeature desc;
>       union {
>           struct {
>               uint32_t *keys;
>           } cpregs;
>       } data;
> -} DynamicGDBXMLInfo;
> +} DynamicGDBFeatureInfo;
>   
>   /* CPU state for each instance of a generic timer (in cp15 c14) */
>   typedef struct ARMGenericTimer {
> @@ -881,10 +879,10 @@ struct ArchCPU {
>       uint64_t *cpreg_vmstate_values;
>       int32_t cpreg_vmstate_array_len;
>   
> -    DynamicGDBXMLInfo dyn_sysreg_xml;
> -    DynamicGDBXMLInfo dyn_svereg_xml;
> -    DynamicGDBXMLInfo dyn_m_systemreg_xml;
> -    DynamicGDBXMLInfo dyn_m_secextreg_xml;
> +    DynamicGDBFeatureInfo dyn_sysreg_feature;
> +    DynamicGDBFeatureInfo dyn_svereg_feature;
> +    DynamicGDBFeatureInfo dyn_m_systemreg_feature;
> +    DynamicGDBFeatureInfo dyn_m_secextreg_feature;

Since now DynamicGDBFeatureInfo.desc contains xmlname, we can replace
all these by a generic 'DynamicGDBFeatureInfo *dyn_features' and have
arm_gdb_get_dynamic_xml() looking up the xmlname.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> @@ -489,14 +497,14 @@ const char *arm_gdb_get_dynamic_xml(CPUState *cs, const char *xmlname)
>       ARMCPU *cpu = ARM_CPU(cs);
>   
>       if (strcmp(xmlname, "system-registers.xml") == 0) {
> -        return cpu->dyn_sysreg_xml.desc;
> +        return cpu->dyn_sysreg_feature.desc.xml;
>       } else if (strcmp(xmlname, "sve-registers.xml") == 0) {
> -        return cpu->dyn_svereg_xml.desc;
> +        return cpu->dyn_svereg_feature.desc.xml;
>       } else if (strcmp(xmlname, "arm-m-system.xml") == 0) {
> -        return cpu->dyn_m_systemreg_xml.desc;
> +        return cpu->dyn_m_systemreg_feature.desc.xml;
>   #ifndef CONFIG_USER_ONLY
>       } else if (strcmp(xmlname, "arm-m-secext.xml") == 0) {
> -        return cpu->dyn_m_secextreg_xml.desc;
> +        return cpu->dyn_m_secextreg_feature.desc.xml;
>   #endif
>       }
>       return NULL;