[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 12/13] target/riscv: Add a preferred ISA extension rule
|
From: |
Atish Kumar Patra |
|
Subject: |
Re: [PATCH v2 12/13] target/riscv: Add a preferred ISA extension rule |
|
Date: |
Wed, 7 Aug 2024 00:46:57 -0700 |
On Mon, Aug 5, 2024 at 6:48 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Jul 24, 2024 at 9:32 AM Atish Patra <atishp@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
>
As per my understanding, the implied rule defines the mandatory
dependent extensions
specified by ISA. The preferred rule allows you to enable more related
extensions which
the user will commonly enable but necessary to.
These preferred extensions can be individually disabled too if a user
wants. This feature needs
be verified though as I just wanted to get feedback on the preferred rule thing.
Let me know if I misunderstood the intention of the implied rule.
> 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 <atishp@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
> >
> >