[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/4] target/riscv: Generate the GDB XML file for CSR register
|
From: |
Alistair Francis |
|
Subject: |
Re: [PATCH 3/4] target/riscv: Generate the GDB XML file for CSR registers dynamically |
|
Date: |
Fri, 15 Jan 2021 14:07:53 -0800 |
On Fri, Jan 15, 2021 at 1:59 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Mon, Jan 11, 2021 at 8:55 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@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
Do you mind rebasing this patch on the current riscv-to-apply.next
branch: https://github.com/alistair23/qemu/tree/riscv-to-apply.next
Alistair
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@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
> >
> >