|
From: | LIU Zhiwei |
Subject: | Re: [PATCH 01/11] riscv: Add privilege level to DisasContext |
Date: | Fri, 16 Sep 2022 14:21:10 +0800 |
User-agent: | Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 |
On 2022/9/16 14:00, Richard Henderson wrote:
On 9/6/22 14:22, Christoph Muellner wrote:From: Christoph Müllner <christoph.muellner@vrull.eu> This allows privileged instructions to check the required privilege level in the translation without calling a helper. Signed-off-by: Christoph Müllner <christoph.muellner@vrull.eu> --- target/riscv/translate.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/target/riscv/translate.c b/target/riscv/translate.c index 63b04e8a94..fd241ff667 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -59,6 +59,9 @@ typedef struct DisasContext {/* pc_succ_insn points to the instruction following base.pc_next */target_ulong pc_succ_insn; target_ulong priv_ver; +#ifndef CONFIG_USER_ONLY + target_ulong priv; +#endif RISCVMXL misa_mxl_max; RISCVMXL xl; uint32_t misa_ext;@@ -1079,6 +1082,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)ctx->mstatus_vs = tb_flags & TB_FLAGS_MSTATUS_VS; ctx->priv_ver = env->priv_ver; #if !defined(CONFIG_USER_ONLY) + ctx->priv = env->priv;Reading directly from env here is, in general, wrong. Items that are constant, like priv_ver, are ok. But otherwise these values must be recorded by cpu_get_tb_cpu_state().This instance will happen to work, because the execution context is already constrained.
Exactly. Thanks for pointing it out.
In this case because env->priv == ctx->mem_idx (see cpu_mmu_index) so, really, you don't need this new field at all. Or, keep the field, because it's usage will be more self-documentary, but copy the value from ctx->mmu_idx and add a comment.if (riscv_has_ext(env, RVH)) { ctx->virt_enabled = riscv_cpu_virt_enabled(env); } else {Incidentally, this (existing) usage appears to be a bug. I can see nothing in cpu_get_tb_cpu_state that corresponds directly to this value.
Agree. Zhiwei
r~
[Prev in Thread] | Current Thread | [Next in Thread] |