Re: [PATCH 3/4] target/riscv: Support MSTATUS.MPV/GVA only when RVH is enabled

[Weiwei Li]

On 2023/6/12 13:40, LIU Zhiwei wrote:
> On 2023/6/12 12:35, Weiwei Li wrote:
> > On 2023/6/12 11:18, LIU Zhiwei wrote:
> > > On 2023/6/12 11:16, Weiwei Li wrote:
> > > > On 2023/6/12 11:08, LIU Zhiwei wrote:
> > > > > On 2023/5/29 20:17, Weiwei Li wrote:
> > > > > > MPV and GVA bits are added by hypervisor extension to mstatus
> > > > > > and mstatush (if MXLEN=32).
> > > > >
> > > > > Have you found the CSR field specifications for them, especially 
> > > > > for GVA.
> > > >
> > > > Yeah.  in the section 9.4.1 of the privilege spec:
> > > >
> > > > "/The hypervisor extension adds two fields, MPV and GVA, to the 
> > > > machine-level mstatus or mstatush CSR/".
> > >
> > > I mean the WARL or other CSR field specifications here.
> >
> > I don't quite get your idea. The only specification for MPV and GVA  
> > I found is in section 9.4.1.  The spec for most of mstatus fields can 
> > be found in  Section 3.1.6
> > "Machine Status Registers (mstatus and mstatush)".
>
> I mean is the GVA field read only or WARL(WPRI or WLRL) for the 
> software? It could be written by the implementation. But I am not sure 
> whether it could be written by the software.

No, I didn't find any description about this.

Regards,

Weiwei Li

> Zhiwei
>
> > Regards,
> >
> > Weiwei Li
> >
> > > Zhiwei
> > >
> > > > Regards,
> > > >
> > > > Weiwei Li
> > > >
> > > > > Zhiwei
> > > > >
> > > > > > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> > > > > > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> > > > > > ---
> > > > > >   target/riscv/csr.c | 10 ++++------
> > > > > >   1 file changed, 4 insertions(+), 6 deletions(-)
> > > > > >
> > > > > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > > > > > index 58499b5afc..6ac11d1f11 100644
> > > > > > --- a/target/riscv/csr.c
> > > > > > +++ b/target/riscv/csr.c
> > > > > > @@ -1311,11 +1311,9 @@ static RISCVException 
> > > > > > write_mstatus(CPURISCVState *env, int csrno,
> > > > > >       }
> > > > > >         if (xl != MXL_RV32 || env->debugger) {
> > > > > > -        /*
> > > > > > -         * RV32: MPV and GVA are not in mstatus. The current 
> > > > > > plan is to
> > > > > > -         * add them to mstatush. For now, we just don't support it.
> > > > > > -         */
> > > > > > -        mask |= MSTATUS_MPV | MSTATUS_GVA;
> > > > > > +        if (riscv_has_ext(env, RVH)) {
> > > > > > +            mask |= MSTATUS_MPV | MSTATUS_GVA;
> > > > > > +        }
> > > > > >           if ((val & MSTATUS64_UXL) != 0) {
> > > > > >               mask |= MSTATUS64_UXL;
> > > > > >           }
> > > > > > @@ -1351,7 +1349,7 @@ static RISCVException 
> > > > > > write_mstatush(CPURISCVState *env, int csrno,
> > > > > >                                        target_ulong val)
> > > > > >   {
> > > > > >       uint64_t valh = (uint64_t)val << 32;
> > > > > > -    uint64_t mask = MSTATUS_MPV | MSTATUS_GVA;
> > > > > > +    uint64_t mask = riscv_has_ext(env, RVH) ? MSTATUS_MPV | 
> > > > > > MSTATUS_GVA : 0;
> > > > > >         env->mstatus = (env->mstatus & ~mask) | (valh & mask);