| Message ID | 20240526122612.473476-5-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | ppc/pnv: Better big-core model, lpar-per-core, PC unit | expand |
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?). 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)); >
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));
On Tue May 28, 2024 at 5:45 PM AEST, Cédric Le Goater wrote: > 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. Okay I'll see how that goes. Good suggestions. Thanks, Nick
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"); + exit(1); + } + + 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));
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(-)