[PATCH RESEND 1/6] target/riscv: Introduce extension implied rules definition

frank.chang@sifive.com posted 6 patches 5 months, 3 weeks ago
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bmeng.cn@gmail.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>
There is a newer version of this series
[PATCH RESEND 1/6] target/riscv: Introduce extension implied rules definition
Posted by frank.chang@sifive.com 5 months, 3 weeks ago
From: Frank Chang <frank.chang@sifive.com>

RISCVCPUImpliedExtsRule is created to store the implied rules.
'is_misa' flag is used to distinguish whether the rule is derived
from the MISA or other extensions.
'ext' stores the MISA bit if 'is_misa' is true. Otherwise, it stores
the offset of the extension defined in RISCVCPUConfig. 'ext' will also
serve as the key of the hash tables to look up the rule in the following
commit.

Signed-off-by: Frank Chang <frank.chang@sifive.com>
---
 target/riscv/cpu.c |  8 ++++++++
 target/riscv/cpu.h | 18 ++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cee6fc4a9a..c7e5cec7ef 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2242,6 +2242,14 @@ RISCVCPUProfile *riscv_profiles[] = {
     NULL,
 };
 
+RISCVCPUImpliedExtsRule *riscv_misa_implied_rules[] = {
+    NULL
+};
+
+RISCVCPUImpliedExtsRule *riscv_ext_implied_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 1501868008..b5a036cf27 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -122,6 +122,24 @@ typedef enum {
     EXT_STATUS_DIRTY,
 } RISCVExtStatus;
 
+typedef struct riscv_cpu_implied_exts_rule RISCVCPUImpliedExtsRule;
+
+struct riscv_cpu_implied_exts_rule {
+    /* Bitmask indicates the rule enabled status for the harts. */
+    uint64_t enabled;
+    /* True if this is a MISA implied rule. */
+    bool is_misa;
+    /* ext is MISA bit if is_misa flag is true, else extension offset. */
+    const uint32_t ext;
+    const uint32_t implied_misas;
+    const uint32_t implied_exts[];
+};
+
+extern RISCVCPUImpliedExtsRule *riscv_misa_implied_rules[];
+extern RISCVCPUImpliedExtsRule *riscv_ext_implied_rules[];
+
+#define RISCV_IMPLIED_EXTS_RULE_END -1
+
 #define MMU_USER_IDX 3
 
 #define MAX_RISCV_PMPS (16)
-- 
2.43.2
Re: [PATCH RESEND 1/6] target/riscv: Introduce extension implied rules definition
Posted by Alistair Francis 5 months, 2 weeks ago
On Wed, Jun 5, 2024 at 4:35 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> RISCVCPUImpliedExtsRule is created to store the implied rules.
> 'is_misa' flag is used to distinguish whether the rule is derived
> from the MISA or other extensions.
> 'ext' stores the MISA bit if 'is_misa' is true. Otherwise, it stores
> the offset of the extension defined in RISCVCPUConfig. 'ext' will also
> serve as the key of the hash tables to look up the rule in the following
> commit.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> ---
>  target/riscv/cpu.c |  8 ++++++++
>  target/riscv/cpu.h | 18 ++++++++++++++++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index cee6fc4a9a..c7e5cec7ef 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -2242,6 +2242,14 @@ RISCVCPUProfile *riscv_profiles[] = {
>      NULL,
>  };
>
> +RISCVCPUImpliedExtsRule *riscv_misa_implied_rules[] = {
> +    NULL
> +};
> +
> +RISCVCPUImpliedExtsRule *riscv_ext_implied_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 1501868008..b5a036cf27 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -122,6 +122,24 @@ typedef enum {
>      EXT_STATUS_DIRTY,
>  } RISCVExtStatus;
>
> +typedef struct riscv_cpu_implied_exts_rule RISCVCPUImpliedExtsRule;
> +
> +struct riscv_cpu_implied_exts_rule {
> +    /* Bitmask indicates the rule enabled status for the harts. */
> +    uint64_t enabled;

I'm not clear why we need this

Alistair

> +    /* True if this is a MISA implied rule. */
> +    bool is_misa;
> +    /* ext is MISA bit if is_misa flag is true, else extension offset. */
> +    const uint32_t ext;
> +    const uint32_t implied_misas;
> +    const uint32_t implied_exts[];
> +};
> +
> +extern RISCVCPUImpliedExtsRule *riscv_misa_implied_rules[];
> +extern RISCVCPUImpliedExtsRule *riscv_ext_implied_rules[];
> +
> +#define RISCV_IMPLIED_EXTS_RULE_END -1
> +
>  #define MMU_USER_IDX 3
>
>  #define MAX_RISCV_PMPS (16)
> --
> 2.43.2
>
>
Re: [PATCH RESEND 1/6] target/riscv: Introduce extension implied rules definition
Posted by Frank Chang 5 months, 2 weeks ago
Hi Alistair,

