|
From: | Daniel Henrique Barboza |
Subject: | Re: [PATCH 1/4] target/riscv: Make MPV only work when MPP != PRV_M |
Date: | Wed, 31 May 2023 06:28:27 -0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 |
On 5/30/23 21:27, Weiwei Li wrote:
On 2023/5/31 04:23, Daniel Henrique Barboza wrote:On 5/29/23 09:17, Weiwei Li wrote:Upon MRET or explicit memory access with MPRV=1, MPV should be ignored when MPP=PRV_M. Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> --- target/riscv/cpu_helper.c | 3 ++- target/riscv/op_helper.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 09ea227ceb..bd892c05d4 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -46,7 +46,8 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch) if (mode == PRV_M && get_field(status, MSTATUS_MPRV)) { mode = get_field(env->mstatus, MSTATUS_MPP); - virt = get_field(env->mstatus, MSTATUS_MPV); + virt = get_field(env->mstatus, MSTATUS_MPV) && + (mode != PRV_M);This change makes more sense in patch 2 where you also removed the 'mode' check for MPRV. As it is now I read the code above and thought "but mode is guaranteed to be == PRV_M, so (mode != PRV_M) is guaranteed to be false every time".No, this 'mode' (get from MPP) is not the previous 'mode'(the current privilege mode).
Oh, right:
if (mode == PRV_M && get_field(status, MSTATUS_MPRV)) { mode = get_field(env->mstatus, MSTATUS_MPP);
'mode' is being reassigned here.
Regards, Weiwei LiThe change in helper_mret() below is fine. Thanks, Danielif (virt) { status = env->vsstatus; } diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c index f563dc3981..9cdb9cdd06 100644 --- a/target/riscv/op_helper.c +++ b/target/riscv/op_helper.c @@ -335,7 +335,8 @@ target_ulong helper_mret(CPURISCVState *env) riscv_raise_exception(env, RISCV_EXCP_INST_ACCESS_FAULT, GETPC()); } - target_ulong prev_virt = get_field(env->mstatus, MSTATUS_MPV); + target_ulong prev_virt = get_field(env->mstatus, MSTATUS_MPV) && + (prev_priv != PRV_M); mstatus = set_field(mstatus, MSTATUS_MIE, get_field(mstatus, MSTATUS_MPIE)); mstatus = set_field(mstatus, MSTATUS_MPIE, 1);
[Prev in Thread] | Current Thread | [Next in Thread] |