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

[Philippe Mathieu-Daudé]

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.od...@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é <phi...@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;