[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 2/3] target/riscv: Fixup setting GVA
From: |
Bin Meng |
Subject: |
Re: [PATCH v4 2/3] target/riscv: Fixup setting GVA |
Date: |
Tue, 21 Dec 2021 15:30:05 +0800 |
On Mon, Dec 20, 2021 at 2:49 PM Alistair Francis
<alistair.francis@opensource.wdc.com> wrote:
>
> From: Alistair Francis <alistair.francis@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.francis@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