Message ID | 20230912132423.268494-4-dbarboza@ventanamicro.com |
---|---|
State | New |
Headers | show |
Series | riscv: 'max' CPU, detect user choice in TCG | expand |
On Tue, Sep 12, 2023 at 10:24:06AM -0300, Daniel Henrique Barboza wrote: > Future patches will split the existing Property arrays even further, and > the existing code in riscv_cpu_add_user_properties() will start to scale > bad with it because it's dealing with KVM constraints mixed in with TCG > constraints. We're going to pay a high price to share a couple of common > lines of code between the two. > > Create a new kvm_riscv_cpu_add_kvm_properties() helper that will be > forked from riscv_cpu_add_user_properties() if we're running KVM. The > helper includes all properties that a KVM CPU will add. The rest of > riscv_cpu_add_user_properties() body will then be relieved from having > to deal with KVM constraints. > > The helper was declared in kvm_stubs.h, while being implemented in > cpu.c, to allow '--enable-debug' builds to work. The compiler won't > remove the kvm_riscv_cpu_add_kvm_properties() reference when > 'kvm_enabled()' is false if we end up with an unused function. Even > though being a KVM only helper we can't implement it in kvm.c due to its > many dependencies inside cpu.c, so make it public in kvm_riscv.h and > keep its implementation in cpu.c for now. We'll move it to kvm.c in the > near future. > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > target/riscv/cpu.c | 65 +++++++++++++++++++++++++--------------- > target/riscv/kvm_riscv.h | 3 ++ > 2 files changed, 44 insertions(+), 24 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index db640e7460..7b7c5649e7 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -1924,7 +1924,7 @@ static Property riscv_cpu_options[] = { > DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64), > }; > > -#ifndef CONFIG_USER_ONLY > +#ifdef CONFIG_KVM > static void cpu_set_cfg_unavailable(Object *obj, Visitor *v, > const char *name, > void *opaque, Error **errp) > @@ -1941,6 +1941,44 @@ static void cpu_set_cfg_unavailable(Object *obj, Visitor *v, > propname); > } > } > + > +static void riscv_cpu_add_kvm_unavail_prop(Object *obj, const char *prop_name) > +{ > + /* Check if KVM created the property already */ > + if (object_property_find(obj, prop_name)) { > + return; > + } > + > + /* > + * Set the default to disabled for every extension > + * unknown to KVM and error out if the user attempts > + * to enable any of them. > + */ > + object_property_add(obj, prop_name, "bool", > + NULL, cpu_set_cfg_unavailable, > + NULL, (void *)prop_name); > +} > + > +void kvm_riscv_cpu_add_kvm_properties(Object *obj) > +{ > + Property *prop; > + DeviceState *dev = DEVICE(obj); > + > + kvm_riscv_init_user_properties(obj); > + riscv_cpu_add_misa_properties(obj); > + > + for (prop = riscv_cpu_extensions; prop && prop->name; prop++) { > + riscv_cpu_add_kvm_unavail_prop(obj, prop->name); > + } > + > + for (int i = 0; i < ARRAY_SIZE(riscv_cpu_options); i++) { > + /* Check if KVM created the property already */ > + if (object_property_find(obj, riscv_cpu_options[i].name)) { > + continue; > + } > + qdev_property_add_static(dev, &riscv_cpu_options[i]); > + } > +} > #endif IIUC, we're leaving kvm_riscv_cpu_add_kvm_properties() in cpu.c due to its dependencies, such as riscv_cpu_add_misa_properties(), but I don't understand why it needs to be public, since it's still only called from this file. Also, to handle the clang debug build I'd guess we still need a !CONFIG_KVM stub, so, in this file, I suggest we do something like, #ifdef CONFIG_KVM static void kvm_riscv_cpu_add_kvm_properties(obj) { ... } #else static void kvm_riscv_cpu_add_kvm_properties(obj) {} #endif But, all that said, I know this is all getting reworked in another in-flight series, so I'm not real worried about exactly how it comes together at the moment. Assuming we pass all compile tests like the clang debug test, then Reviewed-by: Andrew Jones <ajones@ventanamicro.com> Thanks, drew
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index db640e7460..7b7c5649e7 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -1924,7 +1924,7 @@ static Property riscv_cpu_options[] = { DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64), }; -#ifndef CONFIG_USER_ONLY +#ifdef CONFIG_KVM static void cpu_set_cfg_unavailable(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) @@ -1941,6 +1941,44 @@ static void cpu_set_cfg_unavailable(Object *obj, Visitor *v, propname); } } + +static void riscv_cpu_add_kvm_unavail_prop(Object *obj, const char *prop_name) +{ + /* Check if KVM created the property already */ + if (object_property_find(obj, prop_name)) { + return; + } + + /* + * Set the default to disabled for every extension + * unknown to KVM and error out if the user attempts + * to enable any of them. + */ + object_property_add(obj, prop_name, "bool", + NULL, cpu_set_cfg_unavailable, + NULL, (void *)prop_name); +} + +void kvm_riscv_cpu_add_kvm_properties(Object *obj) +{ + Property *prop; + DeviceState *dev = DEVICE(obj); + + kvm_riscv_init_user_properties(obj); + riscv_cpu_add_misa_properties(obj); + + for (prop = riscv_cpu_extensions; prop && prop->name; prop++) { + riscv_cpu_add_kvm_unavail_prop(obj, prop->name); + } + + for (int i = 0; i < ARRAY_SIZE(riscv_cpu_options); i++) { + /* Check if KVM created the property already */ + if (object_property_find(obj, riscv_cpu_options[i].name)) { + continue; + } + qdev_property_add_static(dev, &riscv_cpu_options[i]); + } +} #endif /* @@ -1958,39 +1996,18 @@ static void riscv_cpu_add_user_properties(Object *obj) riscv_add_satp_mode_properties(obj); if (kvm_enabled()) { - kvm_riscv_init_user_properties(obj); + kvm_riscv_cpu_add_kvm_properties(obj); + return; } #endif riscv_cpu_add_misa_properties(obj); for (prop = riscv_cpu_extensions; prop && prop->name; prop++) { -#ifndef CONFIG_USER_ONLY - if (kvm_enabled()) { - /* 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. - */ - object_property_add(obj, prop->name, "bool", - NULL, cpu_set_cfg_unavailable, - NULL, (void *)prop->name); - continue; - } -#endif qdev_property_add_static(dev, prop); } for (int i = 0; i < ARRAY_SIZE(riscv_cpu_options); i++) { - /* Check if KVM created the property already */ - if (object_property_find(obj, riscv_cpu_options[i].name)) { - continue; - } qdev_property_add_static(dev, &riscv_cpu_options[i]); } } diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h index de8c209ebc..69e807fbfb 100644 --- a/target/riscv/kvm_riscv.h +++ b/target/riscv/kvm_riscv.h @@ -19,6 +19,9 @@ #ifndef QEMU_KVM_RISCV_H #define QEMU_KVM_RISCV_H +/* Temporarily implemented in cpu.c */ +void kvm_riscv_cpu_add_kvm_properties(Object *obj); + void kvm_riscv_init_user_properties(Object *cpu_obj); void kvm_riscv_reset_vcpu(RISCVCPU *cpu); void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level);
Future patches will split the existing Property arrays even further, and the existing code in riscv_cpu_add_user_properties() will start to scale bad with it because it's dealing with KVM constraints mixed in with TCG constraints. We're going to pay a high price to share a couple of common lines of code between the two. Create a new kvm_riscv_cpu_add_kvm_properties() helper that will be forked from riscv_cpu_add_user_properties() if we're running KVM. The helper includes all properties that a KVM CPU will add. The rest of riscv_cpu_add_user_properties() body will then be relieved from having to deal with KVM constraints. The helper was declared in kvm_stubs.h, while being implemented in cpu.c, to allow '--enable-debug' builds to work. The compiler won't remove the kvm_riscv_cpu_add_kvm_properties() reference when 'kvm_enabled()' is false if we end up with an unused function. Even though being a KVM only helper we can't implement it in kvm.c due to its many dependencies inside cpu.c, so make it public in kvm_riscv.h and keep its implementation in cpu.c for now. We'll move it to kvm.c in the near future. Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/cpu.c | 65 +++++++++++++++++++++++++--------------- target/riscv/kvm_riscv.h | 3 ++ 2 files changed, 44 insertions(+), 24 deletions(-)