All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
@ 2022-04-15  9:37 ` frank.chang
  0 siblings, 0 replies; 16+ messages in thread
From: frank.chang @ 2022-04-15  9:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, Frank Chang, Bin Meng, Jim Shu, Palmer Dabbelt,
	Alistair Francis

From: Frank Chang <frank.chang@sifive.com>

Allow user to set core's marchid, mvendorid, mipid CSRs through
-cpu command line option.

Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Jim Shu <jim.shu@sifive.com>
---
 target/riscv/cpu.c |  4 ++++
 target/riscv/cpu.h |  4 ++++
 target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
 3 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ddda4906ff..2eea0f9be7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
     DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
     DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
 
+    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
+    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
+    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),
+
     DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
     DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
     DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index c069fe85fa..3ab92deb4b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -370,6 +370,10 @@ struct RISCVCPUConfig {
     bool ext_zve32f;
     bool ext_zve64f;
 
+    uint32_t mvendorid;
+    uint64_t marchid;
+    uint64_t mipid;
+
     /* Vendor-specific custom extensions */
     bool ext_XVentanaCondOps;
 
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 341c2e6f23..9a02038adb 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState *env, int csrno,
     return RISCV_EXCP_NONE;
 }
 
+static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
+                                     target_ulong *val)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    *val = cpu->cfg.mvendorid;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_marchid(CPURISCVState *env, int csrno,
+                                   target_ulong *val)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    *val = cpu->cfg.marchid;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_mipid(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    *val = cpu->cfg.mipid;
+    return RISCV_EXCP_NONE;
+}
+
 static RISCVException read_mhartid(CPURISCVState *env, int csrno,
                                    target_ulong *val)
 {
@@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
 
     /* Machine Information Registers */
-    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
-    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
-    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
-    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
+    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
+    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
+    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
+    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
 
     /* Machine Trap Setup */
     [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,     write_mstatus, NULL,
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
@ 2022-04-15  9:37 ` frank.chang
  0 siblings, 0 replies; 16+ messages in thread
From: frank.chang @ 2022-04-15  9:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-riscv, Frank Chang, Jim Shu, Palmer Dabbelt,
	Alistair Francis, Bin Meng

From: Frank Chang <frank.chang@sifive.com>

Allow user to set core's marchid, mvendorid, mipid CSRs through
-cpu command line option.

Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Jim Shu <jim.shu@sifive.com>
---
 target/riscv/cpu.c |  4 ++++
 target/riscv/cpu.h |  4 ++++
 target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
 3 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ddda4906ff..2eea0f9be7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
     DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
     DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
 
+    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
+    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
+    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),
+
     DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
     DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
     DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index c069fe85fa..3ab92deb4b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -370,6 +370,10 @@ struct RISCVCPUConfig {
     bool ext_zve32f;
     bool ext_zve64f;
 
+    uint32_t mvendorid;
+    uint64_t marchid;
+    uint64_t mipid;
+
     /* Vendor-specific custom extensions */
     bool ext_XVentanaCondOps;
 
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 341c2e6f23..9a02038adb 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState *env, int csrno,
     return RISCV_EXCP_NONE;
 }
 
