Message ID | 20230908060431.1903919-49-alistair.francis@wdc.com |
---|---|
State | New |
Headers | show |
Series | [PULL,01/65] target/riscv/cpu.c: do not run 'host' CPU with TCG | expand |
On 8/9/23 08:04, Alistair Francis wrote: > From: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > > 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 riscv_cpu_add_kvm_properties() 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. > > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > Reviewed-by: Andrew Jones <ajones@ventanamicro.com> > Message-ID: <20230901194627.1214811-4-dbarboza@ventanamicro.com> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > target/riscv/cpu.c | 65 ++++++++++++++++++++++++++++++---------------- > 1 file changed, 42 insertions(+), 23 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index db640e7460..8e6d316500 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -1943,6 +1943,46 @@ static void cpu_set_cfg_unavailable(Object *obj, Visitor *v, > } > #endif > > +#ifndef CONFIG_USER_ONLY #ifdef CONFIG_KVM ? > +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); > +} > + > +static void 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 > + > /* > * Add CPU properties with user-facing flags. > * > @@ -1958,39 +1998,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); > + 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]); > } > }
On 9/8/23 10:21, Philippe Mathieu-Daudé wrote: > On 8/9/23 08:04, Alistair Francis wrote: >> From: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >> >> 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 riscv_cpu_add_kvm_properties() 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. >> >> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> >> Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn> >> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> >> Reviewed-by: Andrew Jones <ajones@ventanamicro.com> >> Message-ID: <20230901194627.1214811-4-dbarboza@ventanamicro.com> >> Signed-off-by: Alistair Francis <alistair.francis@wdc.com> >> --- >> target/riscv/cpu.c | 65 ++++++++++++++++++++++++++++++---------------- >> 1 file changed, 42 insertions(+), 23 deletions(-) >> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >> index db640e7460..8e6d316500 100644 >> --- a/target/riscv/cpu.c >> +++ b/target/riscv/cpu.c >> @@ -1943,6 +1943,46 @@ static void cpu_set_cfg_unavailable(Object *obj, Visitor *v, >> } >> #endif >> +#ifndef CONFIG_USER_ONLY > > #ifdef CONFIG_KVM ? Yeah, CONFIG_KVM would be more fitting here. We're moving all this code to kvm.c in "[PATCH v2 00/19] riscv: split TCG/KVM accelerators from cpu.c", and we won't need neither in the end, so for now it's simpler to temporarily leave it like this. Thanks, Daniel > >> +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); >> +} >> + >> +static void 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 >> + >> /* >> * Add CPU properties with user-facing flags. >> * >> @@ -1958,39 +1998,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); >> + 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]); >> } >> } >
On Sun, Sep 10, 2023 at 6:58 PM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > > > On 9/8/23 10:21, Philippe Mathieu-Daudé wrote: > > On 8/9/23 08:04, Alistair Francis wrote: > >> From: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > >> > >> 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 riscv_cpu_add_kvm_properties() 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. > >> > >> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > >> Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn> > >> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > >> Reviewed-by: Andrew Jones <ajones@ventanamicro.com> > >> Message-ID: <20230901194627.1214811-4-dbarboza@ventanamicro.com> > >> Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > >> --- > >> target/riscv/cpu.c | 65 ++++++++++++++++++++++++++++++---------------- > >> 1 file changed, 42 insertions(+), 23 deletions(-) > >> > >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > >> index db640e7460..8e6d316500 100644 > >> --- a/target/riscv/cpu.c > >> +++ b/target/riscv/cpu.c > >> @@ -1943,6 +1943,46 @@ static void cpu_set_cfg_unavailable(Object *obj, Visitor *v, > >> } > >> #endif > >> +#ifndef CONFIG_USER_ONLY > > > > #ifdef CONFIG_KVM ? > > Yeah, CONFIG_KVM would be more fitting here. We're moving all this code to kvm.c > in "[PATCH v2 00/19] riscv: split TCG/KVM accelerators from cpu.c", and we won't > need neither in the end, so for now it's simpler to temporarily leave it like > this. There are CI failures, so I have converted this to #ifdef CONFIG_KVM which should fix them. Alistair > > > Thanks, > > Daniel > > > > >> +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); > >> +} > >> + > >> +static void 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 > >> + > >> /* > >> * Add CPU properties with user-facing flags. > >> * > >> @@ -1958,39 +1998,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); > >> + 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/cpu.c b/target/riscv/cpu.c index db640e7460..8e6d316500 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -1943,6 +1943,46 @@ static void cpu_set_cfg_unavailable(Object *obj, Visitor *v, } #endif +#ifndef CONFIG_USER_ONLY +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); +} + +static void 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 + /* * Add CPU properties with user-facing flags. * @@ -1958,39 +1998,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); + 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]); } }