Re: [PATCH v3 2/3] target/riscv: Expose priv register for GDB for reads

[Jonathan Behrens]

On Tue, Oct 8, 2019 at 8:27 AM Bin Meng <address@hidden> wrote:
>
> On Tue, Oct 8, 2019 at 8:18 AM Jonathan Behrens <address@hidden> wrote:
> >
> > This patch enables a debugger to read the current privilege level via a 
> > virtual
> > "priv" register. When compiled with CONFIG_USER_ONLY the register is still
> > visible but always reports the value zero.
> >
> > Signed-off-by: Jonathan Behrens <address@hidden>
> > ---
> >  configure                       |  4 ++--
> >  gdb-xml/riscv-32bit-virtual.xml | 11 +++++++++++
> >  gdb-xml/riscv-64bit-virtual.xml | 11 +++++++++++
> >  target/riscv/gdbstub.c          | 23 +++++++++++++++++++++++
> >  4 files changed, 47 insertions(+), 2 deletions(-)
> >  create mode 100644 gdb-xml/riscv-32bit-virtual.xml
> >  create mode 100644 gdb-xml/riscv-64bit-virtual.xml
> >
> > diff --git a/configure b/configure
> > index 30544f52e6..6118a6a045 100755
> > --- a/configure
> > +++ b/configure
> > @@ -7520,13 +7520,13 @@ case "$target_name" in
> >      TARGET_BASE_ARCH=riscv
> >      TARGET_ABI_DIR=riscv
> >      mttcg=yes
> > -    gdb_xml_files="riscv-32bit-cpu.xml riscv-32bit-fpu.xml 
> > riscv-32bit-csr.xml"
> > +    gdb_xml_files="riscv-32bit-cpu.xml riscv-32bit-fpu.xml 
> > riscv-32bit-csr.xml riscv-32bit-virtual.xml"
> >    ;;
> >    riscv64)
> >      TARGET_BASE_ARCH=riscv
> >      TARGET_ABI_DIR=riscv
> >      mttcg=yes
> > -    gdb_xml_files="riscv-64bit-cpu.xml riscv-64bit-fpu.xml 
> > riscv-64bit-csr.xml"
> > +    gdb_xml_files="riscv-64bit-cpu.xml riscv-64bit-fpu.xml 
> > riscv-64bit-csr.xml riscv-64bit-virtual.xml"
> >    ;;
> >    sh4|sh4eb)
> >      TARGET_ARCH=sh4
> > diff --git a/gdb-xml/riscv-32bit-virtual.xml 
> > b/gdb-xml/riscv-32bit-virtual.xml
> > new file mode 100644
> > index 0000000000..905f1c555d
> > --- /dev/null
> > +++ b/gdb-xml/riscv-32bit-virtual.xml
> > @@ -0,0 +1,11 @@
> > +<?xml version="1.0"?>
> > +<!-- Copyright (C) 2018-2019 Free Software Foundation, Inc.
> > +
> > +     Copying and distribution of this file, with or without modification,
> > +     are permitted in any medium without royalty provided the copyright
> > +     notice and this notice are preserved.  -->
> > +
> > +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> > +<feature name="org.gnu.gdb.riscv.virtual">
> > +  <reg name="priv" bitsize="32"/>
> > +</feature>
> > diff --git a/gdb-xml/riscv-64bit-virtual.xml 
> > b/gdb-xml/riscv-64bit-virtual.xml
> > new file mode 100644
> > index 0000000000..62d86c237b
> > --- /dev/null
> > +++ b/gdb-xml/riscv-64bit-virtual.xml
> > @@ -0,0 +1,11 @@
> > +<?xml version="1.0"?>
> > +<!-- Copyright (C) 2018-2019 Free Software Foundation, Inc.
> > +
> > +     Copying and distribution of this file, with or without modification,
> > +     are permitted in any medium without royalty provided the copyright
> > +     notice and this notice are preserved.  -->
> > +
> > +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> > +<feature name="org.gnu.gdb.riscv.virtual">
> > +  <reg name="priv" bitsize="64"/>
> > +</feature>
> > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> > index cb5bfd3d50..33cf7c4c7d 100644
> > --- a/target/riscv/gdbstub.c
> > +++ b/target/riscv/gdbstub.c
> > @@ -373,6 +373,23 @@ static int riscv_gdb_set_csr(CPURISCVState *env, 
> > uint8_t *mem_buf, int n)
> >      return 0;
> >  }
> >
> > +static int riscv_gdb_get_virtual(CPURISCVState *cs, uint8_t *mem_buf, int 
> > n)
> > +{
> > +    if (n == 0) {
> > +#ifdef CONFIG_USER_ONLY
> > +        return gdb_get_regl(mem_buf, 0);
> > +#else
> > +        return gdb_get_regl(mem_buf, cs->priv);
> > +#endif
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int riscv_gdb_set_virtual(CPURISCVState *cs, uint8_t *mem_buf, int 
> > n)
> > +{
> > +    return 0;
> > +}
> > +
> >  void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
> >  {
> >      RISCVCPU *cpu = RISCV_CPU(cs);
> > @@ -385,6 +402,9 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState 
> > *cs)
> >
> >      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-csr.xml", 0);
>
> This should be riscv-32bit-virtual.xml

Good catch. I'll fix this in the next version.


>
> >  #elif defined(TARGET_RISCV64)
> >      if (env->misa & RVF) {
> >          gdb_register_coprocessor(cs, riscv_gdb_get_fpu, riscv_gdb_set_fpu,
> > @@ -393,5 +413,8 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState 
> > *cs)
> >
> >      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
> >  }
>
> Regards,
> Bin
>