On Wed, Oct 18, 2023 at 6:16 AM Akihiko Odaki <akihiko.od...@daynix.com> 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>
Acked-by: Alistair Francis <alistair.fran...@wdc.com>
Alistair
> ---
> target/riscv/cpu.h | 5 +--
> target/riscv/cpu.c | 4 +--
> target/riscv/gdbstub.c | 77 ++++++++++++++++++------------------------
> 3 files changed, 38 insertions(+), 48 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index ef10efd1e7..4a1683c8ad 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -24,6 +24,7 @@
> #include "hw/registerfields.h"
> #include "hw/qdev-properties.h"
> #include "exec/cpu-defs.h"
> +#include "exec/gdbstub.h"
> #include "qemu/cpu-float.h"
> #include "qom/object.h"
> #include "qemu/int128.h"
> @@ -394,8 +395,8 @@ struct ArchCPU {
>
> CPURISCVState env;
>
> - char *dyn_csr_xml;
> - char *dyn_vreg_xml;
> + GDBFeature dyn_csr_feature;
> + GDBFeature dyn_vreg_feature;
>
> /* Configuration Settings */
> RISCVCPUConfig cfg;
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 72124e57fd..5e083b5798 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1432,9 +1432,9 @@ static const char *riscv_gdb_get_dynamic_xml(CPUState
> *cs, const char *xmlname)
> RISCVCPU *cpu = RISCV_CPU(cs);
>
> if (strcmp(xmlname, "riscv-csr.xml") == 0) {
> - return cpu->dyn_csr_xml;
> + return cpu->dyn_csr_feature.xml;
> } else if (strcmp(xmlname, "riscv-vector.xml") == 0) {
> - return cpu->dyn_vreg_xml;
> + return cpu->dyn_vreg_feature.xml;
> }
>
> return NULL;
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index b9528cef5b..b7159f1db8 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -214,13 +214,14 @@ static int riscv_gdb_set_virtual(CPURISCVState *cs,
> uint8_t *mem_buf, int n)
> return 0;
> }
>
> -static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
> +static GDBFeature *riscv_gen_dynamic_csr_feature(CPUState *cs)
> {
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
> RISCVCPU *cpu = RISCV_CPU(cs);
> CPURISCVState *env = &cpu->env;
> - GString *s = g_string_new(NULL);
> + GDBFeatureBuilder builder;
> riscv_csr_predicate_fn predicate;
> + const char *name;
> int bitsize = 16 << mcc->misa_mxl_max;
> int i;
>
> @@ -233,9 +234,8 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int
> base_reg)
> bitsize = 64;
> }
>
> - g_string_printf(s, "<?xml version=\"1.0\"?>");
> - g_string_append_printf(s, "<!DOCTYPE feature SYSTEM
> \"gdb-target.dtd\">");
> - g_string_append_printf(s, "<feature name=\"org.gnu.gdb.riscv.csr\">");
> + gdb_feature_builder_init(&builder, &cpu->dyn_csr_feature,
> + "org.gnu.gdb.riscv.csr", "riscv-csr.xml");
>
> for (i = 0; i < CSR_TABLE_SIZE; i++) {
> if (env->priv_ver < csr_ops[i].min_priv_ver) {
> @@ -243,72 +243,63 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int
> base_reg)
> }
> predicate = csr_ops[i].predicate;
> if (predicate && (predicate(env, i) == RISCV_EXCP_NONE)) {
> - if (csr_ops[i].name) {
> - g_string_append_printf(s, "<reg name=\"%s\"",
> csr_ops[i].name);
> - } else {
> - g_string_append_printf(s, "<reg name=\"csr%03x\"", i);
> + g_autofree char *dynamic_name = NULL;
> + name = csr_ops[i].name;
> + if (!name) {
> + dynamic_name = g_strdup_printf("csr%03x", i);
> + name = dynamic_name;
> }
> - g_string_append_printf(s, " bitsize=\"%d\"", bitsize);
> - g_string_append_printf(s, " regnum=\"%d\"/>", base_reg + i);
> +
> + gdb_feature_builder_append_reg(&builder, name, bitsize,
> + "int", NULL);
> }
> }
>
> - g_string_append_printf(s, "</feature>");
> -
> - cpu->dyn_csr_xml = g_string_free(s, false);
> + gdb_feature_builder_end(&builder);
>
> #if !defined(CONFIG_USER_ONLY)
> env->debugger = false;
> #endif
>
> - return CSR_TABLE_SIZE;
> + return &cpu->dyn_csr_feature;
> }
>
> -static int ricsv_gen_dynamic_vector_xml(CPUState *cs, int base_reg)
> +static GDBFeature *ricsv_gen_dynamic_vector_feature(CPUState *cs)
> {
> RISCVCPU *cpu = RISCV_CPU(cs);
> - GString *s = g_string_new(NULL);
> - g_autoptr(GString) ts = g_string_new("");
> + GDBFeatureBuilder builder;
> int reg_width = cpu->cfg.vlen;
> - int num_regs = 0;
> int i;
>
> - 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.gnu.gdb.riscv.vector\">");
> + gdb_feature_builder_init(&builder, &cpu->dyn_vreg_feature,
> + "org.gnu.gdb.riscv.vector", "riscv-vector.xml");
>
> /* First define types and totals in a whole VL */
> for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
> int count = reg_width / vec_lanes[i].size;
> - g_string_printf(ts, "%s", vec_lanes[i].id);
> - g_string_append_printf(s,
> - "<vector id=\"%s\" type=\"%s\"
> count=\"%d\"/>",
> - ts->str, vec_lanes[i].gdb_type, count);
> + gdb_feature_builder_append_tag(
> + &builder, "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
> + vec_lanes[i].id, vec_lanes[i].gdb_type, count);
> }
>
> /* Define unions */
> - g_string_append_printf(s, "<union id=\"riscv_vector\">");
> + gdb_feature_builder_append_tag(&builder, "<union id=\"riscv_vector\">");
> for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
> - g_string_append_printf(s, "<field name=\"%c\" type=\"%s\"/>",
> - vec_lanes[i].suffix,
> - vec_lanes[i].id);
> + gdb_feature_builder_append_tag(&builder,
> + "<field name=\"%c\" type=\"%s\"/>",
> + vec_lanes[i].suffix, vec_lanes[i].id);
> }
> - g_string_append(s, "</union>");
> + gdb_feature_builder_append_tag(&builder, "</union>");
>
> /* Define vector registers */
> for (i = 0; i < 32; i++) {
> - g_string_append_printf(s,
> - "<reg name=\"v%d\" bitsize=\"%d\""
> - " regnum=\"%d\" group=\"vector\""
> - " type=\"riscv_vector\"/>",
> - i, reg_width, base_reg++);
> - num_regs++;
> + gdb_feature_builder_append_reg(&builder, g_strdup_printf("v%d", i),
> + reg_width, "riscv_vector", "vector");
> }
>
> - g_string_append_printf(s, "</feature>");
> + gdb_feature_builder_end(&builder);
>
> - cpu->dyn_vreg_xml = g_string_free(s, false);
> - return num_regs;
> + return &cpu->dyn_vreg_feature;
> }
>
> void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
> @@ -324,10 +315,9 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState
> *cs)
> 32, "riscv-32bit-fpu.xml", 0);
> }
> if (env->misa_ext & RVV) {
> - int base_reg = cs->gdb_num_regs;
> gdb_register_coprocessor(cs, riscv_gdb_get_vector,
> riscv_gdb_set_vector,
> - ricsv_gen_dynamic_vector_xml(cs, base_reg),
> +
> ricsv_gen_dynamic_vector_feature(cs)->num_regs,
> "riscv-vector.xml", 0);
> }
> switch (mcc->misa_mxl_max) {
> @@ -347,9 +337,8 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState
> *cs)
> }
>
> if (cpu->cfg.ext_icsr) {
> - int base_reg = cs->gdb_num_regs;
> gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
> - riscv_gen_dynamic_csr_xml(cs, base_reg),
> + riscv_gen_dynamic_csr_feature(cs)->num_regs,
> "riscv-csr.xml", 0);
> }
> }
> --
> 2.42.0
>
>
