Re: [PATCH v2 2/6] target/riscv: Introduce extension implied rule helpers

[Alistair Francis]

On Sun, Jun 16, 2024 at 12:48 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> Introduce helpers to enable the extensions based on the implied rules.
> The implied extensions are enabled recursively, so we don't have to
> expand all of them manually. This also eliminates the old-fashioned
> ordering requirement. For example, Zvksg implies Zvks, Zvks implies
> Zvksed, etc., removing the need to check the implied rules of Zvksg
> before Zvks.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Reviewed-by: Jerry Zhang Jian <jerry.zhangjian@sifive.com>
> Tested-by: Max Chou <max.chou@sifive.com>
> ---
>  target/riscv/tcg/tcg-cpu.c | 91 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 91 insertions(+)
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index eb6f7b9d12..f8d6371764 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -36,6 +36,9 @@
>  static GHashTable *multi_ext_user_opts;
>  static GHashTable *misa_ext_user_opts;
>
> +static GHashTable *misa_implied_rules;
> +static GHashTable *ext_implied_rules;
> +
>  static bool cpu_cfg_ext_is_user_set(uint32_t ext_offset)
>  {
>      return g_hash_table_contains(multi_ext_user_opts,
> @@ -836,11 +839,97 @@ static void riscv_cpu_validate_profiles(RISCVCPU *cpu)
>      }
>  }
>
> +static void riscv_cpu_init_implied_exts_rules(void)
> +{
> +    RISCVCPUImpliedExtsRule *rule;
> +    int i;
> +
> +    for (i = 0; (rule = riscv_misa_implied_rules[i]); i++) {
> +        g_hash_table_insert(misa_implied_rules, GUINT_TO_POINTER(rule->ext),
> +                            (gpointer)rule);
> +    }
> +
> +    for (i = 0; (rule = riscv_ext_implied_rules[i]); i++) {
> +        g_hash_table_insert(ext_implied_rules, GUINT_TO_POINTER(rule->ext),
> +                            (gpointer)rule);
> +    }
> +}
> +
> +static void cpu_enable_implied_rule(RISCVCPU *cpu,
> +                                    RISCVCPUImpliedExtsRule *rule)
> +{
> +    CPURISCVState *env = &cpu->env;
> +    RISCVCPUImpliedExtsRule *ir;
> +    bool enabled = false;
> +    int i;
> +
> +#ifndef CONFIG_USER_ONLY
> +    enabled = qatomic_read(&rule->enabled) & BIT_ULL(cpu->env.mhartid);

enabled is a uint64_t, so this limits us to 64 harts right?

The virt machine currently has a limit of 512, so this won't work right?

Alistair

> +#endif
> +
> +    if (!enabled) {
> +        /* Enable the implied MISAs. */
> +        if (rule->implied_misas) {
> +            riscv_cpu_set_misa_ext(env, env->misa_ext | rule->implied_misas);
> +
> +            for (i = 0; misa_bits[i] != 0; i++) {
> +                if (rule->implied_misas & misa_bits[i]) {
> +                    ir = g_hash_table_lookup(misa_implied_rules,
> +                                             GUINT_TO_POINTER(misa_bits[i]));
> +
> +                    if (ir) {
> +                        cpu_enable_implied_rule(cpu, ir);
> +                    }
> +                }
> +            }
> +        }
> +
> +        /* Enable the implied extensions. */
> +        for (i = 0; rule->implied_exts[i] != RISCV_IMPLIED_EXTS_RULE_END; 
> i++) {
> +            cpu_cfg_ext_auto_update(cpu, rule->implied_exts[i], true);
> +
> +            ir = g_hash_table_lookup(ext_implied_rules,
> +                                     
> GUINT_TO_POINTER(rule->implied_exts[i]));
> +
> +            if (ir) {
> +                cpu_enable_implied_rule(cpu, ir);
> +            }
> +        }
> +
> +#ifndef CONFIG_USER_ONLY
> +        qatomic_or(&rule->enabled, BIT_ULL(cpu->env.mhartid));
> +#endif
> +    }
> +}
> +
> +static void riscv_cpu_enable_implied_rules(RISCVCPU *cpu)
> +{
> +    RISCVCPUImpliedExtsRule *rule;
> +    int i;
> +
> +    /* Enable the implied MISAs. */
> +    for (i = 0; (rule = riscv_misa_implied_rules[i]); i++) {
> +        if (riscv_has_ext(&cpu->env, rule->ext)) {
> +            cpu_enable_implied_rule(cpu, rule);
> +        }
> +    }
> +
> +    /* Enable the implied extensions. */
> +    for (i = 0; (rule = riscv_ext_implied_rules[i]); i++) {
> +        if (isa_ext_is_enabled(cpu, rule->ext)) {
> +            cpu_enable_implied_rule(cpu, rule);
> +        }
> +    }
> +}
> +
>  void riscv_tcg_cpu_finalize_features(RISCVCPU *cpu, Error **errp)
>  {
>      CPURISCVState *env = &cpu->env;
>      Error *local_err = NULL;
>
> +    riscv_cpu_init_implied_exts_rules();
> +    riscv_cpu_enable_implied_rules(cpu);
> +
>      riscv_cpu_validate_misa_priv(env, &local_err);
>      if (local_err != NULL) {
>          error_propagate(errp, local_err);
> @@ -1346,6 +1435,8 @@ static void riscv_tcg_cpu_instance_init(CPUState *cs)
>
>      misa_ext_user_opts = g_hash_table_new(NULL, g_direct_equal);
>      multi_ext_user_opts = g_hash_table_new(NULL, g_direct_equal);
> +    misa_implied_rules = g_hash_table_new(NULL, g_direct_equal);
> +    ext_implied_rules = g_hash_table_new(NULL, g_direct_equal);
>      riscv_cpu_add_user_properties(obj);
>
>      if (riscv_cpu_has_max_extensions(obj)) {
> --
> 2.43.2
>
>