[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] ppc/pnv: Drop "num-chips" machine property
From: |
David Gibson |
Subject: |
Re: [PATCH] ppc/pnv: Drop "num-chips" machine property |
Date: |
Tue, 7 Jan 2020 10:49:19 +1100 |
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 <address@hidden>
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
signature.asc
Description: PGP signature