Message ID | 20230731084354.115015-8-akihiko.odaki@daynix.com |
---|---|
State | New |
Headers | show |
Series | plugins: Allow to read registers | expand |
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; >
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>
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; }
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(-)