Hi Radim, On Tue, Nov 25, 2025 at 11:26 PM Radim Krčmář <[email protected]> wrote:
> 2025-11-21T13:04:10+08:00, <[email protected]>: > > From: Frank Chang <[email protected]> > > > > mstatus.MPV only records the previous virtualization state, and does not > > affect pointer masking according to the Zjpm specification. > > > > This patch rewrites riscv_pm_get_pmm() to follow the architectural > > definition of Smmpm, Smnpm, and Ssnpm. > > > > The resulting PMM selection logic for each mode is summarized below: > > > > * mstatus.MXR = 1: pointer masking disabled > > > > * Smmpm + Smnpm + Ssnpm: > > M-mode: mseccfg.PMM > > S-mode: menvcfg.PMM > > U-mode: senvcfg.PMM > > VS-mode: henvcfg.PMM > > VU-mode: senvcfg.PMM > > > > * Smmpm + Smnpm (RVS implemented): > > M-mode: mseccfg.PMM > > S-mode: menvcfg.PMM > > U/VS/VU: disabled (Ssnpm not present) > > > > * Smmpm + Smnpm (RVS not implemented): > > M-mode: mseccfg.PMM > > U-mode: menvcfg.PMM > > S/VS/VU: disabled (no S-mode) > > > > * Smmpm only: > > M-mode: mseccfg.PMM > > Other existing modes: pointer masking disabled > > > > Signed-off-by: Frank Chang <[email protected]> > > Reviewed-by: Daniel Henrique Barboza <[email protected]> > > --- > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > > RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env) > > { > > #ifndef CONFIG_USER_ONLY > > - int priv_mode = cpu_address_mode(env); > > + int priv_mode; > > + bool virt; > > > > - if (get_field(env->mstatus, MSTATUS_MPRV) && > > - get_field(env->mstatus, MSTATUS_MXR)) { > > + if (get_field(env->mstatus, MSTATUS_MXR)) { > > return PMM_FIELD_DISABLED; > > } > > MSTATUS_MXR doesn't disable Pointer masking in M-mode, and we also need > to consider virt, where vsstatus.MXR is in effect as well. > > > > > + riscv_cpu_eff_priv(env, &priv_mode, &virt); > > I think you could put something like this here: > > if ((mode != PRV_M && get_field(env->mstatus, MSTATUS_MXR)) || > (virt && get_field(env->vsstatus, MSTATUS_MXR))) { > return PMM_FIELD_DISABLED; > } > > > /* Get current PMM field */ > > switch (priv_mode) { > > case PRV_M: > > @@ -189,22 +222,30 @@ RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env) > > case PRV_U: > > - if (riscv_has_ext(env, RVS)) { > > + if (!virt) { > > if (riscv_cpu_cfg(env)->ext_ssnpm) { > > return get_field(env->senvcfg, SENVCFG_PMM); > > } > > - } else { > > + > > if (riscv_cpu_cfg(env)->ext_smnpm) { > > - return get_field(env->menvcfg, MENVCFG_PMM); > > + if (!riscv_has_ext(env, RVS)) { > > + return get_field(env->menvcfg, MENVCFG_PMM); > > + } > > + } > > + } else { > > + if (riscv_cpu_cfg(env)->ext_ssnpm) { > > + return get_field(env->senvcfg, SENVCFG_PMM); > > } > > } > > virt doesn't really matter, the original code was correct. > > Thanks. > Thanks for the review, I've sent out the v3 patchset: https://patchew.org/QEMU/[email protected]/ Regards, Frank Chang