On Tue, Jun 11, 2024 at 9:35 AM Alistair Francis <alistair23@gmail.com>
wrote:

> On Wed, Jun 5, 2024 at 4:35 PM <frank.chang@sifive.com> wrote:
> >
> > From: Frank Chang <frank.chang@sifive.com>
> >
> > RISCVCPUImpliedExtsRule is created to store the implied rules.
> > 'is_misa' flag is used to distinguish whether the rule is derived
> > from the MISA or other extensions.
> > 'ext' stores the MISA bit if 'is_misa' is true. Otherwise, it stores
> > the offset of the extension defined in RISCVCPUConfig. 'ext' will also
> > serve as the key of the hash tables to look up the rule in the following
> > commit.
> >
> > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> > ---
> >  target/riscv/cpu.c |  8 ++++++++
> >  target/riscv/cpu.h | 18 ++++++++++++++++++
> >  2 files changed, 26 insertions(+)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index cee6fc4a9a..c7e5cec7ef 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -2242,6 +2242,14 @@ RISCVCPUProfile *riscv_profiles[] = {
> >      NULL,
> >  };
> >
> > +RISCVCPUImpliedExtsRule *riscv_misa_implied_rules[] = {
> > +    NULL
> > +};
> > +
> > +RISCVCPUImpliedExtsRule *riscv_ext_implied_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 1501868008..b5a036cf27 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -122,6 +122,24 @@ typedef enum {
> >      EXT_STATUS_DIRTY,
> >  } RISCVExtStatus;
> >
> > +typedef struct riscv_cpu_implied_exts_rule RISCVCPUImpliedExtsRule;
> > +
> > +struct riscv_cpu_implied_exts_rule {
> > +    /* Bitmask indicates the rule enabled status for the harts. */
> > +    uint64_t enabled;
>
> I'm not clear why we need this
>

This is because a rule may be implied more than once.
e.g. Zcf implies RVF, Zfa also implies RVF.
There's no need to check RVF's implied rule again for Zfa after Zcf's
implied rules are enabled.

The implied rules are checked recursively, so once the rule has been
enabled (per-CPU basis),
the rule (and all its implied rules) will not be rechecked.

Regards,
Frank Chang


> Alistair
>
> > +    /* True if this is a MISA implied rule. */
> > +    bool is_misa;
> > +    /* ext is MISA bit if is_misa flag is true, else extension offset.
> */
> > +    const uint32_t ext;
> > +    const uint32_t implied_misas;
> > +    const uint32_t implied_exts[];
> > +};
> > +
> > +extern RISCVCPUImpliedExtsRule *riscv_misa_implied_rules[];
> > +extern RISCVCPUImpliedExtsRule *riscv_ext_implied_rules[];
> > +
> > +#define RISCV_IMPLIED_EXTS_RULE_END -1
> > +
> >  #define MMU_USER_IDX 3
> >
> >  #define MAX_RISCV_PMPS (16)
> > --
> > 2.43.2
> >
> >
>