Re: [PATCH v2] target/riscv: Expose "priv" register for GDB

[Alistair Francis]

On Fri, Oct 4, 2019 at 8:18 AM Jonathan Behrens <address@hidden> wrote:
>
> This patch enables a debugger to read and write the current privilege level 
> via
> a special "priv" register. When compiled with CONFIG_USER_ONLY the register is
> still visible but is hardwired to zero.
>
> Signed-off-by: Jonathan Behrens <address@hidden>
> ---
>  gdb-xml/riscv-32bit-cpu.xml |  1 +
>  gdb-xml/riscv-64bit-cpu.xml |  1 +
>  target/riscv/cpu.c          |  2 +-
>  target/riscv/gdbstub.c      | 14 ++++++++++++++
>  4 files changed, 17 insertions(+), 1 deletion(-)
> ---
> Changelog V2:
> - Use PRV_H and PRV_S instead of integer literals
>
> diff --git a/gdb-xml/riscv-32bit-cpu.xml b/gdb-xml/riscv-32bit-cpu.xml
> index 0d07aaec85..d6d76aafd8 100644
> --- a/gdb-xml/riscv-32bit-cpu.xml
> +++ b/gdb-xml/riscv-32bit-cpu.xml
> @@ -44,4 +44,5 @@
>    <reg name="t5" bitsize="32" type="int"/>
>    <reg name="t6" bitsize="32" type="int"/>
>    <reg name="pc" bitsize="32" type="code_ptr"/>
> +  <reg name="priv" bitsize="32" type="int"/>
>  </feature>
> diff --git a/gdb-xml/riscv-64bit-cpu.xml b/gdb-xml/riscv-64bit-cpu.xml
> index b8aa424ae4..0758d1b5fe 100644
> --- a/gdb-xml/riscv-64bit-cpu.xml
> +++ b/gdb-xml/riscv-64bit-cpu.xml
> @@ -44,4 +44,5 @@
>    <reg name="t5" bitsize="64" type="int"/>
>    <reg name="t6" bitsize="64" type="int"/>
>    <reg name="pc" bitsize="64" type="code_ptr"/>
> +  <reg name="priv" bitsize="64" type="int" />
>  </feature>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f13e298a36..347989858f 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -475,7 +475,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void 
> *data)
>      cc->synchronize_from_tb = riscv_cpu_synchronize_from_tb;
>      cc->gdb_read_register = riscv_cpu_gdb_read_register;
>      cc->gdb_write_register = riscv_cpu_gdb_write_register;
> -    cc->gdb_num_core_regs = 33;
> +    cc->gdb_num_core_regs = 34;
>  #if defined(TARGET_RISCV32)
>      cc->gdb_core_xml_file = "riscv-32bit-cpu.xml";
>  #elif defined(TARGET_RISCV64)
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index ded140e8d8..7e0822145d 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -278,6 +278,12 @@ int riscv_cpu_gdb_read_register(CPUState *cs, uint8_t 
> *mem_buf, int n)
>          return gdb_get_regl(mem_buf, env->gpr[n]);
>      } else if (n == 32) {
>          return gdb_get_regl(mem_buf, env->pc);
> +    } else if (n == 33) {
> +#ifdef CONFIG_USER_ONLY
> +        return gdb_get_regl(mem_buf, 0);
> +#else
> +        return gdb_get_regl(mem_buf, env->priv);
> +#endif
>      }
>      return 0;
>  }
> @@ -296,6 +302,14 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t 
> *mem_buf, int n)
>      } else if (n == 32) {
>          env->pc = ldtul_p(mem_buf);
>          return sizeof(target_ulong);
> +    } else if (n == 33) {
> +#ifndef CONFIG_USER_ONLY
> +        env->priv = ldtul_p(mem_buf) & 0x3;
> +        if (env->priv == PRV_H) {
> +            env->priv = PRV_S;
> +        }

Why have this? There is no PRV_H so we should never be in that privilege mode.

Alistair

> +#endif
> +        return sizeof(target_ulong);
>      }
>      return 0;
>  }
> --
> 2.23.0
>