On Sat, 21 Dec 2019 11:39:06 +1100 David Gibson <da...@gibson.dropbear.id.au> wrote:
> On Fri, Dec 20, 2019 at 05:51:48PM +0100, Greg Kurz wrote:
> > Multi TCG 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'.
> >
> > Simplify the command line for these setups by computing the default
> > number of chips based on the CPU topology, ie. no need to explicitely
> > set "num-chips" anymore. This must be done at machine init because
> > smp_parse() is called after instance init.
> >
> > Signed-off-by: Greg Kurz <gr...@kaod.org>
>
> Is there actually any reason to retain num-chips at all? Or could we
> just set the number of chips equal to the number of sockets, which
> seems to make sense to me.
>
I don't quite know why "num-chips" was introduced in the first place... so
yes, if it turns out it isn't needed, I'll gladly drop the property.
> > ---
> > hw/ppc/pnv.c | 23 ++++++++++++++++-------
> > 1 file changed, 16 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index f8cf2b6d760f..9b777b7084a0 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -768,6 +768,19 @@ static void pnv_init(MachineState *machine)
> > exit(1);
> > }
> >
> > + if (!pnv->num_chips) {
> > + uint32_t num_chips =
> > + machine->smp.max_cpus / (machine->smp.cores *
> > machine->smp.threads);
> > + Error *local_err = NULL;
> > +
> > + object_property_set_uint(OBJECT(pnv), num_chips, "num-chips",
> > + &local_err);
> > + if (local_err) {
> > + error_report_err(local_err);
> > + exit(1);
> > + }
> > + }
> > +
> > pnv->chips = g_new0(PnvChip *, pnv->num_chips);
> > for (i = 0; i < pnv->num_chips; i++) {
> > char chip_name[32];
> > @@ -1722,6 +1735,9 @@ static void pnv_set_num_chips(Object *obj, Visitor
> > *v, const char *name,
> > */
> > if (!is_power_of_2(num_chips) || num_chips > 4) {
> > error_setg(errp, "invalid number of chips: '%d'", num_chips);
> > + error_append_hint(errp,
> > + "Set 'num-chips' implicitely with '-smp
> > sockets=N'. "
> > + "Valid values are : 1, 2 or 4.\n");
> > return;
> > }
> >
> > @@ -1735,12 +1751,6 @@ static void pnv_set_num_chips(Object *obj, Visitor
> > *v, const char *name,
> > 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",
> > @@ -1874,7 +1884,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[]) {
> >
>
pgpAAf5J9yCAG.pgp
Description: OpenPGP digital signature
