Message ID | 20230627163203.49422-17-dbarboza@ventanamicro.com |
---|---|
State | New |
Headers | show |
Series | target/riscv, KVM: fixes and enhancements | expand |
On Tue, Jun 27, 2023 at 01:32:00PM -0300, Daniel Henrique Barboza wrote: > KVM-specific properties are being created inside target/riscv/kvm.c. But > at this moment we're gathering all the remaining properties from TCG and > adding them as is when running KVM. This creates a situation where > non-KVM properties are setting flags to 'true' due to its default > settings (e.g. Zawrs). Users can also freely enable them via command > line. > > This doesn't impact runtime per se because KVM doesn't care about these > flags, but code such as riscv_isa_string_ext() take those flags into > account. The result is that, for a KVM guest, setting non-KVM properties > will make them appear in the riscv,isa DT. > > We want to keep the same API for both TCG and KVM and at the same time, > when running KVM, forbid non-KVM extensions to be enabled internally. We > accomplish both by changing riscv_cpu_add_user_properties() to add a > mock boolean property for every non-KVM extension in > riscv_cpu_extensions[]. Then, when running KVM, users are still free to > set extensions at will, but we'll error out if a non-KVM extension is > enabled. Setting such extension to 'false' will be ignored. > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > target/riscv/cpu.c | 42 +++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 39 insertions(+), 3 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index b65db165cc..22851b0e93 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -1720,6 +1720,24 @@ static Property riscv_cpu_extensions[] = { > DEFINE_PROP_END_OF_LIST(), > }; > > + > +static void cpu_set_cfg_unavailable(Object *obj, Visitor *v, > + const char *name, > + void *opaque, Error **errp) > +{ > + const char *propname = opaque; > + bool value; > + > + if (!visit_type_bool(v, name, &value, errp)) { > + return; > + } > + > + if (value) { > + error_setg(errp, "extension %s is not available with KVM", > + propname); > + } > +} > + > /* > * Add CPU properties with user-facing flags. > * > @@ -1738,9 +1756,27 @@ static void riscv_cpu_add_user_properties(Object *obj) > riscv_cpu_add_misa_properties(obj); > > for (prop = riscv_cpu_extensions; prop && prop->name; prop++) { > - /* Check if KVM didn't create the property already */ > - if (object_property_find(obj, prop->name)) { > - continue; > + if (riscv_running_kvm()) { > + /* Check if KVM created the property already */ The comment change should have been fixed on patch 12 where it was introduced. > + if (object_property_find(obj, prop->name)) { > + continue; > + } > + > + /* > + * Set the default to disabled for every extension > + * unknown to KVM and error out if the user attempts > + * to enable any of them. > + * > + * We're giving a pass for non-bool properties since they're > + * not related to the availability of extensions and can be > + * safely ignored as is. > + */ > + if (prop->info == &qdev_prop_bool) { > + object_property_add(obj, prop->name, "bool", > + NULL, cpu_set_cfg_unavailable, > + NULL, (void *)prop->name); > + continue; > + } > } > > qdev_property_add_static(dev, prop); > -- > 2.41.0 > Otherwise, Reviewed-by: Andrew Jones <ajones@ventanamicro.com> Thanks, drew
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index b65db165cc..22851b0e93 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -1720,6 +1720,24 @@ static Property riscv_cpu_extensions[] = { DEFINE_PROP_END_OF_LIST(), }; + +static void cpu_set_cfg_unavailable(Object *obj, Visitor *v, + const char *name, + void *opaque, Error **errp) +{ + const char *propname = opaque; + bool value; + + if (!visit_type_bool(v, name, &value, errp)) { + return; + } + + if (value) { + error_setg(errp, "extension %s is not available with KVM", + propname); + } +} + /* * Add CPU properties with user-facing flags. * @@ -1738,9 +1756,27 @@ static void riscv_cpu_add_user_properties(Object *obj) riscv_cpu_add_misa_properties(obj); for (prop = riscv_cpu_extensions; prop && prop->name; prop++) { - /* Check if KVM didn't create the property already */ - if (object_property_find(obj, prop->name)) { - continue; + if (riscv_running_kvm()) { + /* Check if KVM created the property already */ + if (object_property_find(obj, prop->name)) { + continue; + } + + /* + * Set the default to disabled for every extension + * unknown to KVM and error out if the user attempts + * to enable any of them. + * + * We're giving a pass for non-bool properties since they're + * not related to the availability of extensions and can be + * safely ignored as is. + */ + if (prop->info == &qdev_prop_bool) { + object_property_add(obj, prop->name, "bool", + NULL, cpu_set_cfg_unavailable, + NULL, (void *)prop->name); + continue; + } } qdev_property_add_static(dev, prop);
KVM-specific properties are being created inside target/riscv/kvm.c. But at this moment we're gathering all the remaining properties from TCG and adding them as is when running KVM. This creates a situation where non-KVM properties are setting flags to 'true' due to its default settings (e.g. Zawrs). Users can also freely enable them via command line. This doesn't impact runtime per se because KVM doesn't care about these flags, but code such as riscv_isa_string_ext() take those flags into account. The result is that, for a KVM guest, setting non-KVM properties will make them appear in the riscv,isa DT. We want to keep the same API for both TCG and KVM and at the same time, when running KVM, forbid non-KVM extensions to be enabled internally. We accomplish both by changing riscv_cpu_add_user_properties() to add a mock boolean property for every non-KVM extension in riscv_cpu_extensions[]. Then, when running KVM, users are still free to set extensions at will, but we'll error out if a non-KVM extension is enabled. Setting such extension to 'false' will be ignored. Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/cpu.c | 42 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-)