On 8/27/24 01:29, Deepak Gupta wrote:
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 8e1f05e5b1..083d405516 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1022,6 +1022,11 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType
type)
env->load_res = -1;
set_default_nan_mode(1, &env->fp_status);
+#ifdef CONFIG_USER_ONLY
+ /* qemu-user for riscv, fcfi is off by default */
+ env->ufcfien = false;
+#endif
...
@@ -226,6 +226,7 @@ struct CPUArchState {
bool elp;
#ifdef CONFIG_USER_ONLY
uint32_t elf_flags;
+ bool ufcfien;
#endif
Thinking about this more, I think adding separate controls for user-only is a bad
precedent to set. You said you are adding these because senvcfg/menvcfg are ifdefed:
well, that should be the thing that we fix.
The only real user of *envcfg that I see so far is check_zicbo_envcfg, which does not use
the same switch statement as this:
+ switch (env->priv) {
+ case PRV_U:
+ if (riscv_has_ext(env, RVS)) {
+ return env->senvcfg & MENVCFG_LPE;
+ }
+ return env->menvcfg & MENVCFG_LPE;
+ case PRV_S:
+ if (env->virt_enabled) {
+ return env->henvcfg & HENVCFG_LPE;
+ }
+ return env->menvcfg & MENVCFG_LPE;
+ case PRV_M:
+ return env->mseccfg & MSECCFG_MLPE;
+ default:
+ g_assert_not_reached();
+ }
I think your function should look more like check_zicbo_envcfg: (1) PRV_U may be either U
or VU, and different tests apply; (2) M-mode disable means that no lower level may be enabled.
It would be nice if you could generalize check_zicbo_envcfg to apply to your new use as
well. Perhaps some tri-state function to say "enabled", "disabled", "virtual disabled".
r~
