On Wed, Jun 14, 2023 at 7:03 AM Daniel Henrique Barboza
<dbarb...@ventanamicro.com> wrote:
>
> 'marchid' shouldn't be set to a different value as previously set for
> named CPUs.
>
> For all other CPUs it shouldn't be freely set either - the spec requires
> that 'marchid' can't have the MSB (most significant bit) set and every
> other bit set to zero, i.e. 0x80000000 is an invalid 'marchid' value for
> 32 bit CPUs.
>
> As with 'mimpid', setting a default value based on the current QEMU
> version is not a good idea because it implies that the CPU
> implementation changes from one QEMU version to the other. Named CPUs
> should set 'marchid' to a meaningful value instead, and generic CPUs can
> set to any valid value.
>
> For the 'veyron-v1' CPU this is the error thrown if 'marchid' is set to
> a different val:
>
> $ ./build/qemu-system-riscv64 -M virt -nographic -cpu
> veyron-v1,marchid=0x80000000
> qemu-system-riscv64: can't apply global
> veyron-v1-riscv-cpu.marchid=0x80000000:
> Unable to change veyron-v1-riscv-cpu marchid (0x8000000000010000)
>
> And, for generics CPUs, this is the error when trying to set to an
> invalid val:
>
> $ ./build/qemu-system-riscv64 -M virt -nographic -cpu
> rv64,marchid=0x8000000000000000
> qemu-system-riscv64: can't apply global
> rv64-riscv-cpu.marchid=0x8000000000000000:
> Unable to set marchid with MSB (64) bit set and the remaining bits zero
>
> Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
> Reviewed-by: Andrew Jones <ajo...@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 60 ++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 53 insertions(+), 7 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 39c550682a..2eb793188c 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -38,11 +38,6 @@
> #include "tcg/tcg.h"
>
> /* RISC-V CPU definitions */
> -
> -#define RISCV_CPU_MARCHID ((QEMU_VERSION_MAJOR << 16) | \
> - (QEMU_VERSION_MINOR << 8) | \
> - (QEMU_VERSION_MICRO))
> -
> static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
>
> struct isa_ext_data {
> @@ -1733,8 +1728,6 @@ static void riscv_cpu_add_user_properties(Object *obj)
> static Property riscv_cpu_properties[] = {
> DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
>
> - DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid, RISCV_CPU_MARCHID),
> -
> #ifndef CONFIG_USER_ONLY
> DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec, DEFAULT_RSTVEC),
> #endif
> @@ -1881,6 +1874,56 @@ static void cpu_get_mimpid(Object *obj, Visitor *v,
> const char *name,
> visit_type_bool(v, name, &value, errp);
> }
>
> +static void cpu_set_marchid(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + bool dynamic_cpu = riscv_cpu_is_dynamic(obj);
> + RISCVCPU *cpu = RISCV_CPU(obj);
> + uint64_t prev_val = cpu->cfg.marchid;
> + uint64_t value, invalid_val;
> + uint32_t mxlen = 0;
> +
> + if (!visit_type_uint64(v, name, &value, errp)) {
> + return;
> + }
> +
> + if (!dynamic_cpu && prev_val != value) {
> + error_setg(errp, "Unable to change %s marchid (0x%lx)",
> + object_get_typename(obj), prev_val);
> + return;
> + }
> +
> + switch (riscv_cpu_mxl(&cpu->env)) {
> + case MXL_RV32:
> + mxlen = 32;
> + break;
> + case MXL_RV64:
> + case MXL_RV128:
> + mxlen = 64;
> + break;
> + default:
> + g_assert_not_reached();
> + }
> +
> + invalid_val = 1LL << (mxlen - 1);
> +
> + if (value == invalid_val) {
> + error_setg(errp, "Unable to set marchid with MSB (%u) bit set "
> + "and the remaining bits zero", mxlen);
> + return;
> + }
> +
> + cpu->cfg.marchid = value;
> +}
> +
> +static void cpu_get_marchid(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + bool value = RISCV_CPU(obj)->cfg.marchid;
> +
> + visit_type_bool(v, name, &value, errp);
> +}
> +
> static void riscv_cpu_class_init(ObjectClass *c, void *data)
> {
> RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
> @@ -1918,6 +1961,9 @@ static void riscv_cpu_class_init(ObjectClass *c, void
> *data)
> object_class_property_add(c, "mimpid", "uint64", cpu_get_mimpid,
> cpu_set_mimpid, NULL, NULL);
>
> + object_class_property_add(c, "marchid", "uint64", cpu_get_marchid,
> + cpu_set_marchid, NULL, NULL);
> +
> device_class_set_props(dc, riscv_cpu_properties);
> }
>
> --
> 2.40.1
>
>
