On 2022/9/16 14:00, Richard Henderson wrote:
On 9/6/22 14:22, Christoph Muellner wrote:
From: Christoph Müllner <christoph.muell...@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.muell...@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~
