On Mon, Dec 20, 2021 at 2:49 PM Alistair Francis <alistair.fran...@opensource.wdc.com> wrote: > > From: Alistair Francis <alistair.fran...@wdc.com> > > In preperation for adding support for the illegal instruction address
typo: preparation > let's fixup the Hypervisor extension setting GVA logic and improve the > variable names. > > Signed-off-by: Alistair Francis <alistair.fran...@wdc.com> > --- > target/riscv/cpu_helper.c | 21 ++++++--------------- > 1 file changed, 6 insertions(+), 15 deletions(-) > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 9eeed38c7e..9e1f5ee177 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -967,6 +967,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) > > RISCVCPU *cpu = RISCV_CPU(cs); > CPURISCVState *env = &cpu->env; > + bool write_gva = false; > uint64_t s; > > /* cs->exception is 32-bits wide unlike mcause which is XLEN-bits wide > @@ -975,7 +976,6 @@ void riscv_cpu_do_interrupt(CPUState *cs) > bool async = !!(cs->exception_index & RISCV_EXCP_INT_FLAG); > target_ulong cause = cs->exception_index & RISCV_EXCP_INT_MASK; > target_ulong deleg = async ? env->mideleg : env->medeleg; > - bool write_tval = false; > target_ulong tval = 0; > target_ulong htval = 0; > target_ulong mtval2 = 0; > @@ -1004,7 +1004,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) > case RISCV_EXCP_INST_PAGE_FAULT: > case RISCV_EXCP_LOAD_PAGE_FAULT: > case RISCV_EXCP_STORE_PAGE_FAULT: > - write_tval = true; > + write_gva = true; > tval = env->badaddr; > break; > default: > @@ -1041,18 +1041,6 @@ void riscv_cpu_do_interrupt(CPUState *cs) > if (riscv_has_ext(env, RVH)) { > target_ulong hdeleg = async ? env->hideleg : env->hedeleg; > > - if (env->two_stage_lookup && write_tval) { > - /* > - * If we are writing a guest virtual address to stval, set > - * this to 1. If we are trapping to VS we will set this to 0 > - * later. > - */ > - env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 1); > - } else { > - /* For other HS-mode traps, we set this to 0. */ > - env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 0); > - } > - > if (riscv_cpu_virt_enabled(env) && ((hdeleg >> cause) & 1)) { > /* Trap to VS mode */ > /* > @@ -1063,7 +1051,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) > cause == IRQ_VS_EXT) { > cause = cause - 1; > } > - env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 0); > + write_gva = false; > } else if (riscv_cpu_virt_enabled(env)) { > /* Trap into HS mode, from virt */ > riscv_cpu_swap_hypervisor_regs(env); > @@ -1072,6 +1060,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) > env->hstatus = set_field(env->hstatus, HSTATUS_SPV, > riscv_cpu_virt_enabled(env)); > > + > htval = env->guest_phys_fault_addr; > > riscv_cpu_set_virt_enabled(env, 0); > @@ -1079,7 +1068,9 @@ void riscv_cpu_do_interrupt(CPUState *cs) > /* Trap into HS mode */ > env->hstatus = set_field(env->hstatus, HSTATUS_SPV, false); > htval = env->guest_phys_fault_addr; > + write_gva = false; > } > + env->hstatus = set_field(env->hstatus, HSTATUS_GVA, write_gva); This does not look correct to me. The env->hstatus[GVA] should remain untouched in the 2nd and 3rd branch. It only needs to be set in the first branch. > } > > s = env->mstatus; Regards, Bin