Re: [PATCH 5/6] target/riscv: fix checks in hmode/hmode32

[Weiwei Li]

在 2022/7/11 下午2:46, Alistair Francis 写道:
> On Sun, Jul 10, 2022 at 6:24 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
> > - It seems that there is no explicitly description about whether
> > the Hypervisor CSRs requires S extension
> > - Csrs only existed in RV32 will not trigger virtual instruction fault
> > when not in RV32
> >
> > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> > ---
> >   target/riscv/csr.c | 9 ++-------
> >   1 file changed, 2 insertions(+), 7 deletions(-)
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 0d8e98b7a9..975007f1ac 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -311,8 +311,7 @@ static int aia_smode32(CPURISCVState *env, int csrno)
> >
> >   static RISCVException hmode(CPURISCVState *env, int csrno)
> >   {
> > -    if (riscv_has_ext(env, RVS) &&
> > -        riscv_has_ext(env, RVH)) {
> > +    if (riscv_has_ext(env, RVH)) {
> This doesn't seem right. The spec doesn't explicitly say this, but I
> don't see how you can have the Hypervisor extension without S-mode

I'm also wonder about this. However, if H extension implicitly requires  
S-mode,

maybe it's better to add a check for this in riscv_cpu_realize, and we 
just check H here.

Regards,

Weiwei Li

> >           /* Hypervisor extension is supported */
> >           if ((env->priv == PRV_S && !riscv_cpu_virt_enabled(env)) ||
> >               env->priv == PRV_M) {
> > @@ -328,11 +327,7 @@ static RISCVException hmode(CPURISCVState *env, int csrno)
> >   static RISCVException hmode32(CPURISCVState *env, int csrno)
> >   {
> >       if (riscv_cpu_mxl(env) != MXL_RV32) {
> > -        if (!riscv_cpu_virt_enabled(env)) {
> > -            return RISCV_EXCP_ILLEGAL_INST;
> > -        } else {
> > -            return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> > -        }
> > +        return RISCV_EXCP_ILLEGAL_INST;
> Good catch
>
> Alistair
>
> >       }
> >
> >       return hmode(env, csrno);
> > --
> > 2.17.1