diff mbox series

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

Message ID 20240526122612.473476-5-npiggin@gmail.com (mailing list archive)
State New, archived
Headers show
Series ppc/pnv: Better big-core model, lpar-per-core, PC unit | expand

Commit Message

Nicholas Piggin May 26, 2024, 12:26 p.m. UTC 
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(-)

Comments

Harsh Prateek Bora May 28, 2024, 7:10 a.m. UTC | #1 
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));
>
Cédric Le Goater May 28, 2024, 7:45 a.m. UTC | #2 
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));
Nicholas Piggin May 29, 2024, 12:18 a.m. UTC | #3 
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 mbox series

Patch

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));