2025-11-21T13:04:09+08:00, <[email protected]>:
> From: Frank Chang <[email protected]>
>
> This helper returns the current effective privilege mode.
>
> Signed-off-by: Frank Chang <[email protected]>
> ---
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> @@ -38,6 +38,46 @@
> +bool riscv_cpu_eff_priv(CPURISCVState *env, int *priv, bool *virt)
I wonder if this function shouldn't be defined in a header file, so it
can be inlined, because returning values through pointers is quite
inefficient,
> +{
> +#ifndef CONFIG_USER_ONLY
> + int mode = env->priv;
> + bool virt_enabled = env->virt_enabled;
> + bool mode_modified = false;
> +
> +#ifndef CONFIG_USER_ONLY
We know CONFIG_USER_ONLY is not defined at this point.
> + if (mode == PRV_M && get_field(env->mstatus, MSTATUS_MPRV)) {
> + mode = get_field(env->mstatus, MSTATUS_MPP);
> + virt_enabled = get_field(env->mstatus, MSTATUS_MPV) && (mode !=
> PRV_M);
> + mode_modified = true;
> + }
> +#endif
> +
> + if (priv) {
> + *priv = mode;
> + }
> +
> + if (virt) {
> + *virt = virt_enabled;
> + }
> +
> + return mode_modified;
> +#else
> + *priv = env->priv;
Since it's #ifdef CONFIG_USER_ONLY, we can just say
*priv = PRV_U;
> + *virt = false;
> + return false;
> +#endif
> +}
> +
> int riscv_env_mmu_index(CPURISCVState *env, bool ifetch)
> {
> #ifdef CONFIG_USER_ONLY
> @@ -45,19 +85,14 @@ int riscv_env_mmu_index(CPURISCVState *env, bool ifetch)
> #else
> bool virt = env->virt_enabled;
> int mode = env->priv;
> + bool mode_modified = false;
>
> /* All priv -> mmu_idx mapping are here */
> if (!ifetch) {
> - uint64_t status = env->mstatus;
> -
> - if (mode == PRV_M && get_field(status, MSTATUS_MPRV)) {
> - mode = get_field(env->mstatus, MSTATUS_MPP);
> - virt = get_field(env->mstatus, MSTATUS_MPV) &&
> - (mode != PRV_M);
> - if (virt) {
> - status = env->vsstatus;
> - }
> - }
> + mode_modified = riscv_cpu_eff_priv(env, &mode, &virt);
> + uint64_t status = (mode_modified && virt) ? env->vsstatus :
> + env->mstatus;
It is likely a bug that MPRV=1+MPV=1 behaves differently from virt=1,
but your patch preserves the current behavior, as it should.
I had a few nitpicks, but important parts seem fine
Reviewed-by: Radim Krčmář <[email protected]>
Thanks.
