Re: [PATCH 3/4] target/riscv: Generate the GDB XML file for CSR registers dynamically
On Sat, Jan 16, 2021 at 6:08 AM Alistair Francis wrote: > > On Fri, Jan 15, 2021 at 1:59 PM Alistair Francis wrote: > > > > On Mon, Jan 11, 2021 at 8:55 PM Bin Meng wrote: > > > > > > From: Bin Meng > > > > > > 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 > Yes, for sure. Regards, Bin
Re: [PATCH 3/4] target/riscv: Generate the GDB XML file for CSR registers dynamically
On Fri, Jan 15, 2021 at 1:59 PM Alistair Francis wrote:
>
> On Mon, Jan 11, 2021 at 8:55 PM Bin Meng wrote:
> >
> > From: Bin Meng
> >
> > 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
>
> Reviewed-by: Alistair Francis
>
> 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,
> > -
Re: [PATCH 3/4] target/riscv: Generate the GDB XML file for CSR registers dynamically
On Mon, Jan 11, 2021 at 8:55 PM Bin Meng wrote:
>
> From: Bin Meng
>
> 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
Reviewed-by: Alistair Francis
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,
> -CS
