Thanks Daniel. Let's wait for about a week or so for other suggestions to the patches and then I'll send a new updated series.
сб, 7 дек. 2024 г. в 00:08, Daniel Henrique Barboza < dbarb...@ventanamicro.com>: > > > On 12/5/24 8:23 AM, 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 | 74 +++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 79 insertions(+) > > > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > > index 417ff45544..74d7076f5a 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); > > +int 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..2558f869f2 100644 > > --- a/target/riscv/cpu_helper.c > > +++ b/target/riscv/cpu_helper.c > > @@ -214,6 +214,80 @@ 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); > > + /* 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); > > + 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; > > +} > > + > > +int 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(); > > + } > > + return -1; > > You don't need a 'return -1' here since all possible return values are > already > covered in the 'switch' and the default label is using > g_assert_not_reached(), > i.e. you'll never return -1. The compiler will be fine with it - we use > this > kind of pattern all the time (cpu_get_fcfien() does this in this same > file). > > You can remove the -1 return and even turn the helper to uint32_t. That way > the caller knows that there's no need to handle negative values. > > > Thanks, > > Daniel > > > > +} > > + > > #ifndef CONFIG_USER_ONLY > > > > /* > >