Re: [PATCH 2/2] target/riscv: Make the "virt" register writable by GDB

[Alistair Francis]

On Sun, Mar 5, 2023 at 7:43 PM Jim Shu <jim.shu@sifive.com> wrote:
>
> This patch also enables debugger to set current privilege mode to
> VU/VS-mode.
>
> Extend previous commit 81d2929c41d32af138f3562f5a7b309f6eac7ca7 to
> support H-extension.

I'm not sure we want this. What is the use case for this?

Changing the virt mode on the fly like this is likely to break lots of
software. Should we really allow users to do this?

Alistair

>
> Signed-off-by: Jim Shu <jim.shu@sifive.com>
> Reviewed-by: Frank Chang <frank.chang@sifive.com>
> ---
>  target/riscv/gdbstub.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 1755fd9d51..a7f234beaf 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -203,15 +203,29 @@ static int riscv_gdb_get_virtual(CPURISCVState *cs, 
> GByteArray *buf, int n)
>
>  static int riscv_gdb_set_virtual(CPURISCVState *cs, uint8_t *mem_buf, int n)
>  {
> +#ifdef CONFIG_USER_ONLY
> +    if (n >= 0 && n <= 1) {
> +        return sizeof(target_ulong);
> +    }
> +#else
> +    bool virt;
> +
>      if (n == 0) {
> -#ifndef CONFIG_USER_ONLY
>          cs->priv = ldtul_p(mem_buf) & 0x3;
>          if (cs->priv == PRV_H) {
>              cs->priv = PRV_S;
>          }
> -#endif
> +        return sizeof(target_ulong);
> +    } else if (n == 1) {
> +        virt = ldtul_p(mem_buf) & 0x1;
> +        if ((cs->priv == PRV_M) && (virt == true)) {
> +            /* M-mode only supports V=0. */
> +            virt = false;
> +        }
> +        riscv_cpu_set_virt_enabled(cs, virt);
>          return sizeof(target_ulong);
>      }
> +#endif
>      return 0;
>  }
>
> --
> 2.17.1
>
>