| Message ID | 20220710082400.29224-6-liweiwei@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve the U/S/H extension related check | expand |
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 > /* 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 > >
在 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 >> >>
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)) { /* 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; } return hmode(env, csrno);