| Message ID | 20230530194623.272652-2-dbarboza@ventanamicro.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv, KVM: fixes and enhancements | expand |
On Wed, May 31, 2023 at 5:49 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > As it is today it's not possible to use '-cpu host' if the RISC-V host > has RVH enabled. This is the resulting error: > > $ sudo ./qemu/build/qemu-system-riscv64 \ > -machine virt,accel=kvm -m 2G -smp 1 \ > -nographic -snapshot -kernel ./guest_imgs/Image \ > -initrd ./guest_imgs/rootfs_kvm_riscv64.img \ > -append "earlycon=sbi root=/dev/ram rw" \ > -cpu host > qemu-system-riscv64: H extension requires priv spec 1.12.0 > > This happens because we're checking for priv spec for all CPUs, and > since we're not setting env->priv_ver for the 'host' CPU, it's being > default to zero (i.e. PRIV_SPEC_1_10_0). > > In reality env->priv_ver does not make sense when running with the KVM > 'host' CPU. It's used to gate certain CSRs/extensions during translation > to make them unavailable if the hart declares an older spec version. It > doesn't have any other use. E.g. OpenSBI version 1.2 retrieves the spec > checking if the CSR_MCOUNTEREN, CSR_MCOUNTINHIBIT and CSR_MENVCFG CSRs > are available [1]. > > 'priv_ver' is just one example. We're doing a lot of feature validation > and setup during riscv_cpu_realize() that it doesn't apply KVM CPUs. > Validating the feature set for those CPUs is a KVM problem that should > be handled in KVM specific code. > > The new riscv_cpu_realize_features() helper contains all validation > logic that are not applicable to KVM CPUs. riscv_cpu_realize() verifies > if we're dealing with a KVM CPU and, if not, execute the new helper to > proceed with the usual realize() logic for all other CPUs. > > [1] lib/sbi/sbi_hart.c, hart_detect_features() > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/cpu.c | 41 ++++++++++++++++++++++++++++++++--------- > 1 file changed, 32 insertions(+), 9 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 938c7bd87b..72f5433776 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -331,6 +331,15 @@ static void set_satp_mode_default_map(RISCVCPU *cpu) > } > #endif > > +static bool riscv_running_KVM(void) > +{ > +#ifndef CONFIG_USER_ONLY > + return kvm_enabled(); > +#else > + return false; > +#endif > +} > + > static void riscv_any_cpu_init(Object *obj) > { > RISCVCPU *cpu = RISCV_CPU(obj); > @@ -1295,20 +1304,12 @@ static void riscv_cpu_validate_misa_priv(CPURISCVState *env, Error **errp) > } > } > > -static void riscv_cpu_realize(DeviceState *dev, Error **errp) > +static void riscv_cpu_realize_features(DeviceState *dev, Error **errp) > { > - CPUState *cs = CPU(dev); > RISCVCPU *cpu = RISCV_CPU(dev); > CPURISCVState *env = &cpu->env; > - RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev); > Error *local_err = NULL; > > - cpu_exec_realizefn(cs, &local_err); > - if (local_err != NULL) { > - error_propagate(errp, local_err); > - return; > - } > - > riscv_cpu_validate_misa_mxl(cpu, &local_err); > if (local_err != NULL) { > error_propagate(errp, local_err); > @@ -1354,6 +1355,28 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) > } > } > #endif > +} > + > +static void riscv_cpu_realize(DeviceState *dev, Error **errp) > +{ > + CPUState *cs = CPU(dev); > + RISCVCPU *cpu = RISCV_CPU(dev); > + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev); > + Error *local_err = NULL; > + > + cpu_exec_realizefn(cs, &local_err); > + if (local_err != NULL) { > + error_propagate(errp, local_err); > + return; > + } > + > + if (!riscv_running_KVM()) { > + riscv_cpu_realize_features(dev, &local_err); > + if (local_err != NULL) { > + error_propagate(errp, local_err); > + return; > + } > + } > > riscv_cpu_finalize_features(cpu, &local_err); > if (local_err != NULL) { > -- > 2.40.1 > >
On Tue, May 30, 2023 at 04:46:08PM -0300, Daniel Henrique Barboza wrote: > As it is today it's not possible to use '-cpu host' if the RISC-V host > has RVH enabled. This is the resulting error: > > $ sudo ./qemu/build/qemu-system-riscv64 \ > -machine virt,accel=kvm -m 2G -smp 1 \ > -nographic -snapshot -kernel ./guest_imgs/Image \ > -initrd ./guest_imgs/rootfs_kvm_riscv64.img \ > -append "earlycon=sbi root=/dev/ram rw" \ > -cpu host > qemu-system-riscv64: H extension requires priv spec 1.12.0 > > This happens because we're checking for priv spec for all CPUs, and > since we're not setting env->priv_ver for the 'host' CPU, it's being > default to zero (i.e. PRIV_SPEC_1_10_0). > > In reality env->priv_ver does not make sense when running with the KVM > 'host' CPU. It's used to gate certain CSRs/extensions during translation > to make them unavailable if the hart declares an older spec version. It > doesn't have any other use. E.g. OpenSBI version 1.2 retrieves the spec > checking if the CSR_MCOUNTEREN, CSR_MCOUNTINHIBIT and CSR_MENVCFG CSRs > are available [1]. > > 'priv_ver' is just one example. We're doing a lot of feature validation > and setup during riscv_cpu_realize() that it doesn't apply KVM CPUs. > Validating the feature set for those CPUs is a KVM problem that should > be handled in KVM specific code. > > The new riscv_cpu_realize_features() helper contains all validation > logic that are not applicable to KVM CPUs. riscv_cpu_realize() verifies > if we're dealing with a KVM CPU and, if not, execute the new helper to > proceed with the usual realize() logic for all other CPUs. > > [1] lib/sbi/sbi_hart.c, hart_detect_features() > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > target/riscv/cpu.c | 41 ++++++++++++++++++++++++++++++++--------- > 1 file changed, 32 insertions(+), 9 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 938c7bd87b..72f5433776 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -331,6 +331,15 @@ static void set_satp_mode_default_map(RISCVCPU *cpu) > } > #endif > > +static bool riscv_running_KVM(void) KVM should be lowercase > +{ > +#ifndef CONFIG_USER_ONLY > + return kvm_enabled(); > +#else > + return false; > +#endif > +} > + > static void riscv_any_cpu_init(Object *obj) > { > RISCVCPU *cpu = RISCV_CPU(obj); > @@ -1295,20 +1304,12 @@ static void riscv_cpu_validate_misa_priv(CPURISCVState *env, Error **errp) > } > } > > -static void riscv_cpu_realize(DeviceState *dev, Error **errp) > +static void riscv_cpu_realize_features(DeviceState *dev, Error **errp) > { > - CPUState *cs = CPU(dev); > RISCVCPU *cpu = RISCV_CPU(dev); > CPURISCVState *env = &cpu->env; > - RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev); > Error *local_err = NULL; > > - cpu_exec_realizefn(cs, &local_err); > - if (local_err != NULL) { > - error_propagate(errp, local_err); > - return; > - } > - > riscv_cpu_validate_misa_mxl(cpu, &local_err); > if (local_err != NULL) { > error_propagate(errp, local_err); > @@ -1354,6 +1355,28 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) > } > } > #endif > +} > + > +static void riscv_cpu_realize(DeviceState *dev, Error **errp) > +{ > + CPUState *cs = CPU(dev); > + RISCVCPU *cpu = RISCV_CPU(dev); > + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev); > + Error *local_err = NULL; > + > + cpu_exec_realizefn(cs, &local_err); > + if (local_err != NULL) { > + error_propagate(errp, local_err); > + return; > + } > + > + if (!riscv_running_KVM()) { > + riscv_cpu_realize_features(dev, &local_err); > + if (local_err != NULL) { > + error_propagate(errp, local_err); > + return; > + } > + } > > riscv_cpu_finalize_features(cpu, &local_err); > if (local_err != NULL) { > -- > 2.40.1 > > Otherwise, Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 938c7bd87b..72f5433776 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -331,6 +331,15 @@ static void set_satp_mode_default_map(RISCVCPU *cpu) } #endif +static bool riscv_running_KVM(void) +{ +#ifndef CONFIG_USER_ONLY + return kvm_enabled(); +#else + return false; +#endif +} + static void riscv_any_cpu_init(Object *obj) { RISCVCPU *cpu = RISCV_CPU(obj); @@ -1295,20 +1304,12 @@ static void riscv_cpu_validate_misa_priv(CPURISCVState *env, Error **errp) } } -static void riscv_cpu_realize(DeviceState *dev, Error **errp) +static void riscv_cpu_realize_features(DeviceState *dev, Error **errp) { - CPUState *cs = CPU(dev); RISCVCPU *cpu = RISCV_CPU(dev); CPURISCVState *env = &cpu->env; - RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev); Error *local_err = NULL; - cpu_exec_realizefn(cs, &local_err); - if (local_err != NULL) { - error_propagate(errp, local_err); - return; - } - riscv_cpu_validate_misa_mxl(cpu, &local_err); if (local_err != NULL) { error_propagate(errp, local_err); @@ -1354,6 +1355,28 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) } } #endif +} + +static void riscv_cpu_realize(DeviceState *dev, Error **errp) +{ + CPUState *cs = CPU(dev); + RISCVCPU *cpu = RISCV_CPU(dev); + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev); + Error *local_err = NULL; + + cpu_exec_realizefn(cs, &local_err); + if (local_err != NULL) { + error_propagate(errp, local_err); + return; + } + + if (!riscv_running_KVM()) { + riscv_cpu_realize_features(dev, &local_err); + if (local_err != NULL) { + error_propagate(errp, local_err); + return; + } + } riscv_cpu_finalize_features(cpu, &local_err); if (local_err != NULL) {
As it is today it's not possible to use '-cpu host' if the RISC-V host has RVH enabled. This is the resulting error: $ sudo ./qemu/build/qemu-system-riscv64 \ -machine virt,accel=kvm -m 2G -smp 1 \ -nographic -snapshot -kernel ./guest_imgs/Image \ -initrd ./guest_imgs/rootfs_kvm_riscv64.img \ -append "earlycon=sbi root=/dev/ram rw" \ -cpu host qemu-system-riscv64: H extension requires priv spec 1.12.0 This happens because we're checking for priv spec for all CPUs, and since we're not setting env->priv_ver for the 'host' CPU, it's being default to zero (i.e. PRIV_SPEC_1_10_0). In reality env->priv_ver does not make sense when running with the KVM 'host' CPU. It's used to gate certain CSRs/extensions during translation to make them unavailable if the hart declares an older spec version. It doesn't have any other use. E.g. OpenSBI version 1.2 retrieves the spec checking if the CSR_MCOUNTEREN, CSR_MCOUNTINHIBIT and CSR_MENVCFG CSRs are available [1]. 'priv_ver' is just one example. We're doing a lot of feature validation and setup during riscv_cpu_realize() that it doesn't apply KVM CPUs. Validating the feature set for those CPUs is a KVM problem that should be handled in KVM specific code. The new riscv_cpu_realize_features() helper contains all validation logic that are not applicable to KVM CPUs. riscv_cpu_realize() verifies if we're dealing with a KVM CPU and, if not, execute the new helper to proceed with the usual realize() logic for all other CPUs. [1] lib/sbi/sbi_hart.c, hart_detect_features() Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/cpu.c | 41 ++++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-)