Re: Re: [PATCH] RISC-V: Report the QEMU vendor/arch IDs on virtual CPUs

[Andrew Jones]

On Thu, Feb 01, 2024 at 04:06:15PM +0100, Andrew Jones wrote:
> On Wed, Jan 31, 2024 at 10:24:30AM -0800, Palmer Dabbelt wrote:
> > Right now we just report 0 for marchid/mvendorid in QEMU.  That's legal,
> > but it's tricky for users that want to check if they're running on QEMU
> > to do so.  This sets marchid to 42, which I've proposed as the QEMU
> > architecture ID (mvendorid remains 0, just explicitly set, as that's how
> > the ISA handles open source implementations).
> 
> Hi Palmer,
> 
> marchid has this text
> 
> """
> Open-source project architecture IDs are allocated globally by RISC-V
> International, 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.
> """
> 
> and mvendorid has this text
> 
> """
> ...a value of 0 can be
> returned to indicate the field is not implemented or that this is a
> non-commercial implementation.
> """
> 
> We must select zero for mvendorid, since we're a non-commercial
> implementation, and that means we can't set the marchid to a commercial
> ID of our choosing, i.e. some ID with the MSB set. We also can't select
> an archid without the MSB set, though, because RVI needs to allocate
> us that ID. Long story short, I think we need to use mvendorid=0,marchid=0
> unless we ask for and receive an marchid from RVI. Now, looking at mimpid,
> I think we're free to set that to whatever we want, even if the other IDs
> must be zero, but we should probably consider if we also want some bits
> reserved for revisions or something before settling on an ID. Actually,
> my vote would be to get an official marchid from RVI and then mimpid can
> be reserved for other uses.

So it looks like my vote was the plan all along :-) After reading
Alistair's "this has been approved" comment regarding the Link below I
actually bothered to check it and see that 42 is indeed the official RVI
marchid. That wasn't clear to me from the commit message, but I guess I
should have clicked the link!

Sorry for the noise.

Thanks,
drew

> 
> Thanks,
> drew
> 
> 
> 
> > 
> > Link: https://github.com/riscv/riscv-isa-manual/pull/1213
> > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> > ---
> >  target/riscv/cpu.c          | 16 ++++++++++++++++
> >  target/riscv/cpu_vendorid.h |  3 +++
> >  2 files changed, 19 insertions(+)
> > 
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 8cbfc7e781..1aef186f87 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -415,6 +415,9 @@ static void riscv_any_cpu_init(Object *obj)
> >      cpu->cfg.ext_zicsr = true;
> >      cpu->cfg.mmu = true;
> >      cpu->cfg.pmp = true;
> > +
> > +    cpu->cfg.mvendorid = QEMU_MVENDORID;
> > +    cpu->cfg.marchid = QEMU_MARCHID;
> >  }
> >  
> >  static void riscv_max_cpu_init(Object *obj)
> > @@ -432,6 +435,8 @@ static void riscv_max_cpu_init(Object *obj)
> >      set_satp_mode_max_supported(RISCV_CPU(obj), mlx == MXL_RV32 ?
> >                                  VM_1_10_SV32 : VM_1_10_SV57);
> >  #endif
> > +    cpu->cfg.mvendorid = QEMU_MVENDORID;
> > +    cpu->cfg.marchid = QEMU_MARCHID;
> >  }
> >  
> >  #if defined(TARGET_RISCV64)
> > @@ -445,6 +450,8 @@ static void rv64_base_cpu_init(Object *obj)
> >  #ifndef CONFIG_USER_ONLY
> >      set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57);
> >  #endif
> > +    cpu->cfg.mvendorid = QEMU_MVENDORID;
> > +    cpu->cfg.marchid = QEMU_MARCHID;
> >  }
> >  
> >  static void rv64_sifive_u_cpu_init(Object *obj)
> > @@ -569,6 +576,8 @@ static void rv128_base_cpu_init(Object *obj)
> >  #ifndef CONFIG_USER_ONLY
> >      set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV57);
> >  #endif
> > +    cpu->cfg.mvendorid = QEMU_MVENDORID;
> > +    cpu->cfg.marchid = QEMU_MARCHID;
> >  }
> >  
> >  static void rv64i_bare_cpu_init(Object *obj)
> > @@ -591,6 +600,8 @@ static void rv64i_bare_cpu_init(Object *obj)
> >  #ifndef CONFIG_USER_ONLY
> >      set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV64);
> >  #endif
> > +    cpu->cfg.mvendorid = QEMU_MVENDORID;
> > +    cpu->cfg.marchid = QEMU_MARCHID;
> >  }
> >  #else
> >  static void rv32_base_cpu_init(Object *obj)
> > @@ -603,6 +614,8 @@ static void rv32_base_cpu_init(Object *obj)
> >  #ifndef CONFIG_USER_ONLY
> >      set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV32);
> >  #endif
> > +    cpu->cfg.mvendorid = QEMU_MVENDORID;
> > +    cpu->cfg.marchid = QEMU_MARCHID;
> >  }
> >  
> >  static void rv32_sifive_u_cpu_init(Object *obj)
> > @@ -672,6 +685,9 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
> >      cpu->cfg.ext_zifencei = true;
> >      cpu->cfg.ext_zicsr = true;
> >      cpu->cfg.pmp = true;
> > +
> > +    cpu->cfg.mvendorid = QEMU_MVENDORID;
> > +    cpu->cfg.marchid = QEMU_MARCHID;
> >  }
> >  #endif
> >  
> > diff --git a/target/riscv/cpu_vendorid.h b/target/riscv/cpu_vendorid.h
> > index 96b6b9c2cb..486832cd53 100644
> > --- a/target/riscv/cpu_vendorid.h
> > +++ b/target/riscv/cpu_vendorid.h
> > @@ -7,4 +7,7 @@
> >  #define VEYRON_V1_MIMPID        0x111
> >  #define VEYRON_V1_MVENDORID     0x61f
> >  
> > +#define QEMU_VIRT_MVENDORID     0
> > +#define QEMU_VIRT_MARCHID       42
> > +
> >  #endif /*  TARGET_RISCV_CPU_VENDORID_H */
> > -- 
> > 2.43.0
> > 
> >