On Wed, Jul 24, 2024 at 9:32 AM Atish Patra <ati...@rivosinc.com> wrote:
>
> In addition to the implied rule, a preferred rule will be useful
> where an ISA extension may require a list of ISA extension to be
> enabled to use all the features defined in that extension. All
> these extensions may not be implied in the ISA.
This seems practically the same as an implied rule, I'm not sure we
need a separate list of rules
Alistair
>
> This patch just introduces a new preferred rule which allows
> to enable multiple extensions together without burdening the qemu
> commandline user.
>
> Signed-off-by: Atish Patra <ati...@rivosinc.com>
> ---
> target/riscv/cpu.c | 4 ++++
> target/riscv/cpu.h | 17 ++++++++++++++
> target/riscv/tcg/tcg-cpu.c | 57
> ++++++++++++++++++++++++++++++++++++++++------
> 3 files changed, 71 insertions(+), 7 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 393d1d67120e..22ba43c7ff2a 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -2665,6 +2665,10 @@ RISCVCPUImpliedExtsRule
> *riscv_multi_ext_implied_rules[] = {
> NULL
> };
>
> +RISCVCPUPreferredExtsRule *riscv_multi_ext_preferred_rules[] = {
> + NULL
> +};
> +
> static Property riscv_cpu_properties[] = {
> DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index af25550a4a54..d775866344f5 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -142,10 +142,27 @@ typedef struct riscv_cpu_implied_exts_rule {
> const uint32_t implied_multi_exts[];
> } RISCVCPUImpliedExtsRule;
>
> +typedef struct riscv_cpu_preferred_exts_rule {
> +#ifndef CONFIG_USER_ONLY
> + /*
> + * Bitmask indicates the rule enabled status for the harts.
> + * This enhancement is only available in system-mode QEMU,
> + * as we don't have a good way (e.g. mhartid) to distinguish
> + * the SMP cores in user-mode QEMU.
> + */
> + unsigned long *enabled;
> +#endif
> + /* multi extension offset. */
> + const uint32_t ext;
> + const uint32_t preferred_multi_exts[];
> +} RISCVCPUPreferredExtsRule;
> +
> extern RISCVCPUImpliedExtsRule *riscv_misa_ext_implied_rules[];
> extern RISCVCPUImpliedExtsRule *riscv_multi_ext_implied_rules[];
> +extern RISCVCPUPreferredExtsRule *riscv_multi_ext_preferred_rules[];
>
> #define RISCV_IMPLIED_EXTS_RULE_END -1
> +#define RISCV_PREFRRED_EXTS_RULE_END RISCV_IMPLIED_EXTS_RULE_END
>
> #define MMU_USER_IDX 3
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 1c9a87029423..d8f74720815a 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -32,6 +32,7 @@
> #include "hw/core/accel-cpu.h"
> #include "hw/core/tcg-cpu-ops.h"
> #include "tcg/tcg.h"
> +#include <stdio.h>
> #ifndef CONFIG_USER_ONLY
> #include "hw/boards.h"
> #endif
> @@ -733,6 +734,7 @@ static void riscv_cpu_validate_profiles(RISCVCPU *cpu)
> static void riscv_cpu_init_ext_rules(void)
> {
> RISCVCPUImpliedExtsRule *rule;
> + RISCVCPUPreferredExtsRule *prule;
> #ifndef CONFIG_USER_ONLY
> MachineState *ms = MACHINE(qdev_get_machine());
> #endif
> @@ -760,22 +762,40 @@ static void riscv_cpu_init_ext_rules(void)
> GUINT_TO_POINTER(rule->ext), (gpointer)rule);
> }
>
> + for (i = 0; (prule = riscv_multi_ext_preferred_rules[i]); i++) {
> +#ifndef CONFIG_USER_ONLY
> + prule->enabled = bitmap_new(ms->smp.cpus);
> +#endif
> + g_hash_table_insert(multi_ext_enabling_rules,
> + GUINT_TO_POINTER(prule->ext), (gpointer)prule);
> + }
> +
> initialized = true;
> }
>
> static void cpu_enable_ext_rule(RISCVCPU *cpu,
> - RISCVCPUImpliedExtsRule *rule)
> + RISCVCPUImpliedExtsRule *rule,
> + RISCVCPUPreferredExtsRule *prule)
> {
> CPURISCVState *env = &cpu->env;
> RISCVCPUImpliedExtsRule *ir;
> + RISCVCPUPreferredExtsRule *pr;
> bool enabled = false;
> int i;
>
> #ifndef CONFIG_USER_ONLY
> - enabled = test_bit(cpu->env.mhartid, rule->enabled);
> + if (rule) {
> + enabled = test_bit(cpu->env.mhartid, rule->enabled);
> + } else if (prule) {
> + enabled = test_bit(cpu->env.mhartid, prule->enabled);
> + } else {
> + return;
> + }
> #endif
> + if (enabled)
> + return;
>
> - if (!enabled) {
> + if (rule) {
> /* Enable the implied MISAs. */
> if (rule->implied_misa_exts) {
> riscv_cpu_set_misa_ext(env,
> @@ -787,7 +807,7 @@ static void cpu_enable_ext_rule(RISCVCPU *cpu,
> GUINT_TO_POINTER(misa_bits[i]));
>
> if (ir) {
> - cpu_enable_ext_rule(cpu, ir);
> + cpu_enable_ext_rule(cpu, ir, NULL);
> }
> }
> }
> @@ -803,12 +823,27 @@ static void cpu_enable_ext_rule(RISCVCPU *cpu,
> rule->implied_multi_exts[i]));
>
> if (ir) {
> - cpu_enable_ext_rule(cpu, ir);
> + cpu_enable_ext_rule(cpu, ir, NULL);
> }
> }
>
> #ifndef CONFIG_USER_ONLY
> bitmap_set(rule->enabled, cpu->env.mhartid, 1);
> +#endif
> + } else if (prule) {
> + /* Enable the preferred extensions. */
> + for (i = 0;
> + prule->preferred_multi_exts[i] != RISCV_PREFRRED_EXTS_RULE_END;
> i++) {
> + cpu_cfg_ext_auto_update(cpu, prule->preferred_multi_exts[i],
> true);
> + pr = g_hash_table_lookup(multi_ext_enabling_rules,
> + GUINT_TO_POINTER(
> + prule->preferred_multi_exts[i]));
> + if (pr) {
> + cpu_enable_ext_rule(cpu, NULL, prule);
> + }
> + }
> +#ifndef CONFIG_USER_ONLY
> + bitmap_set(prule->enabled, cpu->env.mhartid, 1);
> #endif
> }
> }
> @@ -847,6 +882,7 @@ static void cpu_enable_zc_implied_rules(RISCVCPU *cpu)
> static void riscv_cpu_enable_ext_rules(RISCVCPU *cpu)
> {
> RISCVCPUImpliedExtsRule *rule;
> + RISCVCPUPreferredExtsRule *prule;
> int i;
>
> /* Enable the implied extensions for Zc. */
> @@ -855,14 +891,21 @@ static void riscv_cpu_enable_ext_rules(RISCVCPU *cpu)
> /* Enable the implied MISAs. */
> for (i = 0; (rule = riscv_misa_ext_implied_rules[i]); i++) {
> if (riscv_has_ext(&cpu->env, rule->ext)) {
> - cpu_enable_ext_rule(cpu, rule);
> + cpu_enable_ext_rule(cpu, rule, NULL);
> }
> }
>
> /* Enable the implied extensions. */
> for (i = 0; (rule = riscv_multi_ext_implied_rules[i]); i++) {
> if (isa_ext_is_enabled(cpu, rule->ext)) {
> - cpu_enable_ext_rule(cpu, rule);
> + cpu_enable_ext_rule(cpu, rule, NULL);
> + }
> + }
> +
> + /* Enable the preferred extensions. */
> + for (i = 0; (prule = riscv_multi_ext_preferred_rules[i]); i++) {
> + if (isa_ext_is_enabled(cpu, prule->ext)) {
> + cpu_enable_ext_rule(cpu, NULL, prule);
> }
> }
> }
>
> --
> 2.34.1
>
>
