diff mbox series

[v12,3/7] target/riscv: Add helper functions to calculate current number of masked bits for pointer masking

Message ID 20241205112304.593204-4-baturo.alexey@gmail.com (mailing list archive)
State New
Headers show
Series Pointer Masking update for Zjpm v1.0 | expand

Commit Message

Alexey Baturo Dec. 5, 2024, 11:23 a.m. UTC 
From: Alexey Baturo <baturo.alexey@gmail.com>

Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
---
 target/riscv/cpu.h        |  5 +++
 target/riscv/cpu_helper.c | 74 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

Comments

Daniel Henrique Barboza Dec. 6, 2024, 9:07 p.m. UTC | #1 
On 12/5/24 8:23 AM, baturo.alexey@gmail.com wrote:
> From: Alexey Baturo <baturo.alexey@gmail.com>
> 
> Signed-off-by: Alexey Baturo <baturo.alexey@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
>   
>   /*
Alexey Baturo Dec. 7, 2024, 7:20 a.m. UTC | #2 
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 <
dbarboza@ventanamicro.com>:

>
>
> On 12/5/24 8:23 AM, baturo.alexey@gmail.com wrote:
> > From: Alexey Baturo <baturo.alexey@gmail.com>
> >
> > Signed-off-by: Alexey Baturo <baturo.alexey@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
> >
> >   /*
>
>
diff mbox series

Patch

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;
+}
+
 #ifndef CONFIG_USER_ONLY
 
 /*