Re: [PATCH] include virtualization mode as part of priv

[Yanfeng]

On Thu, 2024-11-28 at 10:39 +1000, Alistair Francis wrote:
> On Thu, Nov 28, 2024 at 12:09 AM Yanfeng <yfliu2008@qq.com> wrote:
> > 
> > 
> > When debugging hypervisor extension based programs, it is convenient to see
> > the
> > current virtualization mode from GDB debugger.
> > 
> > This patch shares the virtualization mode as part of the existing "priv"
> > virtual
> > register, or more specifically via bit(8).
> 
> Interesting concept. This seems fine to me, I don't think this will
> break any existing user.
> 
> Another option though is to add a "virt" virtual register, which might
> be easier for people to figure out as it isn't obvious from GDB what
> bit 8 means. That might be a better approach as then it's really clear
> what the register means.
> 
> > 
> > 
> > > From 0d82561b11e1c2835f14ba5460cfff52f0087530 Mon Sep 17 00:00:00 2001
> > From: Yanfeng Liu <yfliu2008@qq.com>
> > Date: Mon, 18 Nov 2024 08:03:15 +0800
> > Subject: [PATCH] riscv/gdb: share virt mode via priv register
> 
> It seems like something strange happened when you submitted the patch.
> Can you fix this up?
> 
I prepared a patch file via the following steps:

 - Did  `git format-patch --stdout -1 > /tmp/patch`, 
 - Pasted the /tmp/patch content to email composer window, 
 - Filled in email receipts, subject line and a few lines before the patch
context in composer window.

I am wondering if the lines added before the formatted patch content in last
step caused trouble?
When resending email, should I use [patch v2] in subject line? I guess it is
unnecessary as it is the same patch. 

I included it as attachment here as well, hoping it helps.

Regards,
yf
 
> Alistair
> 
> > 
> > This shares virtualization mode together with privilege mode
> > via the `priv` virtual register over the debug interface.
> > 
> > Check logs with gdb-multiarch 12.1:
> > 
> > ```
> > (gdb) info registers priv
> > priv           0x101    prv:1 [Supervisor]
> > (gdb) set $priv = 1
> > (gdb) info registers priv
> > priv           0x1      prv:1 [Supervisor]
> > (gdb) set $priv = 0x101
> > (gdb) info registers priv
> > priv           0x101    prv:1 [Supervisor]
> > (gdb)
> > ```
> > 
> > Signed-off-by: Yanfeng Liu <yfliu2008@qq.com>
> > ---
> >  target/riscv/cpu_bits.h |  4 ++++
> >  target/riscv/gdbstub.c  | 15 +++++++++++++--
> >  2 files changed, 17 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index 385a2c67c2..cc6dece51a 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -623,6 +623,10 @@ typedef enum {
> >  #define PRV_RESERVED 2
> >  #define PRV_M 3
> > 
> > +/* Share virtualization mode as part of priv register */
> > +#define PRV_V                (1 << 8)
> > +
> > +
> >  /* RV32 satp CSR field masks */
> >  #define SATP32_MODE         0x80000000
> >  #define SATP32_ASID         0x7fc00000
> > diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> > index c07df972f1..d9e6ad969a 100644
> > --- a/target/riscv/gdbstub.c
> > +++ b/target/riscv/gdbstub.c
> > @@ -212,8 +212,14 @@ static int riscv_gdb_get_virtual(CPUState *cs,
> > GByteArray
> > *buf, int n)
> >  #else
> >          RISCVCPU *cpu = RISCV_CPU(cs);
> >          CPURISCVState *env = &cpu->env;
> > +        target_ulong ret = env->priv;
> > 
> > -        return gdb_get_regl(buf, env->priv);
> > +        /* include virtualization mode */
> > +
> > +        if (env->virt_enabled) {
> > +            ret |= PRV_V;
> > +        }
> > +        return gdb_get_regl(buf, ret);
> >  #endif
> >      }
> >      return 0;
> > @@ -225,11 +231,16 @@ static int riscv_gdb_set_virtual(CPUState *cs, uint8_t
> > *mem_buf, int n)
> >  #ifndef CONFIG_USER_ONLY
> >          RISCVCPU *cpu = RISCV_CPU(cs);
> >          CPURISCVState *env = &cpu->env;
> > +        target_ulong val = ldtul_p(mem_buf);
> > 
> > -        env->priv = ldtul_p(mem_buf) & 0x3;
> > +        env->priv = val & 0x3;
> >          if (env->priv == PRV_RESERVED) {
> >              env->priv = PRV_S;
> >          }
> > +
> > +        /* Update virtualization mode */
> > +
> > +        env->virt_enabled = (env->priv != PRV_M && (val & PRV_V) != 0);
> >  #endif
> >          return sizeof(target_ulong);
> >      }
> > --
> > 2.34.1
> > 
> > 
> >

virt-in-priv.patch
Description: Text Data