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

[Jim Shu]

On Mon, Mar 6, 2023 at 7:26 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote:
>
>
> On 2023/3/5 17:42, Jim Shu wrote:
> > This patch also enables debugger to set current privilege mode to
> > VU/VS-mode.
> >
> > Extend previous commit 81d2929c41d32af138f3562f5a7b309f6eac7ca7 to
> > support H-extension.
> >
> > 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);
> We should return according to the misa_mxl_max. And this is a bug before
> your commit.

Hi Zhiwei,

After reading other gdbstub.c code, I think it is OK to use
'sizeof(target_ulong)' as virtual register length.
Its length is 32-bit in RV32 and is 64-bit in RV64/RV128. We don't
need to handle RV128 specially.
Virtual register length is same as CSR length and
'riscv_gdb_set_csr()' also use 'sizeof(target_ulong)'.

Jim Shu

> > +    } 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);
> Same error here. Otherwise,
>
> Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
>
> Zhiwei
>
> >       }
> > +#endif
> >       return 0;
> >   }
> >