Message ID | 20230706101738.460804-14-dbarboza@ventanamicro.com |
---|---|
State | New |
Headers | show |
Series | target/riscv, KVM: fixes and enhancements | expand |
On Thu, Jul 6, 2023 at 8:20 PM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > Our design philosophy with KVM properties can be resumed in two main > decisions based on KVM interface availability and what the user wants to > do: > > - if the user disables an extension that the host KVM module doesn't > know about (i.e. it doesn't implement the kvm_get_one_reg() interface), > keep booting the CPU. This will avoid users having to deal with issues > with older KVM versions while disabling features they don't care; > > - for any other case we're going to error out immediately. If the user > wants to enable a feature that KVM doesn't know about this a problem that > is worth aborting - the user must know that the feature wasn't enabled > in the hart. Likewise, if KVM knows about the extension, the user wants > to enable/disable it, and we fail to do it so, that's also a problem we > can't shrug it off. > > In the case of MISA bits we won't even try enabling bits that aren't > already available in the host. The ioctl() is so likely to fail that > it's not worth trying. This check is already done in the previous patch, > in kvm_cpu_set_misa_ext_cfg(), thus we don't need to worry about it now. > > In kvm_riscv_update_cpu_misa_ext() we'll go through every potential user > option and do as follows: > > - if the user didn't set the property or set to the same value of the > host, do nothing; > > - Disable the given extension in KVM. Error out if anything goes wrong. > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > Reviewed-by: Andrew Jones <ajones@ventanamicro.com> Acked-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/kvm.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c > index c55d0ec7ab..7afd6024e6 100644 > --- a/target/riscv/kvm.c > +++ b/target/riscv/kvm.c > @@ -162,6 +162,41 @@ static void kvm_cpu_set_misa_ext_cfg(Object *obj, Visitor *v, > "enabled in the host", misa_ext_cfg->name); > } > > +static void kvm_riscv_update_cpu_misa_ext(RISCVCPU *cpu, CPUState *cs) > +{ > + CPURISCVState *env = &cpu->env; > + uint64_t id, reg; > + int i, ret; > + > + for (i = 0; i < ARRAY_SIZE(kvm_misa_ext_cfgs); i++) { > + KVMCPUConfig *misa_cfg = &kvm_misa_ext_cfgs[i]; > + target_ulong misa_bit = misa_cfg->offset; > + > + if (!misa_cfg->user_set) { > + continue; > + } > + > + /* If we're here we're going to disable the MISA bit */ > + reg = 0; > + id = kvm_riscv_reg_id(env, KVM_REG_RISCV_ISA_EXT, > + misa_cfg->kvm_reg_id); > + ret = kvm_set_one_reg(cs, id, ®); > + if (ret != 0) { > + /* > + * We're not checking for -EINVAL because if the bit is about > + * to be disabled, it means that it was already enabled by > + * KVM. We determined that by fetching the 'isa' register > + * during init() time. Any error at this point is worth > + * aborting. > + */ > + error_report("Unable to set KVM reg %s, error %d", > + misa_cfg->name, ret); > + exit(EXIT_FAILURE); > + } > + env->misa_ext &= ~misa_bit; > + } > +} > + > static void kvm_riscv_add_cpu_user_properties(Object *cpu_obj) > { > int i; > @@ -632,8 +667,13 @@ int kvm_arch_init_vcpu(CPUState *cs) > > if (!object_dynamic_cast(OBJECT(cpu), TYPE_RISCV_CPU_HOST)) { > ret = kvm_vcpu_set_machine_ids(cpu, cs); > + if (ret != 0) { > + return ret; > + } > } > > + kvm_riscv_update_cpu_misa_ext(cpu, cs); > + > return ret; > } > > -- > 2.41.0 > >
diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c index c55d0ec7ab..7afd6024e6 100644 --- a/target/riscv/kvm.c +++ b/target/riscv/kvm.c @@ -162,6 +162,41 @@ static void kvm_cpu_set_misa_ext_cfg(Object *obj, Visitor *v, "enabled in the host", misa_ext_cfg->name); } +static void kvm_riscv_update_cpu_misa_ext(RISCVCPU *cpu, CPUState *cs) +{ + CPURISCVState *env = &cpu->env; + uint64_t id, reg; + int i, ret; + + for (i = 0; i < ARRAY_SIZE(kvm_misa_ext_cfgs); i++) { + KVMCPUConfig *misa_cfg = &kvm_misa_ext_cfgs[i]; + target_ulong misa_bit = misa_cfg->offset; + + if (!misa_cfg->user_set) { + continue; + } + + /* If we're here we're going to disable the MISA bit */ + reg = 0; + id = kvm_riscv_reg_id(env, KVM_REG_RISCV_ISA_EXT, + misa_cfg->kvm_reg_id); + ret = kvm_set_one_reg(cs, id, ®); + if (ret != 0) { + /* + * We're not checking for -EINVAL because if the bit is about + * to be disabled, it means that it was already enabled by + * KVM. We determined that by fetching the 'isa' register + * during init() time. Any error at this point is worth + * aborting. + */ + error_report("Unable to set KVM reg %s, error %d", + misa_cfg->name, ret); + exit(EXIT_FAILURE); + } + env->misa_ext &= ~misa_bit; + } +} + static void kvm_riscv_add_cpu_user_properties(Object *cpu_obj) { int i; @@ -632,8 +667,13 @@ int kvm_arch_init_vcpu(CPUState *cs) if (!object_dynamic_cast(OBJECT(cpu), TYPE_RISCV_CPU_HOST)) { ret = kvm_vcpu_set_machine_ids(cpu, cs); + if (ret != 0) { + return ret; + } } + kvm_riscv_update_cpu_misa_ext(cpu, cs); + return ret; }