On Mon, Dec 16, 2024 at 10:19 PM <baturo.ale...@gmail.com> wrote:
>
> From: Alexey Baturo <baturo.ale...@gmail.com>
>
> Signed-off-by: Alexey Baturo <baturo.ale...@gmail.com>
> ---
> target/riscv/cpu.h | 5 +++
> target/riscv/cpu_helper.c | 73 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 78 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 417ff45544..e7f346ff6b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -768,8 +768,13 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>
> bool riscv_cpu_is_32bit(RISCVCPU *cpu);
>
> +bool riscv_cpu_virt_mem_enabled(CPURISCVState *env);
> +RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env);
> +uint32_t riscv_pm_get_pmlen(RISCVPmPmm pmm);
> +
> RISCVException riscv_csrr(CPURISCVState *env, int csrno,
> target_ulong *ret_value);
> +
> RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
> target_ulong *ret_value,
> target_ulong new_value, target_ulong write_mask);
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index dba04851d5..ed0d275e84 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -214,6 +214,79 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
> *pflags = flags;
> }
>
> +RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env)
> +{
> + RISCVPmPmm pmm = PMM_FIELD_DISABLED;
> +#ifndef CONFIG_USER_ONLY
> + if (get_field(env->mstatus, MSTATUS_MPRV) &&
> + get_field(env->mstatus, MSTATUS_MXR)) {
> + return pmm;
> + }
> + int priv_mode = cpu_address_mode(env);
QEMU style is to not mix code and declarations.
I think a clearer way would be
#ifndef CONFIG_USER_ONLY
RISCVPmPmm pmm = PMM_FIELD_DISABLED;
int priv_mode = cpu_address_mode(env);
if (get_field(env->mstatus, MSTATUS_MPRV) &&
get_field(env->mstatus, MSTATUS_MXR)) {
return pmm;
}
/* Get current PMM field */
...
return pmm;
#else
return PMM_FIELD_DISABLED;
#endif
> + /* Get current PMM field */
> + switch (priv_mode) {
> + case PRV_M:
> + if (riscv_cpu_cfg(env)->ext_smmpm) {
> + pmm = get_field(env->mseccfg, MSECCFG_PMM);
> + }
> + break;
> + case PRV_S:
> + if (riscv_cpu_cfg(env)->ext_smnpm) {
> + if (get_field(env->mstatus, MSTATUS_MPV)) {
> + pmm = get_field(env->henvcfg, HENVCFG_PMM);
> + } else {
> + pmm = get_field(env->menvcfg, MENVCFG_PMM);
> + }
> + }
> + break;
> + case PRV_U:
> + if (riscv_has_ext(env, RVS)) {
> + if (riscv_cpu_cfg(env)->ext_ssnpm) {
> + pmm = get_field(env->senvcfg, SENVCFG_PMM);
> + }
> + } else {
> + if (riscv_cpu_cfg(env)->ext_smnpm) {
> + pmm = get_field(env->menvcfg, MENVCFG_PMM);
> + }
> + }
> + break;
> + default:
> + g_assert_not_reached();
> + }
> +#endif
> + return pmm;
> +}
> +
> +bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
> +{
> + bool virt_mem_en = false;
> +#ifndef CONFIG_USER_ONLY
> + int satp_mode = 0;
> + int priv_mode = cpu_address_mode(env);
New line please :)
> + if (riscv_cpu_mxl(env) == MXL_RV32) {
> + satp_mode = get_field(env->satp, SATP32_MODE);
> + } else {
> + satp_mode = get_field(env->satp, SATP64_MODE);
> + }
> + virt_mem_en = ((satp_mode != VM_1_10_MBARE) && (priv_mode != PRV_M));
> +#endif
> + return virt_mem_en;
I think an #else is probably easier to read
Otherwise:
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
Alistair
> +}
> +
> +uint32_t riscv_pm_get_pmlen(RISCVPmPmm pmm)
> +{
> + switch (pmm) {
> + case PMM_FIELD_DISABLED:
> + return 0;
> + case PMM_FIELD_PMLEN7:
> + return 7;
> + case PMM_FIELD_PMLEN16:
> + return 16;
> + default:
> + g_assert_not_reached();
> + }
> +}
> +
> #ifndef CONFIG_USER_ONLY
>
> /*
> --
> 2.39.5
>
