On Mon, Jan 11, 2021 at 8:55 PM Bin Meng <bmeng...@gmail.com> wrote:
>
> From: Bin Meng <bin.m...@windriver.com>
>
> At present QEMU RISC-V uses a hardcoded XML to report the feature
> "org.gnu.gdb.riscv.csr" [1]. There are two major issues with the
> approach being used currently:
>
> - The XML does not specify the "regnum" field of a CSR entry, hence
> consecutive numbers are used by the remote GDB client to access
> CSRs. In QEMU we have to maintain a map table to convert the GDB
> number to the hardware number which is error prone.
> - The XML contains some CSRs that QEMU does not implement at all,
> which causes an "E14" response sent to remote GDB client.
>
> Change to generate the CSR register list dynamically, based on the
> availability presented in the CSR function table. This new approach
> will reflect a correct list of CSRs that QEMU actually implements.
>
> [1]
> https://sourceware.org/gdb/current/onlinedocs/gdb/RISC_002dV-Features.html#RISC_002dV-Features
>
> Signed-off-by: Bin Meng <bin.m...@windriver.com>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
Alistair
> ---
>
> target/riscv/cpu.h | 2 +
> target/riscv/cpu.c | 12 ++
> target/riscv/gdbstub.c | 308
> +++++++------------------------------------------
> 3 files changed, 58 insertions(+), 264 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 6684316..f810169 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -272,6 +272,8 @@ struct RISCVCPU {
> CPUNegativeOffsetState neg;
> CPURISCVState env;
>
> + char *dyn_csr_xml;
> +
> /* Configuration Settings */
> struct {
> bool ext_i;
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index dfe5d4e..c0dd646 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -569,6 +569,17 @@ static gchar *riscv_gdb_arch_name(CPUState *cs)
> }
> }
>
> +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 NULL;
> +}
> +
> static void riscv_cpu_class_init(ObjectClass *c, void *data)
> {
> RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
> @@ -607,6 +618,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void
> *data)
> cc->write_elf32_note = riscv_cpu_write_elf32_note;
> #endif
> cc->gdb_arch_name = riscv_gdb_arch_name;
> + cc->gdb_get_dynamic_xml = riscv_gdb_get_dynamic_xml;
> #ifdef CONFIG_TCG
> cc->tcg_initialize = riscv_translate_init;
> cc->tlb_fill = riscv_cpu_tlb_fill;
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index eba12a8..5f96b7e 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -20,256 +20,6 @@
> #include "exec/gdbstub.h"
> #include "cpu.h"
>
> -/*
> - * The GDB CSR xml files list them in documentation order, not numerical
> order,
> - * and are missing entries for unnamed CSRs. So we need to map the gdb
> numbers
> - * to the hardware numbers.
> - */
> -
> -static int csr_register_map[] = {
> - CSR_USTATUS,
> - CSR_UIE,
> - CSR_UTVEC,
> - CSR_USCRATCH,
> - CSR_UEPC,
> - CSR_UCAUSE,
> - CSR_UTVAL,
> - CSR_UIP,
> - CSR_FFLAGS,
> - CSR_FRM,
> - CSR_FCSR,
> - CSR_CYCLE,
> - CSR_TIME,
> - CSR_INSTRET,
> - CSR_HPMCOUNTER3,
> - CSR_HPMCOUNTER4,
> - CSR_HPMCOUNTER5,
> - CSR_HPMCOUNTER6,
> - CSR_HPMCOUNTER7,
> - CSR_HPMCOUNTER8,
> - CSR_HPMCOUNTER9,
> - CSR_HPMCOUNTER10,
> - CSR_HPMCOUNTER11,
> - CSR_HPMCOUNTER12,
> - CSR_HPMCOUNTER13,
> - CSR_HPMCOUNTER14,
> - CSR_HPMCOUNTER15,
> - CSR_HPMCOUNTER16,
> - CSR_HPMCOUNTER17,
> - CSR_HPMCOUNTER18,
> - CSR_HPMCOUNTER19,
> - CSR_HPMCOUNTER20,
> - CSR_HPMCOUNTER21,
> - CSR_HPMCOUNTER22,
> - CSR_HPMCOUNTER23,
> - CSR_HPMCOUNTER24,
> - CSR_HPMCOUNTER25,
> - CSR_HPMCOUNTER26,
> - CSR_HPMCOUNTER27,
> - CSR_HPMCOUNTER28,
> - CSR_HPMCOUNTER29,
> - CSR_HPMCOUNTER30,
> - CSR_HPMCOUNTER31,
> - CSR_CYCLEH,
> - CSR_TIMEH,
> - CSR_INSTRETH,
> - CSR_HPMCOUNTER3H,
> - CSR_HPMCOUNTER4H,
> - CSR_HPMCOUNTER5H,
> - CSR_HPMCOUNTER6H,
> - CSR_HPMCOUNTER7H,
> - CSR_HPMCOUNTER8H,
> - CSR_HPMCOUNTER9H,
> - CSR_HPMCOUNTER10H,
> - CSR_HPMCOUNTER11H,
> - CSR_HPMCOUNTER12H,
> - CSR_HPMCOUNTER13H,
> - CSR_HPMCOUNTER14H,
> - CSR_HPMCOUNTER15H,
> - CSR_HPMCOUNTER16H,
> - CSR_HPMCOUNTER17H,
> - CSR_HPMCOUNTER18H,
> - CSR_HPMCOUNTER19H,
> - CSR_HPMCOUNTER20H,
> - CSR_HPMCOUNTER21H,
> - CSR_HPMCOUNTER22H,
> - CSR_HPMCOUNTER23H,
> - CSR_HPMCOUNTER24H,
> - CSR_HPMCOUNTER25H,
> - CSR_HPMCOUNTER26H,
> - CSR_HPMCOUNTER27H,
> - CSR_HPMCOUNTER28H,
> - CSR_HPMCOUNTER29H,
> - CSR_HPMCOUNTER30H,
> - CSR_HPMCOUNTER31H,
> - CSR_SSTATUS,
> - CSR_SEDELEG,
> - CSR_SIDELEG,
> - CSR_SIE,
> - CSR_STVEC,
> - CSR_SCOUNTEREN,
> - CSR_SSCRATCH,
> - CSR_SEPC,
> - CSR_SCAUSE,
> - CSR_STVAL,
> - CSR_SIP,
> - CSR_SATP,
> - CSR_MVENDORID,
> - CSR_MARCHID,
> - CSR_MIMPID,
> - CSR_MHARTID,
> - CSR_MSTATUS,
> - CSR_MISA,
> - CSR_MEDELEG,
> - CSR_MIDELEG,
> - CSR_MIE,
> - CSR_MTVEC,
> - CSR_MCOUNTEREN,
> - CSR_MSCRATCH,
> - CSR_MEPC,
> - CSR_MCAUSE,
> - CSR_MTVAL,
> - CSR_MIP,
> - CSR_MTINST,
> - CSR_MTVAL2,
> - CSR_PMPCFG0,
> - CSR_PMPCFG1,
> - CSR_PMPCFG2,
> - CSR_PMPCFG3,
> - CSR_PMPADDR0,
> - CSR_PMPADDR1,
> - CSR_PMPADDR2,
> - CSR_PMPADDR3,
> - CSR_PMPADDR4,
> - CSR_PMPADDR5,
> - CSR_PMPADDR6,
> - CSR_PMPADDR7,
> - CSR_PMPADDR8,
> - CSR_PMPADDR9,
> - CSR_PMPADDR10,
> - CSR_PMPADDR11,
> - CSR_PMPADDR12,
> - CSR_PMPADDR13,
> - CSR_PMPADDR14,
> - CSR_PMPADDR15,
> - CSR_MCYCLE,
> - CSR_MINSTRET,
> - CSR_MHPMCOUNTER3,
> - CSR_MHPMCOUNTER4,
> - CSR_MHPMCOUNTER5,
> - CSR_MHPMCOUNTER6,
> - CSR_MHPMCOUNTER7,
> - CSR_MHPMCOUNTER8,
> - CSR_MHPMCOUNTER9,
> - CSR_MHPMCOUNTER10,
> - CSR_MHPMCOUNTER11,
> - CSR_MHPMCOUNTER12,
> - CSR_MHPMCOUNTER13,
> - CSR_MHPMCOUNTER14,
> - CSR_MHPMCOUNTER15,
> - CSR_MHPMCOUNTER16,
> - CSR_MHPMCOUNTER17,
> - CSR_MHPMCOUNTER18,
> - CSR_MHPMCOUNTER19,
> - CSR_MHPMCOUNTER20,
> - CSR_MHPMCOUNTER21,
> - CSR_MHPMCOUNTER22,
> - CSR_MHPMCOUNTER23,
> - CSR_MHPMCOUNTER24,
> - CSR_MHPMCOUNTER25,
> - CSR_MHPMCOUNTER26,
> - CSR_MHPMCOUNTER27,
> - CSR_MHPMCOUNTER28,
> - CSR_MHPMCOUNTER29,
> - CSR_MHPMCOUNTER30,
> - CSR_MHPMCOUNTER31,
> - CSR_MCYCLEH,
> - CSR_MINSTRETH,
> - CSR_MHPMCOUNTER3H,
> - CSR_MHPMCOUNTER4H,
> - CSR_MHPMCOUNTER5H,
> - CSR_MHPMCOUNTER6H,
> - CSR_MHPMCOUNTER7H,
> - CSR_MHPMCOUNTER8H,
> - CSR_MHPMCOUNTER9H,
> - CSR_MHPMCOUNTER10H,
> - CSR_MHPMCOUNTER11H,
> - CSR_MHPMCOUNTER12H,
> - CSR_MHPMCOUNTER13H,
> - CSR_MHPMCOUNTER14H,
> - CSR_MHPMCOUNTER15H,
> - CSR_MHPMCOUNTER16H,
> - CSR_MHPMCOUNTER17H,
> - CSR_MHPMCOUNTER18H,
> - CSR_MHPMCOUNTER19H,
> - CSR_MHPMCOUNTER20H,
> - CSR_MHPMCOUNTER21H,
> - CSR_MHPMCOUNTER22H,
> - CSR_MHPMCOUNTER23H,
> - CSR_MHPMCOUNTER24H,
> - CSR_MHPMCOUNTER25H,
> - CSR_MHPMCOUNTER26H,
> - CSR_MHPMCOUNTER27H,
> - CSR_MHPMCOUNTER28H,
> - CSR_MHPMCOUNTER29H,
> - CSR_MHPMCOUNTER30H,
> - CSR_MHPMCOUNTER31H,
> - CSR_MHPMEVENT3,
> - CSR_MHPMEVENT4,
> - CSR_MHPMEVENT5,
> - CSR_MHPMEVENT6,
> - CSR_MHPMEVENT7,
> - CSR_MHPMEVENT8,
> - CSR_MHPMEVENT9,
> - CSR_MHPMEVENT10,
> - CSR_MHPMEVENT11,
> - CSR_MHPMEVENT12,
> - CSR_MHPMEVENT13,
> - CSR_MHPMEVENT14,
> - CSR_MHPMEVENT15,
> - CSR_MHPMEVENT16,
> - CSR_MHPMEVENT17,
> - CSR_MHPMEVENT18,
> - CSR_MHPMEVENT19,
> - CSR_MHPMEVENT20,
> - CSR_MHPMEVENT21,
> - CSR_MHPMEVENT22,
> - CSR_MHPMEVENT23,
> - CSR_MHPMEVENT24,
> - CSR_MHPMEVENT25,
> - CSR_MHPMEVENT26,
> - CSR_MHPMEVENT27,
> - CSR_MHPMEVENT28,
> - CSR_MHPMEVENT29,
> - CSR_MHPMEVENT30,
> - CSR_MHPMEVENT31,
> - CSR_TSELECT,
> - CSR_TDATA1,
> - CSR_TDATA2,
> - CSR_TDATA3,
> - CSR_DCSR,
> - CSR_DPC,
> - CSR_DSCRATCH,
> - CSR_HSTATUS,
> - CSR_HEDELEG,
> - CSR_HIDELEG,
> - CSR_HIE,
> - CSR_HCOUNTEREN,
> - CSR_HTVAL,
> - CSR_HIP,
> - CSR_HTINST,
> - CSR_HGATP,
> - CSR_MBASE,
> - CSR_MBOUND,
> - CSR_MIBASE,
> - CSR_MIBOUND,
> - CSR_MDBASE,
> - CSR_MDBOUND,
> - CSR_MUCOUNTEREN,
> - CSR_MSCOUNTEREN,
> - CSR_MHCOUNTEREN,
> -};
> -
> int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
> {
> RISCVCPU *cpu = RISCV_CPU(cs);
> @@ -315,11 +65,11 @@ static int riscv_gdb_get_fpu(CPURISCVState *env,
> GByteArray *buf, int n)
> target_ulong val = 0;
> int result;
> /*
> - * CSR_FFLAGS is at index 8 in csr_register, and gdb says it is FP
> + * CSR_FFLAGS is at index 1 in csr_register, and gdb says it is FP
> * register 33, so we recalculate the map index.
> * This also works for CSR_FRM and CSR_FCSR.
> */
> - result = riscv_csrrw_debug(env, n - 33 + csr_register_map[8], &val,
> + result = riscv_csrrw_debug(env, n - 32, &val,
> 0, 0);
> if (result == 0) {
> return gdb_get_regl(buf, val);
> @@ -338,11 +88,11 @@ static int riscv_gdb_set_fpu(CPURISCVState *env, uint8_t
> *mem_buf, int n)
> target_ulong val = ldtul_p(mem_buf);
> int result;
> /*
> - * CSR_FFLAGS is at index 8 in csr_register, and gdb says it is FP
> + * CSR_FFLAGS is at index 1 in csr_register, and gdb says it is FP
> * register 33, so we recalculate the map index.
> * This also works for CSR_FRM and CSR_FCSR.
> */
> - result = riscv_csrrw_debug(env, n - 33 + csr_register_map[8], NULL,
> + result = riscv_csrrw_debug(env, n - 32, NULL,
> val, -1);
> if (result == 0) {
> return sizeof(target_ulong);
> @@ -353,11 +103,11 @@ static int riscv_gdb_set_fpu(CPURISCVState *env,
> uint8_t *mem_buf, int n)
>
> static int riscv_gdb_get_csr(CPURISCVState *env, GByteArray *buf, int n)
> {
> - if (n < ARRAY_SIZE(csr_register_map)) {
> + if (n < CSR_TABLE_SIZE) {
> target_ulong val = 0;
> int result;
>
> - result = riscv_csrrw_debug(env, csr_register_map[n], &val, 0, 0);
> + result = riscv_csrrw_debug(env, n, &val, 0, 0);
> if (result == 0) {
> return gdb_get_regl(buf, val);
> }
> @@ -367,11 +117,11 @@ static int riscv_gdb_get_csr(CPURISCVState *env,
> GByteArray *buf, int n)
>
> static int riscv_gdb_set_csr(CPURISCVState *env, uint8_t *mem_buf, int n)
> {
> - if (n < ARRAY_SIZE(csr_register_map)) {
> + if (n < CSR_TABLE_SIZE) {
> target_ulong val = ldtul_p(mem_buf);
> int result;
>
> - result = riscv_csrrw_debug(env, csr_register_map[n], NULL, val, -1);
> + result = riscv_csrrw_debug(env, n, NULL, val, -1);
> if (result == 0) {
> return sizeof(target_ulong);
> }
> @@ -405,6 +155,38 @@ 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)
> +{
> + RISCVCPU *cpu = RISCV_CPU(cs);
> + CPURISCVState *env = &cpu->env;
> + GString *s = g_string_new(NULL);
> + riscv_csr_predicate_fn predicate;
> + int bitsize = riscv_cpu_is_32bit(env) ? 32 : 64;
> + int i;
> +
> + 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\">");
> +
> + for (i = 0; i < CSR_TABLE_SIZE; i++) {
> + predicate = csr_ops[i].predicate;
> + if (predicate && !predicate(env, i)) {
> + 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_string_append_printf(s, " bitsize=\"%d\"", bitsize);
> + g_string_append_printf(s, " regnum=\"%d\"/>", base_reg + i);
> + }
> + }
> +
> + g_string_append_printf(s, "</feature>");
> +
> + cpu->dyn_csr_xml = g_string_free(s, false);
> + return CSR_TABLE_SIZE;
> +}
> +
> void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
> {
> RISCVCPU *cpu = RISCV_CPU(cs);
> @@ -417,16 +199,14 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState
> *cs)
> 36, "riscv-32bit-fpu.xml", 0);
> }
> #if defined(TARGET_RISCV32)
> - gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
> - 240, "riscv-32bit-csr.xml", 0);
> -
> gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
> riscv_gdb_set_virtual,
> 1, "riscv-32bit-virtual.xml", 0);
> #elif defined(TARGET_RISCV64)
> - gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
> - 240, "riscv-64bit-csr.xml", 0);
> -
> gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
> riscv_gdb_set_virtual,
> 1, "riscv-64bit-virtual.xml", 0);
> #endif
> +
> + gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
> + riscv_gen_dynamic_csr_xml(cs, cs->gdb_num_regs),
> + "riscv-csr.xml", 0);
> }
> --
> 2.7.4
>
>
