Re: [RFC PATCH 04/10] ppc/pnv: specialise init for powernv8/9/10 machines

[Cédric Le Goater]

On 5/28/24 09:10, Harsh Prateek Bora wrote:
> Hi Nick,
>
> On 5/26/24 17:56, Nicholas Piggin wrote:
> > This will allow different settings and checks for different
> > machine types with later changes.
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   hw/ppc/pnv.c | 35 ++++++++++++++++++++++++++++++-----
> >   1 file changed, 30 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 6e3a5ccdec..a706de2e36 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -976,11 +976,6 @@ static void pnv_init(MachineState *machine)
> >       pnv->num_chips =
> >           machine->smp.max_cpus / (machine->smp.cores * machine->smp.threads);
> > -    if (machine->smp.threads > 8) {
> > -        error_report("Cannot support more than 8 threads/core "
> > -                     "on a powernv machine");
> > -        exit(1);
> > -    }
> >       if (!is_power_of_2(machine->smp.threads)) {
> >           error_report("Cannot support %d threads/core on a powernv"
> >                        "machine because it must be a power of 2",
> > @@ -1076,6 +1071,33 @@ static void pnv_init(MachineState *machine)
> >       }
> >   }
> > +static void pnv_power8_init(MachineState *machine)
> > +{
> > +    if (machine->smp.threads > 8) {
> > +        error_report("Cannot support more than 8 threads/core "
> > +                     "on a powernv POWER8 machine");
>
> We could use mc->desc for machine name above, so that ..
>
> > +        exit(1);
> > +    }
>
> with this patch, we can reuse p8 init for both p9 and p10 (and not just reuse 
> p9 for p10 with hard coded string?).

Good idea. You could add a 'max_smt' attribute to PnvMachineClass to limit
POWER8 to one.


Thanks,

C.


> With that,
> Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>
> > +
> > +    pnv_init(machine);
> > +}
> > +
> > +static void pnv_power9_init(MachineState *machine)
> > +{
> > +    if (machine->smp.threads > 8) {
> > +        error_report("Cannot support more than 8 threads/core "
> > +                     "on a powernv9/10 machine");
> > +        exit(1);
> > +    }
> > +
> > +    pnv_init(machine);
> > +}
> > +
> > +static void pnv_power10_init(MachineState *machine)
> > +{
> > +    pnv_power9_init(machine);
> > +}
> > +
> >   /*
> >    *    0:21  Reserved - Read as zeros
> >    *   22:24  Chip ID
> > @@ -2423,6 +2445,7 @@ static void pnv_machine_power8_class_init(ObjectClass 
> > *oc, void *data)
> >       };
> >       mc->desc = "IBM PowerNV (Non-Virtualized) POWER8";
> > +    mc->init = pnv_power8_init;
> >       mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
> >       compat_props_add(mc->compat_props, phb_compat, G_N_ELEMENTS(phb_compat));
> > @@ -2449,6 +2472,7 @@ static void pnv_machine_power9_class_init(ObjectClass 
> > *oc, void *data)
> >       };
> >       mc->desc = "IBM PowerNV (Non-Virtualized) POWER9";
> > +    mc->init = pnv_power9_init;
> >       mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.2");
> >       compat_props_add(mc->compat_props, phb_compat, G_N_ELEMENTS(phb_compat));
> > @@ -2473,6 +2497,7 @@ static void pnv_machine_p10_common_class_init(ObjectClass 
> > *oc, void *data)
> >           { TYPE_PNV_PHB_ROOT_PORT, "version", "5" },
> >       };
> > +    mc->init = pnv_power10_init;
> >       mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power10_v2.0");
> >       compat_props_add(mc->compat_props, phb_compat, G_N_ELEMENTS(phb_compat));