hw/ppc/pnv.c | 62 +++++++++++----------------------------------------------- 1 file changed, 12 insertions(+), 50 deletions(-)
The number of CPU chips of the powernv machine is configurable through a
"num-chips" property. This doesn't fit well with the CPU topology, eg.
some configurations can come up with more CPUs than the maximum of CPUs
set in the toplogy. This causes assertion to be hit with mttcg:
-machine powernv,num-chips=2 -smp cores=2 -accel tcg,thread=multi
ERROR:
tcg/tcg.c:789:tcg_register_thread: assertion failed: (n < ms->smp.max_cpus)
Aborted (core dumped)
Mttcg mandates the CPU topology to be dimensioned to the actual number
of CPUs, depending on the number of chips the user asked for. That is,
'-machine num-chips=N' should always have a '-smp' companion with a
topology that meats the resulting number of CPUs, typically
'-smp sockets=N'.
It thus seems that "num-chips" doesn't bring anything but forcing the user
to specify the requested number of chips on the command line twice. Simplify
the command line by computing the number of chips based on the CPU topology
exclusively. The powernv machine isn't a production thing ; it is mostly
used by developpers to prepare the bringup of real HW. Because of this and
for simplicity, this deliberately ignores the official deprecation process
and dumps "num-chips" right away : '-smp sockets=N' is now the only way to
control the number of CPU chips.
This is done at machine init because smp_parse() is called after instance
init.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/pnv.c | 62 +++++++++++-----------------------------------------------
1 file changed, 12 insertions(+), 50 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index f77e7ca84ede..b225ffbb2c41 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -768,6 +768,18 @@ static void pnv_init(MachineState *machine)
exit(1);
}
+ pnv->num_chips =
+ machine->smp.max_cpus / (machine->smp.cores * machine->smp.threads);
+ /*
+ * TODO: should we decide on how many chips we can create based
+ * on #cores and Venice vs. Murano vs. Naples chip type etc...,
+ */
+ if (!is_power_of_2(pnv->num_chips) || pnv->num_chips > 4) {
+ error_report("invalid number of chips: '%d'", pnv->num_chips);
+ error_printf("Try '-smp sockets=N'. Valid values are : 1, 2 or 4.\n");
+ exit(1);
+ }
+
pnv->chips = g_new0(PnvChip *, pnv->num_chips);
for (i = 0; i < pnv->num_chips; i++) {
char chip_name[32];
@@ -1696,53 +1708,6 @@ PnvChip *pnv_get_chip(uint32_t chip_id)
return NULL;
}
-static void pnv_get_num_chips(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
-{
- visit_type_uint32(v, name, &PNV_MACHINE(obj)->num_chips, errp);
-}
-
-static void pnv_set_num_chips(Object *obj, Visitor *v, const char *name,
- void *opaque, Error **errp)
-{
- PnvMachineState *pnv = PNV_MACHINE(obj);
- uint32_t num_chips;
- Error *local_err = NULL;
-
- visit_type_uint32(v, name, &num_chips, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
-
- /*
- * TODO: should we decide on how many chips we can create based
- * on #cores and Venice vs. Murano vs. Naples chip type etc...,
- */
- if (!is_power_of_2(num_chips) || num_chips > 4) {
- error_setg(errp, "invalid number of chips: '%d'", num_chips);
- return;
- }
-
- pnv->num_chips = num_chips;
-}
-
-static void pnv_machine_instance_init(Object *obj)
-{
- PnvMachineState *pnv = PNV_MACHINE(obj);
- pnv->num_chips = 1;
-}
-
-static void pnv_machine_class_props_init(ObjectClass *oc)
-{
- object_class_property_add(oc, "num-chips", "uint32",
- pnv_get_num_chips, pnv_set_num_chips,
- NULL, NULL, NULL);
- object_class_property_set_description(oc, "num-chips",
- "Specifies the number of processor chips",
- NULL);
-}
-
static void pnv_machine_power8_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -1812,8 +1777,6 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
*/
mc->default_ram_size = INITRD_LOAD_ADDR + INITRD_MAX_SIZE;
ispc->print_info = pnv_pic_print_info;
-
- pnv_machine_class_props_init(oc);
}
#define DEFINE_PNV8_CHIP_TYPE(type, class_initfn) \
@@ -1866,7 +1829,6 @@ static const TypeInfo types[] = {
.parent = TYPE_MACHINE,
.abstract = true,
.instance_size = sizeof(PnvMachineState),
- .instance_init = pnv_machine_instance_init,
.class_init = pnv_machine_class_init,
.class_size = sizeof(PnvMachineClass),
.interfaces = (InterfaceInfo[]) {
On Mon, Jan 06, 2020 at 11:29:42AM +0100, Greg Kurz wrote:
> The number of CPU chips of the powernv machine is configurable through a
> "num-chips" property. This doesn't fit well with the CPU topology, eg.
> some configurations can come up with more CPUs than the maximum of CPUs
> set in the toplogy. This causes assertion to be hit with mttcg:
>
> -machine powernv,num-chips=2 -smp cores=2 -accel tcg,thread=multi
>
> ERROR:
> tcg/tcg.c:789:tcg_register_thread: assertion failed: (n < ms->smp.max_cpus)
> Aborted (core dumped)
>
> Mttcg mandates the CPU topology to be dimensioned to the actual number
> of CPUs, depending on the number of chips the user asked for. That is,
> '-machine num-chips=N' should always have a '-smp' companion with a
> topology that meats the resulting number of CPUs, typically
> '-smp sockets=N'.
>
> It thus seems that "num-chips" doesn't bring anything but forcing the user
> to specify the requested number of chips on the command line twice. Simplify
> the command line by computing the number of chips based on the CPU topology
> exclusively. The powernv machine isn't a production thing ; it is mostly
> used by developpers to prepare the bringup of real HW. Because of this and
> for simplicity, this deliberately ignores the official deprecation process
> and dumps "num-chips" right away : '-smp sockets=N' is now the only way to
> control the number of CPU chips.
>
> This is done at machine init because smp_parse() is called after instance
> init.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
Applied to ppc-for-5.0, thanks.
> ---
> hw/ppc/pnv.c | 62 +++++++++++-----------------------------------------------
> 1 file changed, 12 insertions(+), 50 deletions(-)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index f77e7ca84ede..b225ffbb2c41 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -768,6 +768,18 @@ static void pnv_init(MachineState *machine)
> exit(1);
> }
>
> + pnv->num_chips =
> + machine->smp.max_cpus / (machine->smp.cores * machine->smp.threads);
> + /*
> + * TODO: should we decide on how many chips we can create based
> + * on #cores and Venice vs. Murano vs. Naples chip type etc...,
> + */
> + if (!is_power_of_2(pnv->num_chips) || pnv->num_chips > 4) {
> + error_report("invalid number of chips: '%d'", pnv->num_chips);
> + error_printf("Try '-smp sockets=N'. Valid values are : 1, 2 or 4.\n");
> + exit(1);
> + }
> +
> pnv->chips = g_new0(PnvChip *, pnv->num_chips);
> for (i = 0; i < pnv->num_chips; i++) {
> char chip_name[32];
> @@ -1696,53 +1708,6 @@ PnvChip *pnv_get_chip(uint32_t chip_id)
> return NULL;
> }
>
> -static void pnv_get_num_chips(Object *obj, Visitor *v, const char *name,
> - void *opaque, Error **errp)
> -{
> - visit_type_uint32(v, name, &PNV_MACHINE(obj)->num_chips, errp);
> -}
> -
> -static void pnv_set_num_chips(Object *obj, Visitor *v, const char *name,
> - void *opaque, Error **errp)
> -{
> - PnvMachineState *pnv = PNV_MACHINE(obj);
> - uint32_t num_chips;
> - Error *local_err = NULL;
> -
> - visit_type_uint32(v, name, &num_chips, &local_err);
> - if (local_err) {
> - error_propagate(errp, local_err);
> - return;
> - }
> -
> - /*
> - * TODO: should we decide on how many chips we can create based
> - * on #cores and Venice vs. Murano vs. Naples chip type etc...,
> - */
> - if (!is_power_of_2(num_chips) || num_chips > 4) {
> - error_setg(errp, "invalid number of chips: '%d'", num_chips);
> - return;
> - }
> -
> - pnv->num_chips = num_chips;
> -}
> -
> -static void pnv_machine_instance_init(Object *obj)
> -{
> - PnvMachineState *pnv = PNV_MACHINE(obj);
> - pnv->num_chips = 1;
> -}
> -
> -static void pnv_machine_class_props_init(ObjectClass *oc)
> -{
> - object_class_property_add(oc, "num-chips", "uint32",
> - pnv_get_num_chips, pnv_set_num_chips,
> - NULL, NULL, NULL);
> - object_class_property_set_description(oc, "num-chips",
> - "Specifies the number of processor chips",
> - NULL);
> -}
> -
> static void pnv_machine_power8_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> @@ -1812,8 +1777,6 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
> */
> mc->default_ram_size = INITRD_LOAD_ADDR + INITRD_MAX_SIZE;
> ispc->print_info = pnv_pic_print_info;
> -
> - pnv_machine_class_props_init(oc);
> }
>
> #define DEFINE_PNV8_CHIP_TYPE(type, class_initfn) \
> @@ -1866,7 +1829,6 @@ static const TypeInfo types[] = {
> .parent = TYPE_MACHINE,
> .abstract = true,
> .instance_size = sizeof(PnvMachineState),
> - .instance_init = pnv_machine_instance_init,
> .class_init = pnv_machine_class_init,
> .class_size = sizeof(PnvMachineClass),
> .interfaces = (InterfaceInfo[]) {
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On 1/6/20 11:29 AM, Greg Kurz wrote:
> The number of CPU chips of the powernv machine is configurable through a
> "num-chips" property. This doesn't fit well with the CPU topology, eg.
> some configurations can come up with more CPUs than the maximum of CPUs
> set in the toplogy. This causes assertion to be hit with mttcg:
>
> -machine powernv,num-chips=2 -smp cores=2 -accel tcg,thread=multi
>
> ERROR:
> tcg/tcg.c:789:tcg_register_thread: assertion failed: (n < ms->smp.max_cpus)
> Aborted (core dumped)
>
> Mttcg mandates the CPU topology to be dimensioned to the actual number
> of CPUs, depending on the number of chips the user asked for. That is,
> '-machine num-chips=N' should always have a '-smp' companion with a
> topology that meats the resulting number of CPUs, typically
> '-smp sockets=N'.
>
> It thus seems that "num-chips" doesn't bring anything but forcing the user
> to specify the requested number of chips on the command line twice. Simplify
> the command line by computing the number of chips based on the CPU topology
> exclusively. The powernv machine isn't a production thing ; it is mostly
> used by developpers to prepare the bringup of real HW. Because of this and
> for simplicity, this deliberately ignores the official deprecation process
> and dumps "num-chips" right away : '-smp sockets=N' is now the only way to
> control the number of CPU chips.
>
> This is done at machine init because smp_parse() is called after instance
> init.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> hw/ppc/pnv.c | 62 +++++++++++-----------------------------------------------
> 1 file changed, 12 insertions(+), 50 deletions(-)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index f77e7ca84ede..b225ffbb2c41 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -768,6 +768,18 @@ static void pnv_init(MachineState *machine)
> exit(1);
> }
>
> + pnv->num_chips =
> + machine->smp.max_cpus / (machine->smp.cores * machine->smp.threads);
> + /*
> + * TODO: should we decide on how many chips we can create based
> + * on #cores and Venice vs. Murano vs. Naples chip type etc...,
> + */
> + if (!is_power_of_2(pnv->num_chips) || pnv->num_chips > 4) {
> + error_report("invalid number of chips: '%d'", pnv->num_chips);
> + error_printf("Try '-smp sockets=N'. Valid values are : 1, 2 or 4.\n");
> + exit(1);
> + }
> +
> pnv->chips = g_new0(PnvChip *, pnv->num_chips);
> for (i = 0; i < pnv->num_chips; i++) {
> char chip_name[32];
> @@ -1696,53 +1708,6 @@ PnvChip *pnv_get_chip(uint32_t chip_id)
> return NULL;
> }
>
> -static void pnv_get_num_chips(Object *obj, Visitor *v, const char *name,
> - void *opaque, Error **errp)
> -{
> - visit_type_uint32(v, name, &PNV_MACHINE(obj)->num_chips, errp);
> -}
> -
> -static void pnv_set_num_chips(Object *obj, Visitor *v, const char *name,
> - void *opaque, Error **errp)
> -{
> - PnvMachineState *pnv = PNV_MACHINE(obj);
> - uint32_t num_chips;
> - Error *local_err = NULL;
> -
> - visit_type_uint32(v, name, &num_chips, &local_err);
> - if (local_err) {
> - error_propagate(errp, local_err);
> - return;
> - }
> -
> - /*
> - * TODO: should we decide on how many chips we can create based
> - * on #cores and Venice vs. Murano vs. Naples chip type etc...,
> - */
> - if (!is_power_of_2(num_chips) || num_chips > 4) {
> - error_setg(errp, "invalid number of chips: '%d'", num_chips);
> - return;
> - }
> -
> - pnv->num_chips = num_chips;
> -}
> -
> -static void pnv_machine_instance_init(Object *obj)
> -{
> - PnvMachineState *pnv = PNV_MACHINE(obj);
> - pnv->num_chips = 1;
> -}
> -
> -static void pnv_machine_class_props_init(ObjectClass *oc)
> -{
> - object_class_property_add(oc, "num-chips", "uint32",
> - pnv_get_num_chips, pnv_set_num_chips,
> - NULL, NULL, NULL);
> - object_class_property_set_description(oc, "num-chips",
> - "Specifies the number of processor chips",
> - NULL);
> -}
> -
> static void pnv_machine_power8_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> @@ -1812,8 +1777,6 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
> */
> mc->default_ram_size = INITRD_LOAD_ADDR + INITRD_MAX_SIZE;
> ispc->print_info = pnv_pic_print_info;
> -
> - pnv_machine_class_props_init(oc);
> }
>
> #define DEFINE_PNV8_CHIP_TYPE(type, class_initfn) \
> @@ -1866,7 +1829,6 @@ static const TypeInfo types[] = {
> .parent = TYPE_MACHINE,
> .abstract = true,
> .instance_size = sizeof(PnvMachineState),
> - .instance_init = pnv_machine_instance_init,
> .class_init = pnv_machine_class_init,
> .class_size = sizeof(PnvMachineClass),
> .interfaces = (InterfaceInfo[]) {
>
© 2016 - 2024 Red Hat, Inc.