diff mbox series

target/riscv: Support configuarable marchid, mvendorid, mipid CSR values

Message ID 20220415093727.15323-1-frank.chang@sifive.com (mailing list archive)
State New, archived
Headers show
Series target/riscv: Support configuarable marchid, mvendorid, mipid CSR values | expand

Commit Message

Frank Chang April 15, 2022, 9:37 a.m. UTC 
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(-)

Comments

Alistair Francis April 19, 2022, 5:21 a.m. UTC | #1 
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
>
>
Anup Patel April 19, 2022, 5:27 a.m. UTC | #2 
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
> >
> >
>
Frank Chang April 19, 2022, 6 a.m. UTC | #3 
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
> > >
> > >
> >
>
Frank Chang April 19, 2022, 7:04 a.m. UTC | #4 
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
>> > >
>> > >
>> >
>>
>
Alistair Francis April 20, 2022, 7:47 a.m. UTC | #5 
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
>>> > >
>>> > >
>>> >
Alistair Francis April 20, 2022, 7:47 a.m. UTC | #6 
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
>
>
Frank Chang April 20, 2022, 7:55 a.m. UTC | #7 
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
> >>> > >
> >>> > >
> >>> >
>
Frank Chang April 20, 2022, 8:32 a.m. UTC | #8 
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
>> > >
>> > >
>> >
>>
>
diff mbox series

Patch

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,