Re: [PATCH v4 03/11] ppc/pnv: New powernv10-rainier machine type

[Cédric Le Goater]

On 11/21/23 02:33, Nicholas Piggin wrote:
> On Tue Nov 21, 2023 at 9:51 AM AEST, Glenn Miles wrote:
> > Create a new powernv machine type, powernv10-rainier, that
> > will contain rainier-specific devices.
>
> Is the plan to have a base powernv10 common to all and then
> powernv10-rainier looks like a Rainier? Or would powernv10
> just be a rainier?
>
> It's fine to structure code this way, I'm just wondering about
> the machine types available to user. Is a base powernv10 machine
> useful to run?

There are multiple P10 boards defined in Linux :

  aspeed-bmc-ibm-bonnell.dts
  aspeed-bmc-ibm-everest.dts
  aspeed-bmc-ibm-rainier-1s4u.dts
  aspeed-bmc-ibm-rainier-4u.dts
  aspeed-bmc-ibm-rainier.dts

and we could model the machines above with a fixed number of sockets.
The "powernv10" would be the generic system that can be customized
at will on the command line, even I2C devices. There is also the
P10 Denali which is FSP based. This QEMU machine would certainly be
very different. I thought of doing the same for P9 with a -zaius
and include NPU2 models for it. I lacked time and the interest was
small at the time of OpenPOWER.

Anyhow, adding a new machine makes sense and it prepares ground for
possible new ones. I am OK with or without. As primary users, you are
the ones that can tell if there will be a second machine.
 
Thanks,

C.


> > Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
> > ---
> >   hw/ppc/pnv.c | 29 +++++++++++++++++++++++++++--
> >   1 file changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 9c29727337..3481a1220e 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -2249,7 +2249,7 @@ static void pnv_machine_power9_class_init(ObjectClass 
> > *oc, void *data)
> >       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PNV_PHB);
> >   }
> >   
> > -static void pnv_machine_power10_class_init(ObjectClass *oc, void *data)
> > +static void pnv_machine_p10_common_class_init(ObjectClass *oc, void *data)
> >   {
> >       MachineClass *mc = MACHINE_CLASS(oc);
> >       PnvMachineClass *pmc = PNV_MACHINE_CLASS(oc);
> > @@ -2261,7 +2261,6 @@ static void pnv_machine_power10_class_init(ObjectClass 
> > *oc, void *data)
> >           { TYPE_PNV_PHB_ROOT_PORT, "version", "5" },
> >       };
> >   
> > -    mc->desc = "IBM PowerNV (Non-Virtualized) POWER10";
> >       mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power10_v2.0");
> >       compat_props_add(mc->compat_props, phb_compat, G_N_ELEMENTS(phb_compat));
> >   
> > @@ -2274,6 +2273,23 @@ static void pnv_machine_power10_class_init(ObjectClass *oc, void *data)
> >       machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PNV_PHB);
> >   }
> >   
> > +static void pnv_machine_power10_class_init(ObjectClass *oc, void *data)
> > +{
> > +    MachineClass *mc = MACHINE_CLASS(oc);
> > +
> > +    pnv_machine_p10_common_class_init(oc, data);
> > +    mc->desc = "IBM PowerNV (Non-Virtualized) POWER10";
> > +
> > +}
> > +
> > +static void pnv_machine_p10_rainier_class_init(ObjectClass *oc, void *data)
> > +{
> > +    MachineClass *mc = MACHINE_CLASS(oc);
> > +
> > +    pnv_machine_p10_common_class_init(oc, data);
> > +    mc->desc = "IBM PowerNV (Non-Virtualized) POWER10 rainier";
> > +}
> > +
> >   static bool pnv_machine_get_hb(Object *obj, Error **errp)
> >   {
> >       PnvMachineState *pnv = PNV_MACHINE(obj);
> > @@ -2379,6 +2395,15 @@ static void pnv_machine_class_init(ObjectClass *oc, void 
> > *data)
> >       }
> >   
> >   static const TypeInfo types[] = {
> > +    {
> > +        .name          = MACHINE_TYPE_NAME("powernv10-rainier"),
> > +        .parent        = TYPE_PNV_MACHINE,
> > +        .class_init    = pnv_machine_p10_rainier_class_init,
> > +        .interfaces = (InterfaceInfo[]) {
> > +            { TYPE_XIVE_FABRIC },
> > +            { },
> > +        },
> > +    },
> >       {
> >           .name          = MACHINE_TYPE_NAME("powernv10"),
> >           .parent        = TYPE_PNV_MACHINE,