+static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
+                                     target_ulong *val)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    *val = cpu->cfg.mvendorid;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_marchid(CPURISCVState *env, int csrno,
+                                   target_ulong *val)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    *val = cpu->cfg.marchid;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_mipid(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+
+    *val = cpu->cfg.mipid;
+    return RISCV_EXCP_NONE;
+}
+
 static RISCVException read_mhartid(CPURISCVState *env, int csrno,
                                    target_ulong *val)
 {
@@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
 
     /* Machine Information Registers */
-    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
-    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
-    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
-    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
+    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
+    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
+    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
+    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
 
     /* Machine Trap Setup */
     [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,     write_mstatus, NULL,
-- 
2.35.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
  2022-04-15  9:37 ` frank.chang
@ 2022-04-19  5:21   ` Alistair Francis
  -1 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2022-04-19  5:21 UTC (permalink / raw)
  To: Frank Chang
  Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
	Jim Shu, Palmer Dabbelt, Alistair Francis

On Fri, Apr 15, 2022 at 7:37 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> Allow user to set core's marchid, mvendorid, mipid CSRs through
> -cpu command line option.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Reviewed-by: Jim Shu <jim.shu@sifive.com>
> ---
>  target/riscv/cpu.c |  4 ++++
>  target/riscv/cpu.h |  4 ++++
>  target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
>  3 files changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ddda4906ff..2eea0f9be7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
>      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>
> +    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
> +    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
> +    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),

Should we have non-zero defaults here?

Alistair

> +
>      DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>      DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>      DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index c069fe85fa..3ab92deb4b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -370,6 +370,10 @@ struct RISCVCPUConfig {
>      bool ext_zve32f;
>      bool ext_zve64f;
>
> +    uint32_t mvendorid;
> +    uint64_t marchid;
> +    uint64_t mipid;
> +
>      /* Vendor-specific custom extensions */
>      bool ext_XVentanaCondOps;
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 341c2e6f23..9a02038adb 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState *env, int csrno,
>      return RISCV_EXCP_NONE;
>  }
>
> +static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
> +                                     target_ulong *val)
> +{
> +    CPUState *cs = env_cpu(env);
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +
> +    *val = cpu->cfg.mvendorid;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_marchid(CPURISCVState *env, int csrno,
> +                                   target_ulong *val)
> +{
> +    CPUState *cs = env_cpu(env);
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +
> +    *val = cpu->cfg.marchid;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_mipid(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    CPUState *cs = env_cpu(env);
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +
> +    *val = cpu->cfg.mipid;
> +    return RISCV_EXCP_NONE;
> +}
> +
>  static RISCVException read_mhartid(CPURISCVState *env, int csrno,
>                                     target_ulong *val)
>  {
> @@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
>
>      /* Machine Information Registers */
> -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
> -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
> -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
> -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
> +    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
> +    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
> +    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
> +    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
>
>      /* Machine Trap Setup */
>      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,     write_mstatus, NULL,
> --
> 2.35.1
>
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
@ 2022-04-19  5:21   ` Alistair Francis
  0 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2022-04-19  5:21 UTC (permalink / raw)
  To: Frank Chang
  Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Bin Meng,
	Jim Shu, Palmer Dabbelt, Alistair Francis

On Fri, Apr 15, 2022 at 7:37 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> Allow user to set core's marchid, mvendorid, mipid CSRs through
> -cpu command line option.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Reviewed-by: Jim Shu <jim.shu@sifive.com>
> ---
>  target/riscv/cpu.c |  4 ++++
>  target/riscv/cpu.h |  4 ++++
>  target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
>  3 files changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ddda4906ff..2eea0f9be7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
>      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>
> +    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
> +    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
> +    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),

Should we have non-zero defaults here?

Alistair

> +
>      DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>      DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>      DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index c069fe85fa..3ab92deb4b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -370,6 +370,10 @@ struct RISCVCPUConfig {
>      bool ext_zve32f;
>      bool ext_zve64f;
>
> +    uint32_t mvendorid;
> +    uint64_t marchid;
> +    uint64_t mipid;
> +
>      /* Vendor-specific custom extensions */
>      bool ext_XVentanaCondOps;
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 341c2e6f23..9a02038adb 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState *env, int csrno,
>      return RISCV_EXCP_NONE;
>  }
>
> +static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
> +                                     target_ulong *val)
> +{
> +    CPUState *cs = env_cpu(env);
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +
> +    *val = cpu->cfg.mvendorid;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_marchid(CPURISCVState *env, int csrno,
> +                                   target_ulong *val)
> +{
> +    CPUState *cs = env_cpu(env);
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +
> +    *val = cpu->cfg.marchid;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_mipid(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    CPUState *cs = env_cpu(env);
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +
> +    *val = cpu->cfg.mipid;
> +    return RISCV_EXCP_NONE;
> +}
> +
>  static RISCVException read_mhartid(CPURISCVState *env, int csrno,
>                                     target_ulong *val)
>  {
> @@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
>
>      /* Machine Information Registers */
> -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
> -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
> -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
> -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
> +    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
> +    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
> +    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
> +    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
>
>      /* Machine Trap Setup */
>      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,     write_mstatus, NULL,
> --
> 2.35.1
>
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
  2022-04-19  5:21   ` Alistair Francis
@ 2022-04-19  5:27     ` Anup Patel
  -1 siblings, 0 replies; 16+ messages in thread
From: Anup Patel @ 2022-04-19  5:27 UTC (permalink / raw)
  To: Alistair Francis
  Cc: open list:RISC-V, Frank Chang, Bin Meng,
	qemu-devel@nongnu.org Developers, Jim Shu, Palmer Dabbelt,
	Alistair Francis

On Tue, Apr 19, 2022 at 10:52 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Fri, Apr 15, 2022 at 7:37 PM <frank.chang@sifive.com> wrote:
> >
> > From: Frank Chang <frank.chang@sifive.com>
> >
> > Allow user to set core's marchid, mvendorid, mipid CSRs through
> > -cpu command line option.
> >
> > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> > Reviewed-by: Jim Shu <jim.shu@sifive.com>
> > ---
> >  target/riscv/cpu.c |  4 ++++
> >  target/riscv/cpu.h |  4 ++++
> >  target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
> >  3 files changed, 42 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index ddda4906ff..2eea0f9be7 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
> >      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
> >      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
> >
> > +    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
> > +    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
> > +    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),
>
> Should we have non-zero defaults here?

To do that, we need mvendorid for QEMU RISC-V.

The marchid and mipid can be based on the QEMU version number.

Regards,
Anup

>
> Alistair
>
> > +
> >      DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
> >      DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
> >      DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index c069fe85fa..3ab92deb4b 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -370,6 +370,10 @@ struct RISCVCPUConfig {
> >      bool ext_zve32f;
> >      bool ext_zve64f;
> >
> > +    uint32_t mvendorid;
> > +    uint64_t marchid;
> > +    uint64_t mipid;
> > +
> >      /* Vendor-specific custom extensions */
> >      bool ext_XVentanaCondOps;
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 341c2e6f23..9a02038adb 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState *env, int csrno,
> >      return RISCV_EXCP_NONE;
> >  }
> >
> > +static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
> > +                                     target_ulong *val)
> > +{
> > +    CPUState *cs = env_cpu(env);
> > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > +
> > +    *val = cpu->cfg.mvendorid;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_marchid(CPURISCVState *env, int csrno,
> > +                                   target_ulong *val)
> > +{
> > +    CPUState *cs = env_cpu(env);
> > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > +
> > +    *val = cpu->cfg.marchid;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_mipid(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    CPUState *cs = env_cpu(env);
> > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > +
> > +    *val = cpu->cfg.mipid;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> >  static RISCVException read_mhartid(CPURISCVState *env, int csrno,
> >                                     target_ulong *val)
> >  {
> > @@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> >      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
> >
> >      /* Machine Information Registers */
> > -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
> > -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
> > -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
> > -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
> > +    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
> > +    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
> > +    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
> > +    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
> >
> >      /* Machine Trap Setup */
> >      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,     write_mstatus, NULL,
> > --
> > 2.35.1
> >
> >
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
@ 2022-04-19  5:27     ` Anup Patel
  0 siblings, 0 replies; 16+ messages in thread
From: Anup Patel @ 2022-04-19  5:27 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Frank Chang, open list:RISC-V, Bin Meng,
	qemu-devel@nongnu.org Developers, Jim Shu, Palmer Dabbelt,
	Alistair Francis

On Tue, Apr 19, 2022 at 10:52 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Fri, Apr 15, 2022 at 7:37 PM <frank.chang@sifive.com> wrote:
> >
> > From: Frank Chang <frank.chang@sifive.com>
> >
> > Allow user to set core's marchid, mvendorid, mipid CSRs through
> > -cpu command line option.
> >
> > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> > Reviewed-by: Jim Shu <jim.shu@sifive.com>
> > ---
> >  target/riscv/cpu.c |  4 ++++
> >  target/riscv/cpu.h |  4 ++++
> >  target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
> >  3 files changed, 42 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index ddda4906ff..2eea0f9be7 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
> >      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
> >      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
> >
> > +    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
> > +    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
> > +    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),
>
> Should we have non-zero defaults here?

To do that, we need mvendorid for QEMU RISC-V.

The marchid and mipid can be based on the QEMU version number.

Regards,
Anup

>
> Alistair
>
> > +
> >      DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
> >      DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
> >      DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index c069fe85fa..3ab92deb4b 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -370,6 +370,10 @@ struct RISCVCPUConfig {
> >      bool ext_zve32f;
> >      bool ext_zve64f;
> >
> > +    uint32_t mvendorid;
> > +    uint64_t marchid;
> > +    uint64_t mipid;
> > +
> >      /* Vendor-specific custom extensions */
> >      bool ext_XVentanaCondOps;
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 341c2e6f23..9a02038adb 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState *env, int csrno,
> >      return RISCV_EXCP_NONE;
> >  }
> >
> > +static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
> > +                                     target_ulong *val)
> > +{
> > +    CPUState *cs = env_cpu(env);
> > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > +
> > +    *val = cpu->cfg.mvendorid;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_marchid(CPURISCVState *env, int csrno,
> > +                                   target_ulong *val)
> > +{
> > +    CPUState *cs = env_cpu(env);
> > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > +
> > +    *val = cpu->cfg.marchid;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_mipid(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    CPUState *cs = env_cpu(env);
> > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > +
> > +    *val = cpu->cfg.mipid;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> >  static RISCVException read_mhartid(CPURISCVState *env, int csrno,
> >                                     target_ulong *val)
> >  {
> > @@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> >      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
> >
> >      /* Machine Information Registers */
> > -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
> > -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
> > -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
> > -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
> > +    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
> > +    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
> > +    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
> > +    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
> >
> >      /* Machine Trap Setup */
> >      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,     write_mstatus, NULL,
> > --
> > 2.35.1
> >
> >
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
  2022-04-19  5:27     ` Anup Patel
@ 2022-04-19  6:00       ` Frank Chang
  -1 siblings, 0 replies; 16+ messages in thread
From: Frank Chang @ 2022-04-19  6:00 UTC (permalink / raw)
  To: Anup Patel
  Cc: Alistair Francis, open list:RISC-V, Bin Meng,
	qemu-devel@nongnu.org Developers, Jim Shu, Palmer Dabbelt,
	Alistair Francis

[-- Attachment #1: Type: text/plain, Size: 5191 bytes --]

On Tue, Apr 19, 2022 at 1:27 PM Anup Patel <apatel@ventanamicro.com> wrote:

> On Tue, Apr 19, 2022 at 10:52 AM Alistair Francis <alistair23@gmail.com>
> wrote:
> >
> > On Fri, Apr 15, 2022 at 7:37 PM <frank.chang@sifive.com> wrote:
> > >
> > > From: Frank Chang <frank.chang@sifive.com>
> > >
> > > Allow user to set core's marchid, mvendorid, mipid CSRs through
> > > -cpu command line option.
> > >
> > > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> > > Reviewed-by: Jim Shu <jim.shu@sifive.com>
> > > ---
> > >  target/riscv/cpu.c |  4 ++++
> > >  target/riscv/cpu.h |  4 ++++
> > >  target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
> > >  3 files changed, 42 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > index ddda4906ff..2eea0f9be7 100644
> > > --- a/target/riscv/cpu.c
> > > +++ b/target/riscv/cpu.c
> > > @@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
> > >      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
> > >      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
> > >
> > > +    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
> > > +    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
> > > +    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),
> >
> > Should we have non-zero defaults here?
>
> To do that, we need mvendorid for QEMU RISC-V.
>
> The marchid and mipid can be based on the QEMU version number.
>
> Regards,
> Anup
>

The original intention for this patch is to allow users to define
their own $mvendorid, $marchid, and $mipid through the command line
when they initiate QEMU.

If we want to provide the default values for QEMU RISC-V CPU,
just like what Anup said.
We need to define our own mvendorid, which should be a JEDEC manufacturer
ID.
(Perhaps it's fine to just give some random legal JEDEC manufacturer ID
as I don't think we would really want to spend the money on that.)

For $marchid and $mipid,
I agree that it could base on QEMU's version from the QEMU_FULL_VERSION
macro.
(and $marchid should have MSB set to 1 for open-source projects.)

Regards,
Frank Chang


>
> >
> > Alistair
> >
> > > +
> > >      DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
> > >      DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
> > >      DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > > index c069fe85fa..3ab92deb4b 100644
> > > --- a/target/riscv/cpu.h
> > > +++ b/target/riscv/cpu.h
> > > @@ -370,6 +370,10 @@ struct RISCVCPUConfig {
> > >      bool ext_zve32f;
> > >      bool ext_zve64f;
> > >
> > > +    uint32_t mvendorid;
> > > +    uint64_t marchid;
> > > +    uint64_t mipid;
> > > +
> > >      /* Vendor-specific custom extensions */
> > >      bool ext_XVentanaCondOps;
> > >
> > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > > index 341c2e6f23..9a02038adb 100644
> > > --- a/target/riscv/csr.c
> > > +++ b/target/riscv/csr.c
> > > @@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState
> *env, int csrno,
> > >      return RISCV_EXCP_NONE;
> > >  }
> > >
> > > +static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
> > > +                                     target_ulong *val)
> > > +{
> > > +    CPUState *cs = env_cpu(env);
> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > > +
> > > +    *val = cpu->cfg.mvendorid;
> > > +    return RISCV_EXCP_NONE;
> > > +}
> > > +
> > > +static RISCVException read_marchid(CPURISCVState *env, int csrno,
> > > +                                   target_ulong *val)
> > > +{
> > > +    CPUState *cs = env_cpu(env);
> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > > +
> > > +    *val = cpu->cfg.marchid;
> > > +    return RISCV_EXCP_NONE;
> > > +}
> > > +
> > > +static RISCVException read_mipid(CPURISCVState *env, int csrno,
> > > +                                 target_ulong *val)
> > > +{
> > > +    CPUState *cs = env_cpu(env);
> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > > +
> > > +    *val = cpu->cfg.mipid;
> > > +    return RISCV_EXCP_NONE;
> > > +}
> > > +
> > >  static RISCVException read_mhartid(CPURISCVState *env, int csrno,
> > >                                     target_ulong *val)
> > >  {
> > > @@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] =
> {
> > >      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
> > >
> > >      /* Machine Information Registers */
> > > -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
> > > -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
> > > -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
> > > -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
> > > +    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
> > > +    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
> > > +    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
> > > +    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
> > >
> > >      /* Machine Trap Setup */
> > >      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,
>  write_mstatus, NULL,
> > > --
> > > 2.35.1
> > >
> > >
> >
>

[-- Attachment #2: Type: text/html, Size: 7563 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
@ 2022-04-19  6:00       ` Frank Chang
  0 siblings, 0 replies; 16+ messages in thread
From: Frank Chang @ 2022-04-19  6:00 UTC (permalink / raw)
  To: Anup Patel
  Cc: Alistair Francis, open list:RISC-V, Bin Meng,
	qemu-devel@nongnu.org Developers, Jim Shu, Palmer Dabbelt,
	Alistair Francis

[-- Attachment #1: Type: text/plain, Size: 5191 bytes --]

On Tue, Apr 19, 2022 at 1:27 PM Anup Patel <apatel@ventanamicro.com> wrote:

> On Tue, Apr 19, 2022 at 10:52 AM Alistair Francis <alistair23@gmail.com>
> wrote:
> >
> > On Fri, Apr 15, 2022 at 7:37 PM <frank.chang@sifive.com> wrote:
> > >
> > > From: Frank Chang <frank.chang@sifive.com>
> > >
> > > Allow user to set core's marchid, mvendorid, mipid CSRs through
> > > -cpu command line option.
> > >
> > > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> > > Reviewed-by: Jim Shu <jim.shu@sifive.com>
> > > ---
> > >  target/riscv/cpu.c |  4 ++++
> > >  target/riscv/cpu.h |  4 ++++
> > >  target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
> > >  3 files changed, 42 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > index ddda4906ff..2eea0f9be7 100644
> > > --- a/target/riscv/cpu.c
> > > +++ b/target/riscv/cpu.c
> > > @@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
> > >      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
> > >      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
> > >
> > > +    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
> > > +    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
> > > +    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),
> >
> > Should we have non-zero defaults here?
>
> To do that, we need mvendorid for QEMU RISC-V.
>
> The marchid and mipid can be based on the QEMU version number.
>
> Regards,
> Anup
>

The original intention for this patch is to allow users to define
their own $mvendorid, $marchid, and $mipid through the command line
when they initiate QEMU.

If we want to provide the default values for QEMU RISC-V CPU,
just like what Anup said.
We need to define our own mvendorid, which should be a JEDEC manufacturer
ID.
(Perhaps it's fine to just give some random legal JEDEC manufacturer ID
as I don't think we would really want to spend the money on that.)

For $marchid and $mipid,
I agree that it could base on QEMU's version from the QEMU_FULL_VERSION
macro.
(and $marchid should have MSB set to 1 for open-source projects.)

Regards,
Frank Chang


>
> >
> > Alistair
> >
> > > +
> > >      DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
> > >      DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
> > >      DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > > index c069fe85fa..3ab92deb4b 100644
> > > --- a/target/riscv/cpu.h
> > > +++ b/target/riscv/cpu.h
> > > @@ -370,6 +370,10 @@ struct RISCVCPUConfig {
> > >      bool ext_zve32f;
> > >      bool ext_zve64f;
> > >
> > > +    uint32_t mvendorid;
> > > +    uint64_t marchid;
> > > +    uint64_t mipid;
> > > +
> > >      /* Vendor-specific custom extensions */
> > >      bool ext_XVentanaCondOps;
> > >
> > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > > index 341c2e6f23..9a02038adb 100644
> > > --- a/target/riscv/csr.c
> > > +++ b/target/riscv/csr.c
> > > @@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState
> *env, int csrno,
> > >      return RISCV_EXCP_NONE;
> > >  }
> > >
> > > +static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
> > > +                                     target_ulong *val)
> > > +{
> > > +    CPUState *cs = env_cpu(env);
> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > > +
> > > +    *val = cpu->cfg.mvendorid;
> > > +    return RISCV_EXCP_NONE;
> > > +}
> > > +
> > > +static RISCVException read_marchid(CPURISCVState *env, int csrno,
> > > +                                   target_ulong *val)
> > > +{
> > > +    CPUState *cs = env_cpu(env);
> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > > +
> > > +    *val = cpu->cfg.marchid;
> > > +    return RISCV_EXCP_NONE;
> > > +}
> > > +
> > > +static RISCVException read_mipid(CPURISCVState *env, int csrno,
> > > +                                 target_ulong *val)
> > > +{
> > > +    CPUState *cs = env_cpu(env);
> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
> > > +
> > > +    *val = cpu->cfg.mipid;
> > > +    return RISCV_EXCP_NONE;
> > > +}
> > > +
> > >  static RISCVException read_mhartid(CPURISCVState *env, int csrno,
> > >                                     target_ulong *val)
> > >  {
> > > @@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] =
> {
> > >      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
> > >
> > >      /* Machine Information Registers */
> > > -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
> > > -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
> > > -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
> > > -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
> > > +    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
> > > +    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
> > > +    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
> > > +    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
> > >
> > >      /* Machine Trap Setup */
> > >      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,
>  write_mstatus, NULL,
> > > --
> > > 2.35.1
> > >
> > >
> >
>

[-- Attachment #2: Type: text/html, Size: 7563 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
  2022-04-19  6:00       ` Frank Chang
@ 2022-04-19  7:04         ` Frank Chang
  -1 siblings, 0 replies; 16+ messages in thread
From: Frank Chang @ 2022-04-19  7:04 UTC (permalink / raw)
  To: Anup Patel
  Cc: Alistair Francis, open list:RISC-V, Bin Meng,
	qemu-devel@nongnu.org Developers, Jim Shu, Palmer Dabbelt,
	Alistair Francis

[-- Attachment #1: Type: text/plain, Size: 5704 bytes --]

On Tue, Apr 19, 2022 at 2:00 PM Frank Chang <frank.chang@sifive.com> wrote:

> On Tue, Apr 19, 2022 at 1:27 PM Anup Patel <apatel@ventanamicro.com>
> wrote:
>
>> On Tue, Apr 19, 2022 at 10:52 AM Alistair Francis <alistair23@gmail.com>
>> wrote:
>> >
>> > On Fri, Apr 15, 2022 at 7:37 PM <frank.chang@sifive.com> wrote:
>> > >
>> > > From: Frank Chang <frank.chang@sifive.com>
>> > >
>> > > Allow user to set core's marchid, mvendorid, mipid CSRs through
>> > > -cpu command line option.
>> > >
>> > > Signed-off-by: Frank Chang <frank.chang@sifive.com>
>> > > Reviewed-by: Jim Shu <jim.shu@sifive.com>
>> > > ---
>> > >  target/riscv/cpu.c |  4 ++++
>> > >  target/riscv/cpu.h |  4 ++++
>> > >  target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
>> > >  3 files changed, 42 insertions(+), 4 deletions(-)
>> > >
>> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> > > index ddda4906ff..2eea0f9be7 100644
>> > > --- a/target/riscv/cpu.c
>> > > +++ b/target/riscv/cpu.c
>> > > @@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
>> > >      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>> > >      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>> > >
>> > > +    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
>> > > +    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
>> > > +    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),
>> >
>> > Should we have non-zero defaults here?
>>
>> To do that, we need mvendorid for QEMU RISC-V.
>>
>> The marchid and mipid can be based on the QEMU version number.
>>
>> Regards,
>> Anup
>>
>
> The original intention for this patch is to allow users to define
> their own $mvendorid, $marchid, and $mipid through the command line
> when they initiate QEMU.
>
> If we want to provide the default values for QEMU RISC-V CPU,
> just like what Anup said.
> We need to define our own mvendorid, which should be a JEDEC manufacturer
> ID.
> (Perhaps it's fine to just give some random legal JEDEC manufacturer ID
> as I don't think we would really want to spend the money on that.)
>
> For $marchid and $mipid,
> I agree that it could base on QEMU's version from the QEMU_FULL_VERSION
> macro.
> (and $marchid should have MSB set to 1 for open-source projects.)
>
> Regards,
> Frank Chang
>

Another simpler way is to stick with the current approach
and leave $mvendorid, $marchid and $mipid default to 0.
Which is still legal as RISC-V privilege spec says:
"A value of 0 can be returned to indicate the field is not implemented."

Regards,
Frank Chang


>
>
>>
>> >
>> > Alistair
>> >
>> > > +
>> > >      DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>> > >      DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>> > >      DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
>> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> > > index c069fe85fa..3ab92deb4b 100644
>> > > --- a/target/riscv/cpu.h
>> > > +++ b/target/riscv/cpu.h
>> > > @@ -370,6 +370,10 @@ struct RISCVCPUConfig {
>> > >      bool ext_zve32f;
>> > >      bool ext_zve64f;
>> > >
>> > > +    uint32_t mvendorid;
>> > > +    uint64_t marchid;
>> > > +    uint64_t mipid;
>> > > +
>> > >      /* Vendor-specific custom extensions */
>> > >      bool ext_XVentanaCondOps;
>> > >
>> > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> > > index 341c2e6f23..9a02038adb 100644
>> > > --- a/target/riscv/csr.c
>> > > +++ b/target/riscv/csr.c
>> > > @@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState
>> *env, int csrno,
>> > >      return RISCV_EXCP_NONE;
>> > >  }
>> > >
>> > > +static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
>> > > +                                     target_ulong *val)
>> > > +{
>> > > +    CPUState *cs = env_cpu(env);
>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>> > > +
>> > > +    *val = cpu->cfg.mvendorid;
>> > > +    return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > > +static RISCVException read_marchid(CPURISCVState *env, int csrno,
>> > > +                                   target_ulong *val)
>> > > +{
>> > > +    CPUState *cs = env_cpu(env);
>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>> > > +
>> > > +    *val = cpu->cfg.marchid;
>> > > +    return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > > +static RISCVException read_mipid(CPURISCVState *env, int csrno,
>> > > +                                 target_ulong *val)
>> > > +{
>> > > +    CPUState *cs = env_cpu(env);
>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>> > > +
>> > > +    *val = cpu->cfg.mipid;
>> > > +    return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > >  static RISCVException read_mhartid(CPURISCVState *env, int csrno,
>> > >                                     target_ulong *val)
>> > >  {
>> > > @@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE]
>> = {
>> > >      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
>> > >
>> > >      /* Machine Information Registers */
>> > > -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
>> > > -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
>> > > -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
>> > > -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
>> > > +    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
>> > > +    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
>> > > +    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
>> > > +    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
>> > >
>> > >      /* Machine Trap Setup */
>> > >      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,
>>  write_mstatus, NULL,
>> > > --
>> > > 2.35.1
>> > >
>> > >
>> >
>>
>

[-- Attachment #2: Type: text/html, Size: 8476 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
@ 2022-04-19  7:04         ` Frank Chang
  0 siblings, 0 replies; 16+ messages in thread
From: Frank Chang @ 2022-04-19  7:04 UTC (permalink / raw)
  To: Anup Patel
  Cc: Alistair Francis, open list:RISC-V, Bin Meng,
	qemu-devel@nongnu.org Developers, Jim Shu, Palmer Dabbelt,
	Alistair Francis

[-- Attachment #1: Type: text/plain, Size: 5704 bytes --]

On Tue, Apr 19, 2022 at 2:00 PM Frank Chang <frank.chang@sifive.com> wrote:

> On Tue, Apr 19, 2022 at 1:27 PM Anup Patel <apatel@ventanamicro.com>
> wrote:
>
>> On Tue, Apr 19, 2022 at 10:52 AM Alistair Francis <alistair23@gmail.com>
>> wrote:
>> >
>> > On Fri, Apr 15, 2022 at 7:37 PM <frank.chang@sifive.com> wrote:
>> > >
>> > > From: Frank Chang <frank.chang@sifive.com>
>> > >
>> > > Allow user to set core's marchid, mvendorid, mipid CSRs through
>> > > -cpu command line option.
>> > >
>> > > Signed-off-by: Frank Chang <frank.chang@sifive.com>
>> > > Reviewed-by: Jim Shu <jim.shu@sifive.com>
>> > > ---
>> > >  target/riscv/cpu.c |  4 ++++
>> > >  target/riscv/cpu.h |  4 ++++
>> > >  target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
>> > >  3 files changed, 42 insertions(+), 4 deletions(-)
>> > >
>> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> > > index ddda4906ff..2eea0f9be7 100644
>> > > --- a/target/riscv/cpu.c
>> > > +++ b/target/riscv/cpu.c
>> > > @@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
>> > >      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>> > >      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>> > >
>> > > +    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
>> > > +    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
>> > > +    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),
>> >
>> > Should we have non-zero defaults here?
>>
>> To do that, we need mvendorid for QEMU RISC-V.
>>
>> The marchid and mipid can be based on the QEMU version number.
>>
>> Regards,
>> Anup
>>
>
> The original intention for this patch is to allow users to define
> their own $mvendorid, $marchid, and $mipid through the command line
> when they initiate QEMU.
>
> If we want to provide the default values for QEMU RISC-V CPU,
> just like what Anup said.
> We need to define our own mvendorid, which should be a JEDEC manufacturer
> ID.
> (Perhaps it's fine to just give some random legal JEDEC manufacturer ID
> as I don't think we would really want to spend the money on that.)
>
> For $marchid and $mipid,
> I agree that it could base on QEMU's version from the QEMU_FULL_VERSION
> macro.
> (and $marchid should have MSB set to 1 for open-source projects.)
>
> Regards,
> Frank Chang
>

Another simpler way is to stick with the current approach
and leave $mvendorid, $marchid and $mipid default to 0.
Which is still legal as RISC-V privilege spec says:
"A value of 0 can be returned to indicate the field is not implemented."

Regards,
Frank Chang


>
>
>>
>> >
>> > Alistair
>> >
>> > > +
>> > >      DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>> > >      DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>> > >      DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
>> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> > > index c069fe85fa..3ab92deb4b 100644
>> > > --- a/target/riscv/cpu.h
>> > > +++ b/target/riscv/cpu.h
>> > > @@ -370,6 +370,10 @@ struct RISCVCPUConfig {
>> > >      bool ext_zve32f;
>> > >      bool ext_zve64f;
>> > >
>> > > +    uint32_t mvendorid;
>> > > +    uint64_t marchid;
>> > > +    uint64_t mipid;
>> > > +
>> > >      /* Vendor-specific custom extensions */
>> > >      bool ext_XVentanaCondOps;
>> > >
>> > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> > > index 341c2e6f23..9a02038adb 100644
>> > > --- a/target/riscv/csr.c
>> > > +++ b/target/riscv/csr.c
>> > > @@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState
>> *env, int csrno,
>> > >      return RISCV_EXCP_NONE;
>> > >  }
>> > >
>> > > +static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
>> > > +                                     target_ulong *val)
>> > > +{
>> > > +    CPUState *cs = env_cpu(env);
>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>> > > +
>> > > +    *val = cpu->cfg.mvendorid;
>> > > +    return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > > +static RISCVException read_marchid(CPURISCVState *env, int csrno,
>> > > +                                   target_ulong *val)
>> > > +{
>> > > +    CPUState *cs = env_cpu(env);
>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>> > > +
>> > > +    *val = cpu->cfg.marchid;
>> > > +    return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > > +static RISCVException read_mipid(CPURISCVState *env, int csrno,
>> > > +                                 target_ulong *val)
>> > > +{
>> > > +    CPUState *cs = env_cpu(env);
>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>> > > +
>> > > +    *val = cpu->cfg.mipid;
>> > > +    return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > >  static RISCVException read_mhartid(CPURISCVState *env, int csrno,
>> > >                                     target_ulong *val)
>> > >  {
>> > > @@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE]
>> = {
>> > >      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
>> > >
>> > >      /* Machine Information Registers */
>> > > -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
>> > > -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
>> > > -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
>> > > -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
>> > > +    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
>> > > +    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
>> > > +    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
>> > > +    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
>> > >
>> > >      /* Machine Trap Setup */
>> > >      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,
>>  write_mstatus, NULL,
>> > > --
>> > > 2.35.1
>> > >
>> > >
>> >
>>
>

[-- Attachment #2: Type: text/html, Size: 8476 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
  2022-04-19  7:04         ` Frank Chang
  (?)
@ 2022-04-20  7:47         ` Alistair Francis
  2022-04-20  7:55           ` Frank Chang
  -1 siblings, 1 reply; 16+ messages in thread
From: Alistair Francis @ 2022-04-20  7:47 UTC (permalink / raw)
  To: Frank Chang
  Cc: Anup Patel, open list:RISC-V, Bin Meng,
	qemu-devel@nongnu.org Developers, Jim Shu, Palmer Dabbelt,
	Alistair Francis

On Tue, Apr 19, 2022 at 5:04 PM Frank Chang <frank.chang@sifive.com> wrote:
>
> On Tue, Apr 19, 2022 at 2:00 PM Frank Chang <frank.chang@sifive.com> wrote:
>>
>> On Tue, Apr 19, 2022 at 1:27 PM Anup Patel <apatel@ventanamicro.com> wrote:
>>>
>>> On Tue, Apr 19, 2022 at 10:52 AM Alistair Francis <alistair23@gmail.com> wrote:
>>> >
>>> > On Fri, Apr 15, 2022 at 7:37 PM <frank.chang@sifive.com> wrote:
>>> > >
>>> > > From: Frank Chang <frank.chang@sifive.com>
>>> > >
>>> > > Allow user to set core's marchid, mvendorid, mipid CSRs through
>>> > > -cpu command line option.
>>> > >
>>> > > Signed-off-by: Frank Chang <frank.chang@sifive.com>
>>> > > Reviewed-by: Jim Shu <jim.shu@sifive.com>
>>> > > ---
>>> > >  target/riscv/cpu.c |  4 ++++
>>> > >  target/riscv/cpu.h |  4 ++++
>>> > >  target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
>>> > >  3 files changed, 42 insertions(+), 4 deletions(-)
>>> > >
>>> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>> > > index ddda4906ff..2eea0f9be7 100644
>>> > > --- a/target/riscv/cpu.c
>>> > > +++ b/target/riscv/cpu.c
>>> > > @@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
>>> > >      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>>> > >      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>>> > >
>>> > > +    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
>>> > > +    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
>>> > > +    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),
>>> >
>>> > Should we have non-zero defaults here?
>>>
>>> To do that, we need mvendorid for QEMU RISC-V.
>>>
>>> The marchid and mipid can be based on the QEMU version number.
>>>
>>> Regards,
>>> Anup
>>
>>
>> The original intention for this patch is to allow users to define
>> their own $mvendorid, $marchid, and $mipid through the command line
>> when they initiate QEMU.
>>
>> If we want to provide the default values for QEMU RISC-V CPU,
>> just like what Anup said.
>> We need to define our own mvendorid, which should be a JEDEC manufacturer ID.
>> (Perhaps it's fine to just give some random legal JEDEC manufacturer ID
>> as I don't think we would really want to spend the money on that.)

This is fine as zero I think.

>>
>> For $marchid and $mipid,
>> I agree that it could base on QEMU's version from the QEMU_FULL_VERSION macro.
>> (and $marchid should have MSB set to 1 for open-source projects.)

There could be some use in setting this to the QEMU version by
default. It doesn't really get us much though, but might be useful.

I'll take this patch as is, feel free to send a new patch if you want
to add a default value

Alistair

>>
>> Regards,
>> Frank Chang
>
>
> Another simpler way is to stick with the current approach
> and leave $mvendorid, $marchid and $mipid default to 0.
> Which is still legal as RISC-V privilege spec says:
> "A value of 0 can be returned to indicate the field is not implemented."
>
> Regards,
> Frank Chang
>
>>
>>
>>>
>>>
>>> >
>>> > Alistair
>>> >
>>> > > +
>>> > >      DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>>> > >      DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>>> > >      DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
>>> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>>> > > index c069fe85fa..3ab92deb4b 100644
>>> > > --- a/target/riscv/cpu.h
>>> > > +++ b/target/riscv/cpu.h
>>> > > @@ -370,6 +370,10 @@ struct RISCVCPUConfig {
>>> > >      bool ext_zve32f;
>>> > >      bool ext_zve64f;
>>> > >
>>> > > +    uint32_t mvendorid;
>>> > > +    uint64_t marchid;
>>> > > +    uint64_t mipid;
>>> > > +
>>> > >      /* Vendor-specific custom extensions */
>>> > >      bool ext_XVentanaCondOps;
>>> > >
>>> > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>>> > > index 341c2e6f23..9a02038adb 100644
>>> > > --- a/target/riscv/csr.c
>>> > > +++ b/target/riscv/csr.c
>>> > > @@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState *env, int csrno,
>>> > >      return RISCV_EXCP_NONE;
>>> > >  }
>>> > >
>>> > > +static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
>>> > > +                                     target_ulong *val)
>>> > > +{
>>> > > +    CPUState *cs = env_cpu(env);
>>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>>> > > +
>>> > > +    *val = cpu->cfg.mvendorid;
>>> > > +    return RISCV_EXCP_NONE;
>>> > > +}
>>> > > +
>>> > > +static RISCVException read_marchid(CPURISCVState *env, int csrno,
>>> > > +                                   target_ulong *val)
>>> > > +{
>>> > > +    CPUState *cs = env_cpu(env);
>>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>>> > > +
>>> > > +    *val = cpu->cfg.marchid;
>>> > > +    return RISCV_EXCP_NONE;
>>> > > +}
>>> > > +
>>> > > +static RISCVException read_mipid(CPURISCVState *env, int csrno,
>>> > > +                                 target_ulong *val)
>>> > > +{
>>> > > +    CPUState *cs = env_cpu(env);
>>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>>> > > +
>>> > > +    *val = cpu->cfg.mipid;
>>> > > +    return RISCV_EXCP_NONE;
>>> > > +}
>>> > > +
>>> > >  static RISCVException read_mhartid(CPURISCVState *env, int csrno,
>>> > >                                     target_ulong *val)
>>> > >  {
>>> > > @@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>>> > >      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
>>> > >
>>> > >      /* Machine Information Registers */
>>> > > -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
>>> > > -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
>>> > > -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
>>> > > -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
>>> > > +    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
>>> > > +    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
>>> > > +    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
>>> > > +    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
>>> > >
>>> > >      /* Machine Trap Setup */
>>> > >      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,     write_mstatus, NULL,
>>> > > --
>>> > > 2.35.1
>>> > >
>>> > >
>>> >


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
  2022-04-15  9:37 ` frank.chang
@ 2022-04-20  7:47   ` Alistair Francis
  -1 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2022-04-20  7:47 UTC (permalink / raw)
  To: Frank Chang
  Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
	Jim Shu, Palmer Dabbelt, Alistair Francis

On Fri, Apr 15, 2022 at 7:37 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> Allow user to set core's marchid, mvendorid, mipid CSRs through
> -cpu command line option.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Reviewed-by: Jim Shu <jim.shu@sifive.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.c |  4 ++++
>  target/riscv/cpu.h |  4 ++++
>  target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
>  3 files changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ddda4906ff..2eea0f9be7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
>      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>
> +    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
> +    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
> +    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),
> +
>      DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>      DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>      DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index c069fe85fa..3ab92deb4b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -370,6 +370,10 @@ struct RISCVCPUConfig {
>      bool ext_zve32f;
>      bool ext_zve64f;
>
> +    uint32_t mvendorid;
> +    uint64_t marchid;
> +    uint64_t mipid;
> +
>      /* Vendor-specific custom extensions */
>      bool ext_XVentanaCondOps;
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 341c2e6f23..9a02038adb 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState *env, int csrno,
>      return RISCV_EXCP_NONE;
>  }
>
> +static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
> +                                     target_ulong *val)
> +{
> +    CPUState *cs = env_cpu(env);
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +
> +    *val = cpu->cfg.mvendorid;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_marchid(CPURISCVState *env, int csrno,
> +                                   target_ulong *val)
> +{
> +    CPUState *cs = env_cpu(env);
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +
> +    *val = cpu->cfg.marchid;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_mipid(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    CPUState *cs = env_cpu(env);
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +
> +    *val = cpu->cfg.mipid;
> +    return RISCV_EXCP_NONE;
> +}
> +
>  static RISCVException read_mhartid(CPURISCVState *env, int csrno,
>                                     target_ulong *val)
>  {
> @@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
>
>      /* Machine Information Registers */
> -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
> -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
> -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
> -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
> +    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
> +    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
> +    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
> +    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
>
>      /* Machine Trap Setup */
>      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,     write_mstatus, NULL,
> --
> 2.35.1
>
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
@ 2022-04-20  7:47   ` Alistair Francis
  0 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2022-04-20  7:47 UTC (permalink / raw)
  To: Frank Chang
  Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Bin Meng,
	Jim Shu, Palmer Dabbelt, Alistair Francis

On Fri, Apr 15, 2022 at 7:37 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> Allow user to set core's marchid, mvendorid, mipid CSRs through
> -cpu command line option.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Reviewed-by: Jim Shu <jim.shu@sifive.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.c |  4 ++++
>  target/riscv/cpu.h |  4 ++++
>  target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
>  3 files changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ddda4906ff..2eea0f9be7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
>      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>
> +    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
> +    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
> +    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),
> +
>      DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>      DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>      DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index c069fe85fa..3ab92deb4b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -370,6 +370,10 @@ struct RISCVCPUConfig {
>      bool ext_zve32f;
>      bool ext_zve64f;
>
> +    uint32_t mvendorid;
> +    uint64_t marchid;
> +    uint64_t mipid;
> +
>      /* Vendor-specific custom extensions */
>      bool ext_XVentanaCondOps;
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 341c2e6f23..9a02038adb 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState *env, int csrno,
>      return RISCV_EXCP_NONE;
>  }
>
> +static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
> +                                     target_ulong *val)
> +{
> +    CPUState *cs = env_cpu(env);
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +
> +    *val = cpu->cfg.mvendorid;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_marchid(CPURISCVState *env, int csrno,
> +                                   target_ulong *val)
> +{
> +    CPUState *cs = env_cpu(env);
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +
> +    *val = cpu->cfg.marchid;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_mipid(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    CPUState *cs = env_cpu(env);
> +    RISCVCPU *cpu = RISCV_CPU(cs);
> +
> +    *val = cpu->cfg.mipid;
> +    return RISCV_EXCP_NONE;
> +}
> +
>  static RISCVException read_mhartid(CPURISCVState *env, int csrno,
>                                     target_ulong *val)
>  {
> @@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
>
>      /* Machine Information Registers */
> -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
> -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
> -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
> -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
> +    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
> +    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
> +    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
> +    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
>
>      /* Machine Trap Setup */
>      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,     write_mstatus, NULL,
> --
> 2.35.1
>
>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
  2022-04-20  7:47         ` Alistair Francis
@ 2022-04-20  7:55           ` Frank Chang
  0 siblings, 0 replies; 16+ messages in thread
From: Frank Chang @ 2022-04-20  7:55 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Anup Patel, open list:RISC-V, Bin Meng,
	qemu-devel@nongnu.org Developers, Jim Shu, Palmer Dabbelt,
	Alistair Francis

[-- Attachment #1: Type: text/plain, Size: 6726 bytes --]

On Wed, Apr 20, 2022 at 3:47 PM Alistair Francis <alistair23@gmail.com>
wrote:

> On Tue, Apr 19, 2022 at 5:04 PM Frank Chang <frank.chang@sifive.com>
> wrote:
> >
> > On Tue, Apr 19, 2022 at 2:00 PM Frank Chang <frank.chang@sifive.com>
> wrote:
> >>
> >> On Tue, Apr 19, 2022 at 1:27 PM Anup Patel <apatel@ventanamicro.com>
> wrote:
> >>>
> >>> On Tue, Apr 19, 2022 at 10:52 AM Alistair Francis <
> alistair23@gmail.com> wrote:
> >>> >
> >>> > On Fri, Apr 15, 2022 at 7:37 PM <frank.chang@sifive.com> wrote:
> >>> > >
> >>> > > From: Frank Chang <frank.chang@sifive.com>
> >>> > >
> >>> > > Allow user to set core's marchid, mvendorid, mipid CSRs through
> >>> > > -cpu command line option.
> >>> > >
> >>> > > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> >>> > > Reviewed-by: Jim Shu <jim.shu@sifive.com>
> >>> > > ---
> >>> > >  target/riscv/cpu.c |  4 ++++
> >>> > >  target/riscv/cpu.h |  4 ++++
> >>> > >  target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
> >>> > >  3 files changed, 42 insertions(+), 4 deletions(-)
> >>> > >
> >>> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> >>> > > index ddda4906ff..2eea0f9be7 100644
> >>> > > --- a/target/riscv/cpu.c
> >>> > > +++ b/target/riscv/cpu.c
> >>> > > @@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
> >>> > >      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
> >>> > >      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
> >>> > >
> >>> > > +    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
> >>> > > +    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
> >>> > > +    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),
> >>> >
> >>> > Should we have non-zero defaults here?
> >>>
> >>> To do that, we need mvendorid for QEMU RISC-V.
> >>>
> >>> The marchid and mipid can be based on the QEMU version number.
> >>>
> >>> Regards,
> >>> Anup
> >>
> >>
> >> The original intention for this patch is to allow users to define
> >> their own $mvendorid, $marchid, and $mipid through the command line
> >> when they initiate QEMU.
> >>
> >> If we want to provide the default values for QEMU RISC-V CPU,
> >> just like what Anup said.
> >> We need to define our own mvendorid, which should be a JEDEC
> manufacturer ID.
> >> (Perhaps it's fine to just give some random legal JEDEC manufacturer ID
> >> as I don't think we would really want to spend the money on that.)
>
> This is fine as zero I think.
>
> >>
> >> For $marchid and $mipid,
> >> I agree that it could base on QEMU's version from the QEMU_FULL_VERSION
> macro.
> >> (and $marchid should have MSB set to 1 for open-source projects.)
>
> There could be some use in setting this to the QEMU version by
> default. It doesn't really get us much though, but might be useful.
>
> I'll take this patch as is, feel free to send a new patch if you want
> to add a default value
>
> Alistair
>

Sure, I'll set QEMU version by default in the next version patchset.

Regards,
Frank Chang


>
> >>
> >> Regards,
> >> Frank Chang
> >
> >
> > Another simpler way is to stick with the current approach
> > and leave $mvendorid, $marchid and $mipid default to 0.
> > Which is still legal as RISC-V privilege spec says:
> > "A value of 0 can be returned to indicate the field is not implemented."
> >
> > Regards,
> > Frank Chang
> >
> >>
> >>
> >>>
> >>>
> >>> >
> >>> > Alistair
> >>> >
> >>> > > +
> >>> > >      DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
> >>> > >      DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
> >>> > >      DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
> >>> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> >>> > > index c069fe85fa..3ab92deb4b 100644
> >>> > > --- a/target/riscv/cpu.h
> >>> > > +++ b/target/riscv/cpu.h
> >>> > > @@ -370,6 +370,10 @@ struct RISCVCPUConfig {
> >>> > >      bool ext_zve32f;
> >>> > >      bool ext_zve64f;
> >>> > >
> >>> > > +    uint32_t mvendorid;
> >>> > > +    uint64_t marchid;
> >>> > > +    uint64_t mipid;
> >>> > > +
> >>> > >      /* Vendor-specific custom extensions */
> >>> > >      bool ext_XVentanaCondOps;
> >>> > >
> >>> > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> >>> > > index 341c2e6f23..9a02038adb 100644
> >>> > > --- a/target/riscv/csr.c
> >>> > > +++ b/target/riscv/csr.c
> >>> > > @@ -603,6 +603,36 @@ static RISCVException
> write_ignore(CPURISCVState *env, int csrno,
> >>> > >      return RISCV_EXCP_NONE;
> >>> > >  }
> >>> > >
> >>> > > +static RISCVException read_mvendorid(CPURISCVState *env, int
> csrno,
> >>> > > +                                     target_ulong *val)
> >>> > > +{
> >>> > > +    CPUState *cs = env_cpu(env);
> >>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
> >>> > > +
> >>> > > +    *val = cpu->cfg.mvendorid;
> >>> > > +    return RISCV_EXCP_NONE;
> >>> > > +}
> >>> > > +
> >>> > > +static RISCVException read_marchid(CPURISCVState *env, int csrno,
> >>> > > +                                   target_ulong *val)
> >>> > > +{
> >>> > > +    CPUState *cs = env_cpu(env);
> >>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
> >>> > > +
> >>> > > +    *val = cpu->cfg.marchid;
> >>> > > +    return RISCV_EXCP_NONE;
> >>> > > +}
> >>> > > +
> >>> > > +static RISCVException read_mipid(CPURISCVState *env, int csrno,
> >>> > > +                                 target_ulong *val)
> >>> > > +{
> >>> > > +    CPUState *cs = env_cpu(env);
> >>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
> >>> > > +
> >>> > > +    *val = cpu->cfg.mipid;
> >>> > > +    return RISCV_EXCP_NONE;
> >>> > > +}
> >>> > > +
> >>> > >  static RISCVException read_mhartid(CPURISCVState *env, int csrno,
> >>> > >                                     target_ulong *val)
> >>> > >  {
> >>> > > @@ -3098,10 +3128,10 @@ riscv_csr_operations
> csr_ops[CSR_TABLE_SIZE] = {
> >>> > >      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
> >>> > >
> >>> > >      /* Machine Information Registers */
> >>> > > -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
> >>> > > -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
> >>> > > -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
> >>> > > -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
> >>> > > +    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
> >>> > > +    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
> >>> > > +    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
> >>> > > +    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
> >>> > >
> >>> > >      /* Machine Trap Setup */
> >>> > >      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,
>  write_mstatus, NULL,
> >>> > > --
> >>> > > 2.35.1
> >>> > >
> >>> > >
> >>> >
>

[-- Attachment #2: Type: text/html, Size: 10594 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
  2022-04-19  6:00       ` Frank Chang
@ 2022-04-20  8:32         ` Frank Chang
  -1 siblings, 0 replies; 16+ messages in thread
From: Frank Chang @ 2022-04-20  8:32 UTC (permalink / raw)
  To: Anup Patel
  Cc: Alistair Francis, open list:RISC-V, Bin Meng,
	qemu-devel@nongnu.org Developers, Jim Shu, Palmer Dabbelt,
	Alistair Francis

[-- Attachment #1: Type: text/plain, Size: 5882 bytes --]

On Tue, Apr 19, 2022 at 2:00 PM Frank Chang <frank.chang@sifive.com> wrote:

> On Tue, Apr 19, 2022 at 1:27 PM Anup Patel <apatel@ventanamicro.com>
> wrote:
>
>> On Tue, Apr 19, 2022 at 10:52 AM Alistair Francis <alistair23@gmail.com>
>> wrote:
>> >
>> > On Fri, Apr 15, 2022 at 7:37 PM <frank.chang@sifive.com> wrote:
>> > >
>> > > From: Frank Chang <frank.chang@sifive.com>
>> > >
>> > > Allow user to set core's marchid, mvendorid, mipid CSRs through
>> > > -cpu command line option.
>> > >
>> > > Signed-off-by: Frank Chang <frank.chang@sifive.com>
>> > > Reviewed-by: Jim Shu <jim.shu@sifive.com>
>> > > ---
>> > >  target/riscv/cpu.c |  4 ++++
>> > >  target/riscv/cpu.h |  4 ++++
>> > >  target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
>> > >  3 files changed, 42 insertions(+), 4 deletions(-)
>> > >
>> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> > > index ddda4906ff..2eea0f9be7 100644
>> > > --- a/target/riscv/cpu.c
>> > > +++ b/target/riscv/cpu.c
>> > > @@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
>> > >      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>> > >      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>> > >
>> > > +    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
>> > > +    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
>> > > +    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),
>> >
>> > Should we have non-zero defaults here?
>>
>> To do that, we need mvendorid for QEMU RISC-V.
>>
>> The marchid and mipid can be based on the QEMU version number.
>>
>> Regards,
>> Anup
>>
>
> The original intention for this patch is to allow users to define
> their own $mvendorid, $marchid, and $mipid through the command line
> when they initiate QEMU.
>
> If we want to provide the default values for QEMU RISC-V CPU,
> just like what Anup said.
> We need to define our own mvendorid, which should be a JEDEC manufacturer
> ID.
> (Perhaps it's fine to just give some random legal JEDEC manufacturer ID
> as I don't think we would really want to spend the money on that.)
>
> For $marchid and $mipid,
> I agree that it could base on QEMU's version from the QEMU_FULL_VERSION
> macro.
> (and $marchid should have MSB set to 1 for open-source projects.)
>

Sorry, I made a mistake.

The MSB of $marchid should be cleared for open source projects:
"Open-source project architecture IDs are allocated globally by the RISC-V
Foundation, and have
non-zero architecture IDs with a zero most-significant-bit (MSB).
Commercial architecture IDs are
allocated by each commercial vendor independently, but must have the MSB
set and cannot contain
zero in the remaining MXLEN-1 bits."

Regards,
Frank Chang


>
> Regards,
> Frank Chang
>
>
>>
>> >
>> > Alistair
>> >
>> > > +
>> > >      DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>> > >      DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>> > >      DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
>> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> > > index c069fe85fa..3ab92deb4b 100644
>> > > --- a/target/riscv/cpu.h
>> > > +++ b/target/riscv/cpu.h
>> > > @@ -370,6 +370,10 @@ struct RISCVCPUConfig {
>> > >      bool ext_zve32f;
>> > >      bool ext_zve64f;
>> > >
>> > > +    uint32_t mvendorid;
>> > > +    uint64_t marchid;
>> > > +    uint64_t mipid;
>> > > +
>> > >      /* Vendor-specific custom extensions */
>> > >      bool ext_XVentanaCondOps;
>> > >
>> > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> > > index 341c2e6f23..9a02038adb 100644
>> > > --- a/target/riscv/csr.c
>> > > +++ b/target/riscv/csr.c
>> > > @@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState
>> *env, int csrno,
>> > >      return RISCV_EXCP_NONE;
>> > >  }
>> > >
>> > > +static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
>> > > +                                     target_ulong *val)
>> > > +{
>> > > +    CPUState *cs = env_cpu(env);
>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>> > > +
>> > > +    *val = cpu->cfg.mvendorid;
>> > > +    return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > > +static RISCVException read_marchid(CPURISCVState *env, int csrno,
>> > > +                                   target_ulong *val)
>> > > +{
>> > > +    CPUState *cs = env_cpu(env);
>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>> > > +
>> > > +    *val = cpu->cfg.marchid;
>> > > +    return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > > +static RISCVException read_mipid(CPURISCVState *env, int csrno,
>> > > +                                 target_ulong *val)
>> > > +{
>> > > +    CPUState *cs = env_cpu(env);
>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>> > > +
>> > > +    *val = cpu->cfg.mipid;
>> > > +    return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > >  static RISCVException read_mhartid(CPURISCVState *env, int csrno,
>> > >                                     target_ulong *val)
>> > >  {
>> > > @@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE]
>> = {
>> > >      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
>> > >
>> > >      /* Machine Information Registers */
>> > > -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
>> > > -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
>> > > -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
>> > > -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
>> > > +    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
>> > > +    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
>> > > +    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
>> > > +    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
>> > >
>> > >      /* Machine Trap Setup */
>> > >      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,
>>  write_mstatus, NULL,
>> > > --
>> > > 2.35.1
>> > >
>> > >
>> >
>>
>

[-- Attachment #2: Type: text/html, Size: 8671 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values
@ 2022-04-20  8:32         ` Frank Chang
  0 siblings, 0 replies; 16+ messages in thread
From: Frank Chang @ 2022-04-20  8:32 UTC (permalink / raw)
  To: Anup Patel
  Cc: Alistair Francis, open list:RISC-V, Bin Meng,
	qemu-devel@nongnu.org Developers, Jim Shu, Palmer Dabbelt,
	Alistair Francis

[-- Attachment #1: Type: text/plain, Size: 5882 bytes --]

On Tue, Apr 19, 2022 at 2:00 PM Frank Chang <frank.chang@sifive.com> wrote:

> On Tue, Apr 19, 2022 at 1:27 PM Anup Patel <apatel@ventanamicro.com>
> wrote:
>
>> On Tue, Apr 19, 2022 at 10:52 AM Alistair Francis <alistair23@gmail.com>
>> wrote:
>> >
>> > On Fri, Apr 15, 2022 at 7:37 PM <frank.chang@sifive.com> wrote:
>> > >
>> > > From: Frank Chang <frank.chang@sifive.com>
>> > >
>> > > Allow user to set core's marchid, mvendorid, mipid CSRs through
>> > > -cpu command line option.
>> > >
>> > > Signed-off-by: Frank Chang <frank.chang@sifive.com>
>> > > Reviewed-by: Jim Shu <jim.shu@sifive.com>
>> > > ---
>> > >  target/riscv/cpu.c |  4 ++++
>> > >  target/riscv/cpu.h |  4 ++++
>> > >  target/riscv/csr.c | 38 ++++++++++++++++++++++++++++++++++----
>> > >  3 files changed, 42 insertions(+), 4 deletions(-)
>> > >
>> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> > > index ddda4906ff..2eea0f9be7 100644
>> > > --- a/target/riscv/cpu.c
>> > > +++ b/target/riscv/cpu.c
>> > > @@ -786,6 +786,10 @@ static Property riscv_cpu_properties[] = {
>> > >      DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>> > >      DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>> > >
>> > > +    DEFINE_PROP_UINT32("mvendorid", RISCVCPU, cfg.mvendorid, 0),
>> > > +    DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, 0),
>> > > +    DEFINE_PROP_UINT64("mipid", RISCVCPU, cfg.mipid, 0),
>> >
>> > Should we have non-zero defaults here?
>>
>> To do that, we need mvendorid for QEMU RISC-V.
>>
>> The marchid and mipid can be based on the QEMU version number.
>>
>> Regards,
>> Anup
>>
>
> The original intention for this patch is to allow users to define
> their own $mvendorid, $marchid, and $mipid through the command line
> when they initiate QEMU.
>
> If we want to provide the default values for QEMU RISC-V CPU,
> just like what Anup said.
> We need to define our own mvendorid, which should be a JEDEC manufacturer
> ID.
> (Perhaps it's fine to just give some random legal JEDEC manufacturer ID
> as I don't think we would really want to spend the money on that.)
>
> For $marchid and $mipid,
> I agree that it could base on QEMU's version from the QEMU_FULL_VERSION
> macro.
> (and $marchid should have MSB set to 1 for open-source projects.)
>

Sorry, I made a mistake.

The MSB of $marchid should be cleared for open source projects:
"Open-source project architecture IDs are allocated globally by the RISC-V
Foundation, and have
non-zero architecture IDs with a zero most-significant-bit (MSB).
Commercial architecture IDs are
allocated by each commercial vendor independently, but must have the MSB
set and cannot contain
zero in the remaining MXLEN-1 bits."

Regards,
Frank Chang


>
> Regards,
> Frank Chang
>
>
>>
>> >
>> > Alistair
>> >
>> > > +
>> > >      DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>> > >      DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>> > >      DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
>> > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> > > index c069fe85fa..3ab92deb4b 100644
>> > > --- a/target/riscv/cpu.h
>> > > +++ b/target/riscv/cpu.h
>> > > @@ -370,6 +370,10 @@ struct RISCVCPUConfig {
>> > >      bool ext_zve32f;
>> > >      bool ext_zve64f;
>> > >
>> > > +    uint32_t mvendorid;
>> > > +    uint64_t marchid;
>> > > +    uint64_t mipid;
>> > > +
>> > >      /* Vendor-specific custom extensions */
>> > >      bool ext_XVentanaCondOps;
>> > >
>> > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> > > index 341c2e6f23..9a02038adb 100644
>> > > --- a/target/riscv/csr.c
>> > > +++ b/target/riscv/csr.c
>> > > @@ -603,6 +603,36 @@ static RISCVException write_ignore(CPURISCVState
>> *env, int csrno,
>> > >      return RISCV_EXCP_NONE;
>> > >  }
>> > >
>> > > +static RISCVException read_mvendorid(CPURISCVState *env, int csrno,
>> > > +                                     target_ulong *val)
>> > > +{
>> > > +    CPUState *cs = env_cpu(env);
>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>> > > +
>> > > +    *val = cpu->cfg.mvendorid;
>> > > +    return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > > +static RISCVException read_marchid(CPURISCVState *env, int csrno,
>> > > +                                   target_ulong *val)
>> > > +{
>> > > +    CPUState *cs = env_cpu(env);
>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>> > > +
>> > > +    *val = cpu->cfg.marchid;
>> > > +    return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > > +static RISCVException read_mipid(CPURISCVState *env, int csrno,
>> > > +                                 target_ulong *val)
>> > > +{
>> > > +    CPUState *cs = env_cpu(env);
>> > > +    RISCVCPU *cpu = RISCV_CPU(cs);
>> > > +
>> > > +    *val = cpu->cfg.mipid;
>> > > +    return RISCV_EXCP_NONE;
>> > > +}
>> > > +
>> > >  static RISCVException read_mhartid(CPURISCVState *env, int csrno,
>> > >                                     target_ulong *val)
>> > >  {
>> > > @@ -3098,10 +3128,10 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE]
>> = {
>> > >      [CSR_MINSTRETH] = { "minstreth", any32, read_instreth },
>> > >
>> > >      /* Machine Information Registers */
>> > > -    [CSR_MVENDORID] = { "mvendorid", any,   read_zero    },
>> > > -    [CSR_MARCHID]   = { "marchid",   any,   read_zero    },
>> > > -    [CSR_MIMPID]    = { "mimpid",    any,   read_zero    },
>> > > -    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid },
>> > > +    [CSR_MVENDORID] = { "mvendorid", any,   read_mvendorid },
>> > > +    [CSR_MARCHID]   = { "marchid",   any,   read_marchid   },
>> > > +    [CSR_MIMPID]    = { "mimpid",    any,   read_mipid     },
>> > > +    [CSR_MHARTID]   = { "mhartid",   any,   read_mhartid   },
>> > >
>> > >      /* Machine Trap Setup */
>> > >      [CSR_MSTATUS]     = { "mstatus",    any,   read_mstatus,
>>  write_mstatus, NULL,
>> > > --
>> > > 2.35.1
>> > >
>> > >
>> >
>>
>

[-- Attachment #2: Type: text/html, Size: 8671 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-04-20  8:35 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-15  9:37 [PATCH] target/riscv: Support configuarable marchid, mvendorid, mipid CSR values frank.chang
2022-04-15  9:37 ` frank.chang
2022-04-19  5:21 ` Alistair Francis
2022-04-19  5:21   ` Alistair Francis
2022-04-19  5:27   ` Anup Patel
2022-04-19  5:27     ` Anup Patel
2022-04-19  6:00     ` Frank Chang
2022-04-19  6:00       ` Frank Chang
2022-04-19  7:04       ` Frank Chang
2022-04-19  7:04         ` Frank Chang
2022-04-20  7:47         ` Alistair Francis
2022-04-20  7:55           ` Frank Chang
2022-04-20  8:32       ` Frank Chang
2022-04-20  8:32         ` Frank Chang
2022-04-20  7:47 ` Alistair Francis
2022-04-20  7:47   ` Alistair Francis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.