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;
/* Timers used by the generic (architected) timer */
QEMUTimer *gt_timer[NUM_GTIMERS];
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 0f01bc32a8..8421a755af 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1388,7 +1388,7 @@ static inline uint64_t pmu_counter_mask(CPUARMState *env)
}
#ifdef TARGET_AARCH64
-int arm_gen_dynamic_svereg_xml(CPUState *cpu, int base_reg);
+GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cpu, int base_reg);
int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg);
int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg);
int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg);
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index f421c5d041..cd35bac013 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -25,11 +25,11 @@
#include "internals.h"
#include "cpregs.h"
-typedef struct RegisterSysregXmlParam {
+typedef struct RegisterSysregFeatureParam {
CPUState *cs;
GString *s;
int n;
-} RegisterSysregXmlParam;
+} RegisterSysregFeatureParam;
/* Old gdb always expect FPA registers. Newer (xml-aware) gdb only expect
whatever the target description contains. Due to a historical mishap
@@ -243,7 +243,7 @@ static int arm_gdb_get_sysreg(CPUARMState *env, GByteArray *buf, int reg)
const ARMCPRegInfo *ri;
uint32_t key;
- key = cpu->dyn_sysreg_xml.data.cpregs.keys[reg];
+ key = cpu->dyn_sysreg_feature.data.cpregs.keys[reg];
ri = get_arm_cp_reginfo(cpu->cp_regs, key);
if (ri) {
if (cpreg_field_is_64bit(ri)) {
@@ -260,7 +260,8 @@ static int arm_gdb_set_sysreg(CPUARMState *env, uint8_t *buf, int reg)
return 0;
}
-static void arm_gen_one_xml_sysreg_tag(GString *s, DynamicGDBXMLInfo *dyn_xml,
+static void arm_gen_one_feature_sysreg(GString *s,
+ DynamicGDBFeatureInfo *dyn_feature,
ARMCPRegInfo *ri, uint32_t ri_key,
int bitsize, int regnum)
{
@@ -268,25 +269,25 @@ static void arm_gen_one_xml_sysreg_tag(GString *s, DynamicGDBXMLInfo *dyn_xml,
g_string_append_printf(s, " bitsize=\"%d\"", bitsize);
g_string_append_printf(s, " regnum=\"%d\"", regnum);
g_string_append_printf(s, " group=\"cp_regs\"/>");
- dyn_xml->data.cpregs.keys[dyn_xml->num] = ri_key;
- dyn_xml->num++;
+ dyn_feature->data.cpregs.keys[dyn_feature->desc.num_regs] = ri_key;
+ dyn_feature->desc.num_regs++;
}
-static void arm_register_sysreg_for_xml(gpointer key, gpointer value,
- gpointer p)
+static void arm_register_sysreg_for_feature(gpointer key, gpointer value,
+ gpointer p)
{
uint32_t ri_key = (uintptr_t)key;
ARMCPRegInfo *ri = value;
- RegisterSysregXmlParam *param = (RegisterSysregXmlParam *)p;
+ RegisterSysregFeatureParam *param = (RegisterSysregFeatureParam *)p;
GString *s = param->s;
ARMCPU *cpu = ARM_CPU(param->cs);
CPUARMState *env = &cpu->env;
- DynamicGDBXMLInfo *dyn_xml = &cpu->dyn_sysreg_xml;
+ DynamicGDBFeatureInfo *dyn_feature = &cpu->dyn_sysreg_feature;
if (!(ri->type & (ARM_CP_NO_RAW | ARM_CP_NO_GDB))) {
if (arm_feature(env, ARM_FEATURE_AARCH64)) {
if (ri->state == ARM_CP_STATE_AA64) {
- arm_gen_one_xml_sysreg_tag(s , dyn_xml, ri, ri_key, 64,
+ arm_gen_one_feature_sysreg(s , dyn_feature, ri, ri_key, 64,
param->n++);
}
} else {
@@ -296,10 +297,10 @@ static void arm_register_sysreg_for_xml(gpointer key, gpointer value,
return;
}
if (ri->type & ARM_CP_64BIT) {
- arm_gen_one_xml_sysreg_tag(s , dyn_xml, ri, ri_key, 64,
+ arm_gen_one_feature_sysreg(s , dyn_feature, ri, ri_key, 64,
param->n++);
} else {
- arm_gen_one_xml_sysreg_tag(s , dyn_xml, ri, ri_key, 32,
+ arm_gen_one_feature_sysreg(s , dyn_feature, ri, ri_key, 32,
param->n++);
}
}
@@ -307,21 +308,24 @@ static void arm_register_sysreg_for_xml(gpointer key, gpointer value,
}
}
-static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
+static GDBFeature *arm_gen_dynamic_sysreg_feature(CPUState *cs, int base_reg)
{
ARMCPU *cpu = ARM_CPU(cs);
GString *s = g_string_new(NULL);
- RegisterSysregXmlParam param = {cs, s, base_reg};
+ RegisterSysregFeatureParam param = {cs, s, base_reg};
+ DynamicGDBFeatureInfo *dyn_feature = &cpu->dyn_sysreg_feature;
+ gsize num_regs = g_hash_table_size(cpu->cp_regs);
- cpu->dyn_sysreg_xml.num = 0;
- cpu->dyn_sysreg_xml.data.cpregs.keys = g_new(uint32_t, g_hash_table_size(cpu->cp_regs));
+ dyn_feature->desc.num_regs = 0;
+ dyn_feature->data.cpregs.keys = g_new(uint32_t, num_regs);
g_string_printf(s, "<?xml version=\"1.0\"?>");
g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
g_string_append_printf(s, "<feature name=\"org.qemu.gdb.arm.sys.regs\">");
- g_hash_table_foreach(cpu->cp_regs, arm_register_sysreg_for_xml, ¶m);
+ g_hash_table_foreach(cpu->cp_regs, arm_register_sysreg_for_feature, ¶m);
g_string_append_printf(s, "</feature>");
- cpu->dyn_sysreg_xml.desc = g_string_free(s, false);
- return cpu->dyn_sysreg_xml.num;
+ dyn_feature->desc.xmlname = "system-registers.xml";
+ dyn_feature->desc.xml = g_string_free(s, false);
+ return &dyn_feature->desc;
}
#ifdef CONFIG_TCG
@@ -413,7 +417,8 @@ static int arm_gdb_set_m_systemreg(CPUARMState *env, uint8_t *buf, int reg)
return 0; /* TODO */
}
-static int arm_gen_dynamic_m_systemreg_xml(CPUState *cs, int orig_base_reg)
+static GDBFeature *arm_gen_dynamic_m_systemreg_feature(CPUState *cs,
+ int orig_base_reg)
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
@@ -434,10 +439,11 @@ static int arm_gen_dynamic_m_systemreg_xml(CPUState *cs, int orig_base_reg)
}
g_string_append_printf(s, "</feature>");
- cpu->dyn_m_systemreg_xml.desc = g_string_free(s, false);
- cpu->dyn_m_systemreg_xml.num = base_reg - orig_base_reg;
+ cpu->dyn_m_systemreg_feature.desc.xmlname = "arm-m-system.xml";
+ cpu->dyn_m_systemreg_feature.desc.xml = g_string_free(s, false);
+ cpu->dyn_m_systemreg_feature.desc.num_regs = base_reg - orig_base_reg;
- return cpu->dyn_m_systemreg_xml.num;
+ return &cpu->dyn_m_systemreg_feature.desc;
}
#ifndef CONFIG_USER_ONLY
@@ -455,7 +461,8 @@ static int arm_gdb_set_m_secextreg(CPUARMState *env, uint8_t *buf, int reg)
return 0; /* TODO */
}
-static int arm_gen_dynamic_m_secextreg_xml(CPUState *cs, int orig_base_reg)
+static GDBFeature *arm_gen_dynamic_m_secextreg_feature(CPUState *cs,
+ int orig_base_reg)
{
ARMCPU *cpu = ARM_CPU(cs);
GString *s = g_string_new(NULL);
@@ -476,10 +483,11 @@ static int arm_gen_dynamic_m_secextreg_xml(CPUState *cs, int orig_base_reg)
}
g_string_append_printf(s, "</feature>");
- cpu->dyn_m_secextreg_xml.desc = g_string_free(s, false);
- cpu->dyn_m_secextreg_xml.num = base_reg - orig_base_reg;
+ cpu->dyn_m_secextreg_feature.desc.xmlname = "arm-m-secext.xml";
+ cpu->dyn_m_secextreg_feature.desc.xml = g_string_free(s, false);
+ cpu->dyn_m_secextreg_feature.desc.num_regs = base_reg - orig_base_reg;
- return cpu->dyn_m_secextreg_xml.num;
+ return &cpu->dyn_m_secextreg_feature.desc;
}
#endif
#endif /* CONFIG_TCG */
@@ -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;
@@ -514,7 +522,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
*/
#ifdef TARGET_AARCH64
if (isar_feature_aa64_sve(&cpu->isar)) {
- int nreg = arm_gen_dynamic_svereg_xml(cs, cs->gdb_num_regs);
+ int nreg = arm_gen_dynamic_svereg_feature(cs, cs->gdb_num_regs)->num_regs;
gdb_register_coprocessor(cs, aarch64_gdb_get_sve_reg,
aarch64_gdb_set_sve_reg, nreg,
"sve-registers.xml", 0);
@@ -560,20 +568,20 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
1, "arm-m-profile-mve.xml", 0);
}
gdb_register_coprocessor(cs, arm_gdb_get_sysreg, arm_gdb_set_sysreg,
- arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs),
+ arm_gen_dynamic_sysreg_feature(cs, cs->gdb_num_regs)->num_regs,
"system-registers.xml", 0);
#ifdef CONFIG_TCG
if (arm_feature(env, ARM_FEATURE_M) && tcg_enabled()) {
gdb_register_coprocessor(cs,
arm_gdb_get_m_systemreg, arm_gdb_set_m_systemreg,
- arm_gen_dynamic_m_systemreg_xml(cs, cs->gdb_num_regs),
+ arm_gen_dynamic_m_systemreg_feature(cs, cs->gdb_num_regs)->num_regs,
"arm-m-system.xml", 0);
#ifndef CONFIG_USER_ONLY
if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
gdb_register_coprocessor(cs,
arm_gdb_get_m_secextreg, arm_gdb_set_m_secextreg,
- arm_gen_dynamic_m_secextreg_xml(cs, cs->gdb_num_regs),
+ arm_gen_dynamic_m_secextreg_feature(cs, cs->gdb_num_regs)->num_regs,
"arm-m-secext.xml", 0);
}
#endif
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index d7b79a6589..20483ef9bc 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -316,11 +316,11 @@ static void output_vector_union_type(GString *s, int reg_width,
g_string_append(s, "</union>");
}
-int arm_gen_dynamic_svereg_xml(CPUState *cs, int orig_base_reg)
+GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int orig_base_reg)
{
ARMCPU *cpu = ARM_CPU(cs);
GString *s = g_string_new(NULL);
- DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
+ DynamicGDBFeatureInfo *info = &cpu->dyn_svereg_feature;
int reg_width = cpu->sve_max_vq * 128;
int pred_width = cpu->sve_max_vq * 16;
int base_reg = orig_base_reg;
@@ -375,7 +375,8 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int orig_base_reg)
g_string_append_printf(s, "</feature>");
- info->desc = g_string_free(s, false);
- info->num = base_reg - orig_base_reg;
- return info->num;
+ info->desc.xmlname = "sve-registers.xml";
+ info->desc.xml = g_string_free(s, false);
+ info->desc.num_regs = base_reg - orig_base_reg;
+ return &info->desc;
}
--
2.41.0
Akihiko Odaki <akihiko.odaki@daynix.com> writes:
> 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.
FWIW one of the aims I had with my stalled rewrite of the register API
was to move all this XML generation into common code:
https://github.com/qemu/qemu/compare/master...stsquad:qemu:introspection/registers#diff-f6409265629976beb19cc9b8d96889b67c006a265586615f491e7d59dd83dc44R68
to avoid each of the targets having to mess with constructing their own
XML and just concentrate of the semantics of each register type.
>
> 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;
>
> /* Timers used by the generic (architected) timer */
> QEMUTimer *gt_timer[NUM_GTIMERS];
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 0f01bc32a8..8421a755af 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1388,7 +1388,7 @@ static inline uint64_t pmu_counter_mask(CPUARMState *env)
> }
>
> #ifdef TARGET_AARCH64
> -int arm_gen_dynamic_svereg_xml(CPUState *cpu, int base_reg);
> +GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cpu, int base_reg);
> int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg);
> int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg);
> int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg);
> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
> index f421c5d041..cd35bac013 100644
> --- a/target/arm/gdbstub.c
> +++ b/target/arm/gdbstub.c
> @@ -25,11 +25,11 @@
> #include "internals.h"
> #include "cpregs.h"
>
> -typedef struct RegisterSysregXmlParam {
> +typedef struct RegisterSysregFeatureParam {
> CPUState *cs;
> GString *s;
> int n;
> -} RegisterSysregXmlParam;
> +} RegisterSysregFeatureParam;
>
> /* Old gdb always expect FPA registers. Newer (xml-aware) gdb only expect
> whatever the target description contains. Due to a historical mishap
> @@ -243,7 +243,7 @@ static int arm_gdb_get_sysreg(CPUARMState *env, GByteArray *buf, int reg)
> const ARMCPRegInfo *ri;
> uint32_t key;
>
> - key = cpu->dyn_sysreg_xml.data.cpregs.keys[reg];
> + key = cpu->dyn_sysreg_feature.data.cpregs.keys[reg];
> ri = get_arm_cp_reginfo(cpu->cp_regs, key);
> if (ri) {
> if (cpreg_field_is_64bit(ri)) {
> @@ -260,7 +260,8 @@ static int arm_gdb_set_sysreg(CPUARMState *env, uint8_t *buf, int reg)
> return 0;
> }
>
> -static void arm_gen_one_xml_sysreg_tag(GString *s, DynamicGDBXMLInfo *dyn_xml,
> +static void arm_gen_one_feature_sysreg(GString *s,
> + DynamicGDBFeatureInfo *dyn_feature,
> ARMCPRegInfo *ri, uint32_t ri_key,
> int bitsize, int regnum)
> {
> @@ -268,25 +269,25 @@ static void arm_gen_one_xml_sysreg_tag(GString *s, DynamicGDBXMLInfo *dyn_xml,
> g_string_append_printf(s, " bitsize=\"%d\"", bitsize);
> g_string_append_printf(s, " regnum=\"%d\"", regnum);
> g_string_append_printf(s, " group=\"cp_regs\"/>");
> - dyn_xml->data.cpregs.keys[dyn_xml->num] = ri_key;
> - dyn_xml->num++;
> + dyn_feature->data.cpregs.keys[dyn_feature->desc.num_regs] = ri_key;
> + dyn_feature->desc.num_regs++;
> }
>
> -static void arm_register_sysreg_for_xml(gpointer key, gpointer value,
> - gpointer p)
> +static void arm_register_sysreg_for_feature(gpointer key, gpointer value,
> + gpointer p)
> {
> uint32_t ri_key = (uintptr_t)key;
> ARMCPRegInfo *ri = value;
> - RegisterSysregXmlParam *param = (RegisterSysregXmlParam *)p;
> + RegisterSysregFeatureParam *param = (RegisterSysregFeatureParam *)p;
> GString *s = param->s;
> ARMCPU *cpu = ARM_CPU(param->cs);
> CPUARMState *env = &cpu->env;
> - DynamicGDBXMLInfo *dyn_xml = &cpu->dyn_sysreg_xml;
> + DynamicGDBFeatureInfo *dyn_feature = &cpu->dyn_sysreg_feature;
>
> if (!(ri->type & (ARM_CP_NO_RAW | ARM_CP_NO_GDB))) {
> if (arm_feature(env, ARM_FEATURE_AARCH64)) {
> if (ri->state == ARM_CP_STATE_AA64) {
> - arm_gen_one_xml_sysreg_tag(s , dyn_xml, ri, ri_key, 64,
> + arm_gen_one_feature_sysreg(s , dyn_feature, ri, ri_key, 64,
> param->n++);
> }
> } else {
> @@ -296,10 +297,10 @@ static void arm_register_sysreg_for_xml(gpointer key, gpointer value,
> return;
> }
> if (ri->type & ARM_CP_64BIT) {
> - arm_gen_one_xml_sysreg_tag(s , dyn_xml, ri, ri_key, 64,
> + arm_gen_one_feature_sysreg(s , dyn_feature, ri, ri_key, 64,
> param->n++);
> } else {
> - arm_gen_one_xml_sysreg_tag(s , dyn_xml, ri, ri_key, 32,
> + arm_gen_one_feature_sysreg(s , dyn_feature, ri, ri_key, 32,
> param->n++);
> }
> }
> @@ -307,21 +308,24 @@ static void arm_register_sysreg_for_xml(gpointer key, gpointer value,
> }
> }
>
> -static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
> +static GDBFeature *arm_gen_dynamic_sysreg_feature(CPUState *cs, int base_reg)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> GString *s = g_string_new(NULL);
> - RegisterSysregXmlParam param = {cs, s, base_reg};
> + RegisterSysregFeatureParam param = {cs, s, base_reg};
> + DynamicGDBFeatureInfo *dyn_feature = &cpu->dyn_sysreg_feature;
> + gsize num_regs = g_hash_table_size(cpu->cp_regs);
>
> - cpu->dyn_sysreg_xml.num = 0;
> - cpu->dyn_sysreg_xml.data.cpregs.keys = g_new(uint32_t, g_hash_table_size(cpu->cp_regs));
> + dyn_feature->desc.num_regs = 0;
> + dyn_feature->data.cpregs.keys = g_new(uint32_t, num_regs);
> g_string_printf(s, "<?xml version=\"1.0\"?>");
> g_string_append_printf(s, "<!DOCTYPE target SYSTEM \"gdb-target.dtd\">");
> g_string_append_printf(s, "<feature name=\"org.qemu.gdb.arm.sys.regs\">");
> - g_hash_table_foreach(cpu->cp_regs, arm_register_sysreg_for_xml, ¶m);
> + g_hash_table_foreach(cpu->cp_regs, arm_register_sysreg_for_feature, ¶m);
> g_string_append_printf(s, "</feature>");
> - cpu->dyn_sysreg_xml.desc = g_string_free(s, false);
> - return cpu->dyn_sysreg_xml.num;
> + dyn_feature->desc.xmlname = "system-registers.xml";
> + dyn_feature->desc.xml = g_string_free(s, false);
> + return &dyn_feature->desc;
> }
>
> #ifdef CONFIG_TCG
> @@ -413,7 +417,8 @@ static int arm_gdb_set_m_systemreg(CPUARMState *env, uint8_t *buf, int reg)
> return 0; /* TODO */
> }
>
> -static int arm_gen_dynamic_m_systemreg_xml(CPUState *cs, int orig_base_reg)
> +static GDBFeature *arm_gen_dynamic_m_systemreg_feature(CPUState *cs,
> + int orig_base_reg)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> CPUARMState *env = &cpu->env;
> @@ -434,10 +439,11 @@ static int arm_gen_dynamic_m_systemreg_xml(CPUState *cs, int orig_base_reg)
> }
>
> g_string_append_printf(s, "</feature>");
> - cpu->dyn_m_systemreg_xml.desc = g_string_free(s, false);
> - cpu->dyn_m_systemreg_xml.num = base_reg - orig_base_reg;
> + cpu->dyn_m_systemreg_feature.desc.xmlname = "arm-m-system.xml";
> + cpu->dyn_m_systemreg_feature.desc.xml = g_string_free(s, false);
> + cpu->dyn_m_systemreg_feature.desc.num_regs = base_reg - orig_base_reg;
>
> - return cpu->dyn_m_systemreg_xml.num;
> + return &cpu->dyn_m_systemreg_feature.desc;
> }
>
> #ifndef CONFIG_USER_ONLY
> @@ -455,7 +461,8 @@ static int arm_gdb_set_m_secextreg(CPUARMState *env, uint8_t *buf, int reg)
> return 0; /* TODO */
> }
>
> -static int arm_gen_dynamic_m_secextreg_xml(CPUState *cs, int orig_base_reg)
> +static GDBFeature *arm_gen_dynamic_m_secextreg_feature(CPUState *cs,
> + int orig_base_reg)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> GString *s = g_string_new(NULL);
> @@ -476,10 +483,11 @@ static int arm_gen_dynamic_m_secextreg_xml(CPUState *cs, int orig_base_reg)
> }
>
> g_string_append_printf(s, "</feature>");
> - cpu->dyn_m_secextreg_xml.desc = g_string_free(s, false);
> - cpu->dyn_m_secextreg_xml.num = base_reg - orig_base_reg;
> + cpu->dyn_m_secextreg_feature.desc.xmlname = "arm-m-secext.xml";
> + cpu->dyn_m_secextreg_feature.desc.xml = g_string_free(s, false);
> + cpu->dyn_m_secextreg_feature.desc.num_regs = base_reg - orig_base_reg;
>
> - return cpu->dyn_m_secextreg_xml.num;
> + return &cpu->dyn_m_secextreg_feature.desc;
> }
> #endif
> #endif /* CONFIG_TCG */
> @@ -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;
> @@ -514,7 +522,7 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
> */
> #ifdef TARGET_AARCH64
> if (isar_feature_aa64_sve(&cpu->isar)) {
> - int nreg = arm_gen_dynamic_svereg_xml(cs, cs->gdb_num_regs);
> + int nreg = arm_gen_dynamic_svereg_feature(cs, cs->gdb_num_regs)->num_regs;
> gdb_register_coprocessor(cs, aarch64_gdb_get_sve_reg,
> aarch64_gdb_set_sve_reg, nreg,
> "sve-registers.xml", 0);
> @@ -560,20 +568,20 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
> 1, "arm-m-profile-mve.xml", 0);
> }
> gdb_register_coprocessor(cs, arm_gdb_get_sysreg, arm_gdb_set_sysreg,
> - arm_gen_dynamic_sysreg_xml(cs, cs->gdb_num_regs),
> + arm_gen_dynamic_sysreg_feature(cs, cs->gdb_num_regs)->num_regs,
> "system-registers.xml", 0);
>
> #ifdef CONFIG_TCG
> if (arm_feature(env, ARM_FEATURE_M) && tcg_enabled()) {
> gdb_register_coprocessor(cs,
> arm_gdb_get_m_systemreg, arm_gdb_set_m_systemreg,
> - arm_gen_dynamic_m_systemreg_xml(cs, cs->gdb_num_regs),
> + arm_gen_dynamic_m_systemreg_feature(cs, cs->gdb_num_regs)->num_regs,
> "arm-m-system.xml", 0);
> #ifndef CONFIG_USER_ONLY
> if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
> gdb_register_coprocessor(cs,
> arm_gdb_get_m_secextreg, arm_gdb_set_m_secextreg,
> - arm_gen_dynamic_m_secextreg_xml(cs, cs->gdb_num_regs),
> + arm_gen_dynamic_m_secextreg_feature(cs, cs->gdb_num_regs)->num_regs,
> "arm-m-secext.xml", 0);
> }
> #endif
> diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
> index d7b79a6589..20483ef9bc 100644
> --- a/target/arm/gdbstub64.c
> +++ b/target/arm/gdbstub64.c
> @@ -316,11 +316,11 @@ static void output_vector_union_type(GString *s, int reg_width,
> g_string_append(s, "</union>");
> }
>
> -int arm_gen_dynamic_svereg_xml(CPUState *cs, int orig_base_reg)
> +GDBFeature *arm_gen_dynamic_svereg_feature(CPUState *cs, int orig_base_reg)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> GString *s = g_string_new(NULL);
> - DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
> + DynamicGDBFeatureInfo *info = &cpu->dyn_svereg_feature;
> int reg_width = cpu->sve_max_vq * 128;
> int pred_width = cpu->sve_max_vq * 16;
> int base_reg = orig_base_reg;
> @@ -375,7 +375,8 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int orig_base_reg)
>
> g_string_append_printf(s, "</feature>");
>
> - info->desc = g_string_free(s, false);
> - info->num = base_reg - orig_base_reg;
> - return info->num;
> + info->desc.xmlname = "sve-registers.xml";
> + info->desc.xml = g_string_free(s, false);
> + info->desc.num_regs = base_reg - orig_base_reg;
> + return &info->desc;
> }
Otherwise:
Acked-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
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;
On 2023/07/31 22:44, Philippe Mathieu-Daudé wrote:
> 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.
In patch 12, cpu-specific gdb_get_dynamic_xml() function is removed, and
gdbstub instead looks up an internal list it holds.
>
> 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;
>
© 2016 - 2026 Red Hat, Inc.