Message ID | 20220526100536.49672-5-apatel@ventanamicro.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | QEMU RISC-V nested virtualization fixes | expand |
On Thu, May 26, 2022 at 8:09 PM Anup Patel <apatel@ventanamicro.com> wrote: > > We should disable extensions in riscv_cpu_realize() if minimum required > priv spec version is not satisfied. This also ensures that machines with > priv spec v1.11 (or lower) cannot enable H, V, and various multi-letter > extensions. > > Fixes: a775398be2e ("target/riscv: Add isa extenstion strings to the > device tree") > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > --- > target/riscv/cpu.c | 56 +++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 51 insertions(+), 5 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index b086eb25da..e6e878ceb3 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -43,9 +43,13 @@ static const char riscv_single_letter_exts[] = "IEMAFDQCPVH"; > > struct isa_ext_data { > const char *name; > - bool enabled; > + int min_version; > + bool *enabled; > }; > > +#define ISA_EDATA_ENTRY(name, prop) {#name, PRIV_VERSION_1_10_0, &cpu->cfg.prop} > +#define ISA_EDATA_ENTRY2(name, min_ver, prop) {#name, min_ver, &cpu->cfg.prop} > + > const char * const riscv_int_regnames[] = { > "x0/zero", "x1/ra", "x2/sp", "x3/gp", "x4/tp", "x5/t0", "x6/t1", > "x7/t2", "x8/s0", "x9/s1", "x10/a0", "x11/a1", "x12/a2", "x13/a3", > @@ -513,8 +517,42 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) > CPURISCVState *env = &cpu->env; > RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev); > CPUClass *cc = CPU_CLASS(mcc); > - int priv_version = -1; > + int i, priv_version = -1; > Error *local_err = NULL; > + struct isa_ext_data isa_edata_arr[] = { > + ISA_EDATA_ENTRY2(h, PRIV_VERSION_1_12_0, ext_h), > + ISA_EDATA_ENTRY2(v, PRIV_VERSION_1_12_0, ext_v), > + ISA_EDATA_ENTRY2(zicsr, PRIV_VERSION_1_10_0, ext_icsr), > + ISA_EDATA_ENTRY2(zifencei, PRIV_VERSION_1_10_0, ext_ifencei), > + ISA_EDATA_ENTRY2(zfh, PRIV_VERSION_1_12_0, ext_zfh), > + ISA_EDATA_ENTRY2(zfhmin, PRIV_VERSION_1_12_0, ext_zfhmin), > + ISA_EDATA_ENTRY2(zfinx, PRIV_VERSION_1_12_0, ext_zfinx), > + ISA_EDATA_ENTRY2(zdinx, PRIV_VERSION_1_12_0, ext_zdinx), > + ISA_EDATA_ENTRY2(zba, PRIV_VERSION_1_12_0, ext_zba), > + ISA_EDATA_ENTRY2(zbb, PRIV_VERSION_1_12_0, ext_zbb), > + ISA_EDATA_ENTRY2(zbc, PRIV_VERSION_1_12_0, ext_zbc), > + ISA_EDATA_ENTRY2(zbkb, PRIV_VERSION_1_12_0, ext_zbkb), > + ISA_EDATA_ENTRY2(zbkc, PRIV_VERSION_1_12_0, ext_zbkc), > + ISA_EDATA_ENTRY2(zbkx, PRIV_VERSION_1_12_0, ext_zbkx), > + ISA_EDATA_ENTRY2(zbs, PRIV_VERSION_1_12_0, ext_zbs), > + ISA_EDATA_ENTRY2(zk, PRIV_VERSION_1_12_0, ext_zk), > + ISA_EDATA_ENTRY2(zkn, PRIV_VERSION_1_12_0, ext_zkn), > + ISA_EDATA_ENTRY2(zknd, PRIV_VERSION_1_12_0, ext_zknd), > + ISA_EDATA_ENTRY2(zkne, PRIV_VERSION_1_12_0, ext_zkne), > + ISA_EDATA_ENTRY2(zknh, PRIV_VERSION_1_12_0, ext_zknh), > + ISA_EDATA_ENTRY2(zkr, PRIV_VERSION_1_12_0, ext_zkr), > + ISA_EDATA_ENTRY2(zks, PRIV_VERSION_1_12_0, ext_zks), > + ISA_EDATA_ENTRY2(zksed, PRIV_VERSION_1_12_0, ext_zksed), > + ISA_EDATA_ENTRY2(zksh, PRIV_VERSION_1_12_0, ext_zksh), > + ISA_EDATA_ENTRY2(zkt, PRIV_VERSION_1_12_0, ext_zkt), > + ISA_EDATA_ENTRY2(zve32f, PRIV_VERSION_1_12_0, ext_zve32f), > + ISA_EDATA_ENTRY2(zve64f, PRIV_VERSION_1_12_0, ext_zve64f), > + ISA_EDATA_ENTRY2(zhinx, PRIV_VERSION_1_12_0, ext_zhinx), > + ISA_EDATA_ENTRY2(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin), > + ISA_EDATA_ENTRY2(svinval, PRIV_VERSION_1_12_0, ext_svinval), > + ISA_EDATA_ENTRY2(svnapot, PRIV_VERSION_1_12_0, ext_svnapot), > + ISA_EDATA_ENTRY2(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), > + }; > > cpu_exec_realizefn(cs, &local_err); > if (local_err != NULL) { > @@ -541,6 +579,16 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) > set_priv_version(env, priv_version); > } > > + /* Force disable extensions if priv spec version does not match */ > + for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) { > + if (*isa_edata_arr[i].enabled && > + (env->priv_ver < isa_edata_arr[i].min_version)) { > + *isa_edata_arr[i].enabled = false; > + warn_report("privilege spec version does not match for %s extension", > + isa_edata_arr[i].name); This should indicate to the user that we are disabling the extension because of this Alistair > + } > + } > + > if (cpu->cfg.mmu) { > riscv_set_feature(env, RISCV_FEATURE_MMU); > } > @@ -1005,8 +1053,6 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data) > device_class_set_props(dc, riscv_cpu_properties); > } > > -#define ISA_EDATA_ENTRY(name, prop) {#name, cpu->cfg.prop} > - > static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len) > { > char *old = *isa_str; > @@ -1064,7 +1110,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len) > }; > > for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) { > - if (isa_edata_arr[i].enabled) { > + if (*isa_edata_arr[i].enabled) { > new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL); > g_free(old); > old = new; > -- > 2.34.1 > >
On Mon, Jun 6, 2022 at 7:25 AM Alistair Francis <alistair23@gmail.com> wrote: > > On Thu, May 26, 2022 at 8:09 PM Anup Patel <apatel@ventanamicro.com> wrote: > > > > We should disable extensions in riscv_cpu_realize() if minimum required > > priv spec version is not satisfied. This also ensures that machines with > > priv spec v1.11 (or lower) cannot enable H, V, and various multi-letter > > extensions. > > > > Fixes: a775398be2e ("target/riscv: Add isa extenstion strings to the > > device tree") > > Signed-off-by: Anup Patel <apatel@ventanamicro.com> > > --- > > target/riscv/cpu.c | 56 +++++++++++++++++++++++++++++++++++++++++----- > > 1 file changed, 51 insertions(+), 5 deletions(-) > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > index b086eb25da..e6e878ceb3 100644 > > --- a/target/riscv/cpu.c > > +++ b/target/riscv/cpu.c > > @@ -43,9 +43,13 @@ static const char riscv_single_letter_exts[] = "IEMAFDQCPVH"; > > > > struct isa_ext_data { > > const char *name; > > - bool enabled; > > + int min_version; > > + bool *enabled; > > }; > > > > +#define ISA_EDATA_ENTRY(name, prop) {#name, PRIV_VERSION_1_10_0, &cpu->cfg.prop} > > +#define ISA_EDATA_ENTRY2(name, min_ver, prop) {#name, min_ver, &cpu->cfg.prop} > > + > > const char * const riscv_int_regnames[] = { > > "x0/zero", "x1/ra", "x2/sp", "x3/gp", "x4/tp", "x5/t0", "x6/t1", > > "x7/t2", "x8/s0", "x9/s1", "x10/a0", "x11/a1", "x12/a2", "x13/a3", > > @@ -513,8 +517,42 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) > > CPURISCVState *env = &cpu->env; > > RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev); > > CPUClass *cc = CPU_CLASS(mcc); > > - int priv_version = -1; > > + int i, priv_version = -1; > > Error *local_err = NULL; > > + struct isa_ext_data isa_edata_arr[] = { > > + ISA_EDATA_ENTRY2(h, PRIV_VERSION_1_12_0, ext_h), > > + ISA_EDATA_ENTRY2(v, PRIV_VERSION_1_12_0, ext_v), > > + ISA_EDATA_ENTRY2(zicsr, PRIV_VERSION_1_10_0, ext_icsr), > > + ISA_EDATA_ENTRY2(zifencei, PRIV_VERSION_1_10_0, ext_ifencei), > > + ISA_EDATA_ENTRY2(zfh, PRIV_VERSION_1_12_0, ext_zfh), > > + ISA_EDATA_ENTRY2(zfhmin, PRIV_VERSION_1_12_0, ext_zfhmin), > > + ISA_EDATA_ENTRY2(zfinx, PRIV_VERSION_1_12_0, ext_zfinx), > > + ISA_EDATA_ENTRY2(zdinx, PRIV_VERSION_1_12_0, ext_zdinx), > > + ISA_EDATA_ENTRY2(zba, PRIV_VERSION_1_12_0, ext_zba), > > + ISA_EDATA_ENTRY2(zbb, PRIV_VERSION_1_12_0, ext_zbb), > > + ISA_EDATA_ENTRY2(zbc, PRIV_VERSION_1_12_0, ext_zbc), > > + ISA_EDATA_ENTRY2(zbkb, PRIV_VERSION_1_12_0, ext_zbkb), > > + ISA_EDATA_ENTRY2(zbkc, PRIV_VERSION_1_12_0, ext_zbkc), > > + ISA_EDATA_ENTRY2(zbkx, PRIV_VERSION_1_12_0, ext_zbkx), > > + ISA_EDATA_ENTRY2(zbs, PRIV_VERSION_1_12_0, ext_zbs), > > + ISA_EDATA_ENTRY2(zk, PRIV_VERSION_1_12_0, ext_zk), > > + ISA_EDATA_ENTRY2(zkn, PRIV_VERSION_1_12_0, ext_zkn), > > + ISA_EDATA_ENTRY2(zknd, PRIV_VERSION_1_12_0, ext_zknd), > > + ISA_EDATA_ENTRY2(zkne, PRIV_VERSION_1_12_0, ext_zkne), > > + ISA_EDATA_ENTRY2(zknh, PRIV_VERSION_1_12_0, ext_zknh), > > + ISA_EDATA_ENTRY2(zkr, PRIV_VERSION_1_12_0, ext_zkr), > > + ISA_EDATA_ENTRY2(zks, PRIV_VERSION_1_12_0, ext_zks), > > + ISA_EDATA_ENTRY2(zksed, PRIV_VERSION_1_12_0, ext_zksed), > > + ISA_EDATA_ENTRY2(zksh, PRIV_VERSION_1_12_0, ext_zksh), > > + ISA_EDATA_ENTRY2(zkt, PRIV_VERSION_1_12_0, ext_zkt), > > + ISA_EDATA_ENTRY2(zve32f, PRIV_VERSION_1_12_0, ext_zve32f), > > + ISA_EDATA_ENTRY2(zve64f, PRIV_VERSION_1_12_0, ext_zve64f), > > + ISA_EDATA_ENTRY2(zhinx, PRIV_VERSION_1_12_0, ext_zhinx), > > + ISA_EDATA_ENTRY2(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin), > > + ISA_EDATA_ENTRY2(svinval, PRIV_VERSION_1_12_0, ext_svinval), > > + ISA_EDATA_ENTRY2(svnapot, PRIV_VERSION_1_12_0, ext_svnapot), > > + ISA_EDATA_ENTRY2(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), > > + }; > > > > cpu_exec_realizefn(cs, &local_err); > > if (local_err != NULL) { > > @@ -541,6 +579,16 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) > > set_priv_version(env, priv_version); > > } > > > > + /* Force disable extensions if priv spec version does not match */ > > + for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) { > > + if (*isa_edata_arr[i].enabled && > > + (env->priv_ver < isa_edata_arr[i].min_version)) { > > + *isa_edata_arr[i].enabled = false; > > + warn_report("privilege spec version does not match for %s extension", > > + isa_edata_arr[i].name); > > This should indicate to the user that we are disabling the extension > because of this Sure, I will update. Regards, Anup > > Alistair > > > + } > > + } > > + > > if (cpu->cfg.mmu) { > > riscv_set_feature(env, RISCV_FEATURE_MMU); > > } > > @@ -1005,8 +1053,6 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data) > > device_class_set_props(dc, riscv_cpu_properties); > > } > > > > -#define ISA_EDATA_ENTRY(name, prop) {#name, cpu->cfg.prop} > > - > > static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len) > > { > > char *old = *isa_str; > > @@ -1064,7 +1110,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len) > > }; > > > > for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) { > > - if (isa_edata_arr[i].enabled) { > > + if (*isa_edata_arr[i].enabled) { > > new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL); > > g_free(old); > > old = new; > > -- > > 2.34.1 > > > >
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index b086eb25da..e6e878ceb3 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -43,9 +43,13 @@ static const char riscv_single_letter_exts[] = "IEMAFDQCPVH"; struct isa_ext_data { const char *name; - bool enabled; + int min_version; + bool *enabled; }; +#define ISA_EDATA_ENTRY(name, prop) {#name, PRIV_VERSION_1_10_0, &cpu->cfg.prop} +#define ISA_EDATA_ENTRY2(name, min_ver, prop) {#name, min_ver, &cpu->cfg.prop} + const char * const riscv_int_regnames[] = { "x0/zero", "x1/ra", "x2/sp", "x3/gp", "x4/tp", "x5/t0", "x6/t1", "x7/t2", "x8/s0", "x9/s1", "x10/a0", "x11/a1", "x12/a2", "x13/a3", @@ -513,8 +517,42 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) CPURISCVState *env = &cpu->env; RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev); CPUClass *cc = CPU_CLASS(mcc); - int priv_version = -1; + int i, priv_version = -1; Error *local_err = NULL; + struct isa_ext_data isa_edata_arr[] = { + ISA_EDATA_ENTRY2(h, PRIV_VERSION_1_12_0, ext_h), + ISA_EDATA_ENTRY2(v, PRIV_VERSION_1_12_0, ext_v), + ISA_EDATA_ENTRY2(zicsr, PRIV_VERSION_1_10_0, ext_icsr), + ISA_EDATA_ENTRY2(zifencei, PRIV_VERSION_1_10_0, ext_ifencei), + ISA_EDATA_ENTRY2(zfh, PRIV_VERSION_1_12_0, ext_zfh), + ISA_EDATA_ENTRY2(zfhmin, PRIV_VERSION_1_12_0, ext_zfhmin), + ISA_EDATA_ENTRY2(zfinx, PRIV_VERSION_1_12_0, ext_zfinx), + ISA_EDATA_ENTRY2(zdinx, PRIV_VERSION_1_12_0, ext_zdinx), + ISA_EDATA_ENTRY2(zba, PRIV_VERSION_1_12_0, ext_zba), + ISA_EDATA_ENTRY2(zbb, PRIV_VERSION_1_12_0, ext_zbb), + ISA_EDATA_ENTRY2(zbc, PRIV_VERSION_1_12_0, ext_zbc), + ISA_EDATA_ENTRY2(zbkb, PRIV_VERSION_1_12_0, ext_zbkb), + ISA_EDATA_ENTRY2(zbkc, PRIV_VERSION_1_12_0, ext_zbkc), + ISA_EDATA_ENTRY2(zbkx, PRIV_VERSION_1_12_0, ext_zbkx), + ISA_EDATA_ENTRY2(zbs, PRIV_VERSION_1_12_0, ext_zbs), + ISA_EDATA_ENTRY2(zk, PRIV_VERSION_1_12_0, ext_zk), + ISA_EDATA_ENTRY2(zkn, PRIV_VERSION_1_12_0, ext_zkn), + ISA_EDATA_ENTRY2(zknd, PRIV_VERSION_1_12_0, ext_zknd), + ISA_EDATA_ENTRY2(zkne, PRIV_VERSION_1_12_0, ext_zkne), + ISA_EDATA_ENTRY2(zknh, PRIV_VERSION_1_12_0, ext_zknh), + ISA_EDATA_ENTRY2(zkr, PRIV_VERSION_1_12_0, ext_zkr), + ISA_EDATA_ENTRY2(zks, PRIV_VERSION_1_12_0, ext_zks), + ISA_EDATA_ENTRY2(zksed, PRIV_VERSION_1_12_0, ext_zksed), + ISA_EDATA_ENTRY2(zksh, PRIV_VERSION_1_12_0, ext_zksh), + ISA_EDATA_ENTRY2(zkt, PRIV_VERSION_1_12_0, ext_zkt), + ISA_EDATA_ENTRY2(zve32f, PRIV_VERSION_1_12_0, ext_zve32f), + ISA_EDATA_ENTRY2(zve64f, PRIV_VERSION_1_12_0, ext_zve64f), + ISA_EDATA_ENTRY2(zhinx, PRIV_VERSION_1_12_0, ext_zhinx), + ISA_EDATA_ENTRY2(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin), + ISA_EDATA_ENTRY2(svinval, PRIV_VERSION_1_12_0, ext_svinval), + ISA_EDATA_ENTRY2(svnapot, PRIV_VERSION_1_12_0, ext_svnapot), + ISA_EDATA_ENTRY2(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt), + }; cpu_exec_realizefn(cs, &local_err); if (local_err != NULL) { @@ -541,6 +579,16 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) set_priv_version(env, priv_version); } + /* Force disable extensions if priv spec version does not match */ + for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) { + if (*isa_edata_arr[i].enabled && + (env->priv_ver < isa_edata_arr[i].min_version)) { + *isa_edata_arr[i].enabled = false; + warn_report("privilege spec version does not match for %s extension", + isa_edata_arr[i].name); + } + } + if (cpu->cfg.mmu) { riscv_set_feature(env, RISCV_FEATURE_MMU); } @@ -1005,8 +1053,6 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data) device_class_set_props(dc, riscv_cpu_properties); } -#define ISA_EDATA_ENTRY(name, prop) {#name, cpu->cfg.prop} - static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len) { char *old = *isa_str; @@ -1064,7 +1110,7 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len) }; for (i = 0; i < ARRAY_SIZE(isa_edata_arr); i++) { - if (isa_edata_arr[i].enabled) { + if (*isa_edata_arr[i].enabled) { new = g_strconcat(old, "_", isa_edata_arr[i].name, NULL); g_free(old); old = new;
We should disable extensions in riscv_cpu_realize() if minimum required priv spec version is not satisfied. This also ensures that machines with priv spec v1.11 (or lower) cannot enable H, V, and various multi-letter extensions. Fixes: a775398be2e ("target/riscv: Add isa extenstion strings to the device tree") Signed-off-by: Anup Patel <apatel@ventanamicro.com> --- target/riscv/cpu.c | 56 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 51 insertions(+), 5 deletions(-)