| Message ID | 20231103195956.1998255-14-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | gdbstub and plugin read register and windows support | expand |
Alex Bennée <alex.bennee@linaro.org> writes: > From: Akihiko Odaki <akihiko.odaki@daynix.com> > > 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> > Message-Id: <20231025093128.33116-7-akihiko.odaki@daynix.com> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> I bisected the following failure: ./qemu-riscv64 -g 1234 ./tests/tcg/riscv64-linux-user/sha512 and: gdb-multiarch ./tests/tcg/riscv64-linux-user/sha512 -ex "target remote localhost:1234" -x ../../tests/tcg/multiarch/gdbstub/registers.py gives: warning: Architecture rejected target-supplied description Ignoring packet error, continuing... Ignoring packet error, continuing... Ignoring packet error, continuing... Ignoring packet error, continuing... > --- > target/riscv/cpu.h | 5 +-- > target/riscv/cpu.c | 4 +-- > target/riscv/gdbstub.c | 79 +++++++++++++++++++----------------------- > 3 files changed, 40 insertions(+), 48 deletions(-) > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index f8ffa5ee38..73ec1d3b79 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" > @@ -395,8 +396,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 ac4a6c7eec..5200fba9b9 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -1421,9 +1421,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 524bede865..a3ac0212d1 100644 > --- a/target/riscv/gdbstub.c > +++ b/target/riscv/gdbstub.c > @@ -212,12 +212,13 @@ 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, int base_reg) > { > 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 << env->misa_mxl_max; > int i; > > @@ -230,9 +231,9 @@ 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", > + base_reg); > > for (i = 0; i < CSR_TABLE_SIZE; i++) { > if (env->priv_ver < csr_ops[i].min_priv_ver) { > @@ -240,72 +241,64 @@ 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, i, > + "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, int base_reg) > { > 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", > + base_reg); > > /* 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, i, "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) > @@ -320,10 +313,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, cs->gdb_num_regs)->num_regs, > "riscv-vector.xml", 0); > } > switch (env->misa_mxl_max) { > @@ -343,9 +335,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, cs->gdb_num_regs)->num_regs, > "riscv-csr.xml", 0); > } > }
Alex Bennée <alex.bennee@linaro.org> writes: > Alex Bennée <alex.bennee@linaro.org> writes: > >> From: Akihiko Odaki <akihiko.odaki@daynix.com> >> >> 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> >> Message-Id: <20231025093128.33116-7-akihiko.odaki@daynix.com> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > > I bisected the following failure: > > ./qemu-riscv64 -g 1234 ./tests/tcg/riscv64-linux-user/sha512 > > and: > > gdb-multiarch ./tests/tcg/riscv64-linux-user/sha512 -ex "target remote localhost:1234" -x ../../tests/tcg/multiarch/gdbstub/registers.py > > gives: > > warning: Architecture rejected target-supplied description > Ignoring packet error, continuing... > Ignoring packet error, continuing... > Ignoring packet error, continuing... > Ignoring packet error, continuing... I don't know if the discontinuous register numbering could be the problem: <feature name="org.gnu.gdb.riscv.csr"> <reg name="fflags" bitsize="64" type="int" regnum="67"/> <reg name="frm" bitsize="64" type="int" regnum="68"/> <reg name="fcsr" bitsize="64" type="int" regnum="69"/> <reg name="cycle" bitsize="64" type="int" regnum="3138"/> <reg name="time" bitsize="64" type="int" regnum="3139"/> <reg name="instret" bitsize="64" type="int" regnum="3140"/> </feature> ?
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index f8ffa5ee38..73ec1d3b79 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" @@ -395,8 +396,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 ac4a6c7eec..5200fba9b9 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -1421,9 +1421,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 524bede865..a3ac0212d1 100644 --- a/target/riscv/gdbstub.c +++ b/target/riscv/gdbstub.c @@ -212,12 +212,13 @@ 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, int base_reg) { 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 << env->misa_mxl_max; int i; @@ -230,9 +231,9 @@ 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", + base_reg); for (i = 0; i < CSR_TABLE_SIZE; i++) { if (env->priv_ver < csr_ops[i].min_priv_ver) { @@ -240,72 +241,64 @@ 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, i, + "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, int base_reg) { 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", + base_reg); /* 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, i, "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) @@ -320,10 +313,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, cs->gdb_num_regs)->num_regs, "riscv-vector.xml", 0); } switch (env->misa_mxl_max) { @@ -343,9 +335,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, cs->gdb_num_regs)->num_regs, "riscv-csr.xml", 0); } }