[Qemu-devel] [PATCH] ppc/pnv: Introduce PowerNV machines with fixed CPU models

Cédric Le Goater posted 1 patch 4 years, 9 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
hw/ppc/pnv.c | 97 +++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 85 insertions(+), 12 deletions(-)
[Qemu-devel] [PATCH] ppc/pnv: Introduce PowerNV machines with fixed CPU models
Posted by Cédric Le Goater 4 years, 9 months ago
Make the current "powernv" machine an abstract type and derive from it
new machines with specific CPU models: power8, power8e, power8nvl,
power9.

The "powernv" machine is now an alias on the "powernv9" machine.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/pnv.c | 97 +++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 85 insertions(+), 12 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 8cef6d17f111..36f57479a1f5 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -600,6 +600,7 @@ static void pnv_chip_power9_pic_print_info(PnvChip *chip, Monitor *mon)
 static void pnv_init(MachineState *machine)
 {
     PnvMachineState *pnv = PNV_MACHINE(machine);
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
     MemoryRegion *ram;
     char *fw_filename;
     long fw_size;
@@ -659,16 +660,25 @@ static void pnv_init(MachineState *machine)
         }
     }
 
-    /* Create the processor chips */
-    i = strlen(machine->cpu_type) - strlen(POWERPC_CPU_TYPE_SUFFIX);
-    chip_typename = g_strdup_printf(PNV_CHIP_TYPE_NAME("%.*s"),
-                                    i, machine->cpu_type);
-    if (!object_class_by_name(chip_typename)) {
-        error_report("invalid CPU model '%.*s' for %s machine",
-                     i, machine->cpu_type, MACHINE_GET_CLASS(machine)->name);
+    /*
+     * Do a sanity check on the specified CPU to check compatibility
+     * with the machine default. In the future, we might want to
+     * create the PnvChip with a compatible CPU model but for now, we
+     * use the machine default.
+     */
+    if (object_class_by_name(mc->default_cpu_type) !=
+        object_class_by_name(machine->cpu_type)) {
+        error_report("invalid CPU model '%s' for %s machine",
+                     machine->cpu_type, mc->name);
         exit(1);
     }
 
+    /* Create the processor chips */
+    i = strlen(mc->default_cpu_type) - strlen(POWERPC_CPU_TYPE_SUFFIX);
+    chip_typename = g_strdup_printf(PNV_CHIP_TYPE_NAME("%.*s"),
+                                    i, mc->default_cpu_type);
+    assert(object_class_by_name(chip_typename));
+
     pnv->chips = g_new0(PnvChip *, pnv->num_chips);
     for (i = 0; i < pnv->num_chips; i++) {
         char chip_name[32];
@@ -1343,25 +1353,69 @@ static void pnv_machine_class_props_init(ObjectClass *oc)
                               NULL);
 }
 
-static void pnv_machine_class_init(ObjectClass *oc, void *data)
+static void pnv_machine_power8e_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
+
+    mc->desc = "IBM PowerNV (Non-Virtualized) POWER8E";
+    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8e_v2.1");
+
+    xic->icp_get = pnv_icp_get;
+    xic->ics_get = pnv_ics_get;
+    xic->ics_resend = pnv_ics_resend;
+}
+
+static void pnv_machine_power8_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
     XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
+
+    mc->desc = "IBM PowerNV (Non-Virtualized) POWER8";
+    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
+
+    xic->icp_get = pnv_icp_get;
+    xic->ics_get = pnv_ics_get;
+    xic->ics_resend = pnv_ics_resend;
+}
+
+static void pnv_machine_power8nvl_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
+
+    mc->desc = "IBM PowerNV (Non-Virtualized) POWER8NVL";
+    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8nvl_v1.0");
+
+    xic->icp_get = pnv_icp_get;
+    xic->ics_get = pnv_ics_get;
+    xic->ics_resend = pnv_ics_resend;
+}
+
+static void pnv_machine_power9_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->desc = "IBM PowerNV (Non-Virtualized) POWER9";
+    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
+
+    mc->alias = "powernv";
+}
+
+static void pnv_machine_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
     InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
 
     mc->desc = "IBM PowerNV (Non-Virtualized)";
     mc->init = pnv_init;
     mc->reset = pnv_reset;
     mc->max_cpus = MAX_CPUS;
-    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
     mc->block_default_type = IF_IDE; /* Pnv provides a AHCI device for
                                       * storage */
     mc->no_parallel = 1;
     mc->default_boot_order = NULL;
     mc->default_ram_size = 2 * GiB;
-    xic->icp_get = pnv_icp_get;
-    xic->ics_get = pnv_ics_get;
-    xic->ics_resend = pnv_ics_resend;
     ispc->print_info = pnv_pic_print_info;
 
     pnv_machine_class_props_init(oc);
@@ -1381,10 +1435,29 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
         .parent        = TYPE_PNV9_CHIP,          \
     }
 
+#define DEFINE_PNV_MACHINE_TYPE(cpu, class_initfn)      \
+    {                                                   \
+        .name          = MACHINE_TYPE_NAME(cpu),        \
+        .parent        = TYPE_PNV_MACHINE,              \
+        .instance_size = sizeof(PnvMachineState),       \
+        .instance_init = pnv_machine_instance_init,     \
+        .class_init    = class_initfn,                  \
+        .interfaces = (InterfaceInfo[]) {               \
+            { TYPE_XICS_FABRIC },                       \
+            { TYPE_INTERRUPT_STATS_PROVIDER },          \
+            { },                                        \
+        },                                              \
+    }
+
 static const TypeInfo types[] = {
+    DEFINE_PNV_MACHINE_TYPE("powernv8e", pnv_machine_power8e_class_init),
+    DEFINE_PNV_MACHINE_TYPE("powernv8", pnv_machine_power8_class_init),
+    DEFINE_PNV_MACHINE_TYPE("powernv8nvl", pnv_machine_power8nvl_class_init),
+    DEFINE_PNV_MACHINE_TYPE("powernv9", pnv_machine_power9_class_init),
     {
         .name          = TYPE_PNV_MACHINE,
         .parent        = TYPE_MACHINE,
+        .abstract       = true,
         .instance_size = sizeof(PnvMachineState),
         .instance_init = pnv_machine_instance_init,
         .class_init    = pnv_machine_class_init,
-- 
2.21.0


Re: [Qemu-devel] [PATCH] ppc/pnv: Introduce PowerNV machines with fixed CPU models
Posted by David Gibson 4 years, 9 months ago
On Mon, Jul 22, 2019 at 08:23:47PM +0200, Cédric Le Goater wrote:
> Make the current "powernv" machine an abstract type and derive from it
> new machines with specific CPU models: power8, power8e, power8nvl,
> power9.
> 
> The "powernv" machine is now an alias on the "powernv9" machine.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Ah, sorry, I wasn't clear here.  I don't think we need a different
machine type for every cpu model, I just think we should have powernv8
and powernv9.  POWER8E and POWER8NVL don't significantly change the
system design (IIUC) so they can still be done with "-machine powernv8
-cpu POWER8E" or whatever.  I expect the same will be true for POWER9'
when that comes along

> ---
>  hw/ppc/pnv.c | 97 +++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 85 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 8cef6d17f111..36f57479a1f5 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -600,6 +600,7 @@ static void pnv_chip_power9_pic_print_info(PnvChip *chip, Monitor *mon)
>  static void pnv_init(MachineState *machine)
>  {
>      PnvMachineState *pnv = PNV_MACHINE(machine);
> +    MachineClass *mc = MACHINE_GET_CLASS(machine);
>      MemoryRegion *ram;
>      char *fw_filename;
>      long fw_size;
> @@ -659,16 +660,25 @@ static void pnv_init(MachineState *machine)
>          }
>      }
>  
> -    /* Create the processor chips */
> -    i = strlen(machine->cpu_type) - strlen(POWERPC_CPU_TYPE_SUFFIX);
> -    chip_typename = g_strdup_printf(PNV_CHIP_TYPE_NAME("%.*s"),
> -                                    i, machine->cpu_type);
> -    if (!object_class_by_name(chip_typename)) {
> -        error_report("invalid CPU model '%.*s' for %s machine",
> -                     i, machine->cpu_type, MACHINE_GET_CLASS(machine)->name);
> +    /*
> +     * Do a sanity check on the specified CPU to check compatibility
> +     * with the machine default. In the future, we might want to
> +     * create the PnvChip with a compatible CPU model but for now, we
> +     * use the machine default.
> +     */
> +    if (object_class_by_name(mc->default_cpu_type) !=
> +        object_class_by_name(machine->cpu_type)) {
> +        error_report("invalid CPU model '%s' for %s machine",
> +                     machine->cpu_type, mc->name);
>          exit(1);
>      }
>  
> +    /* Create the processor chips */
> +    i = strlen(mc->default_cpu_type) - strlen(POWERPC_CPU_TYPE_SUFFIX);
> +    chip_typename = g_strdup_printf(PNV_CHIP_TYPE_NAME("%.*s"),
> +                                    i, mc->default_cpu_type);
> +    assert(object_class_by_name(chip_typename));
> +
>      pnv->chips = g_new0(PnvChip *, pnv->num_chips);
>      for (i = 0; i < pnv->num_chips; i++) {
>          char chip_name[32];
> @@ -1343,25 +1353,69 @@ static void pnv_machine_class_props_init(ObjectClass *oc)
>                                NULL);
>  }
>  
> -static void pnv_machine_class_init(ObjectClass *oc, void *data)
> +static void pnv_machine_power8e_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +    XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
> +
> +    mc->desc = "IBM PowerNV (Non-Virtualized) POWER8E";
> +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8e_v2.1");
> +
> +    xic->icp_get = pnv_icp_get;
> +    xic->ics_get = pnv_ics_get;
> +    xic->ics_resend = pnv_ics_resend;
> +}
> +
> +static void pnv_machine_power8_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
>      XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
> +
> +    mc->desc = "IBM PowerNV (Non-Virtualized) POWER8";
> +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
> +
> +    xic->icp_get = pnv_icp_get;
> +    xic->ics_get = pnv_ics_get;
> +    xic->ics_resend = pnv_ics_resend;
> +}
> +
> +static void pnv_machine_power8nvl_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +    XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
> +
> +    mc->desc = "IBM PowerNV (Non-Virtualized) POWER8NVL";
> +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8nvl_v1.0");
> +
> +    xic->icp_get = pnv_icp_get;
> +    xic->ics_get = pnv_ics_get;
> +    xic->ics_resend = pnv_ics_resend;
> +}
> +
> +static void pnv_machine_power9_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
> +    mc->desc = "IBM PowerNV (Non-Virtualized) POWER9";
> +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
> +
> +    mc->alias = "powernv";
> +}
> +
> +static void pnv_machine_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
>      InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
>  
>      mc->desc = "IBM PowerNV (Non-Virtualized)";
>      mc->init = pnv_init;
>      mc->reset = pnv_reset;
>      mc->max_cpus = MAX_CPUS;
> -    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
>      mc->block_default_type = IF_IDE; /* Pnv provides a AHCI device for
>                                        * storage */
>      mc->no_parallel = 1;
>      mc->default_boot_order = NULL;
>      mc->default_ram_size = 2 * GiB;
> -    xic->icp_get = pnv_icp_get;
> -    xic->ics_get = pnv_ics_get;
> -    xic->ics_resend = pnv_ics_resend;
>      ispc->print_info = pnv_pic_print_info;
>  
>      pnv_machine_class_props_init(oc);
> @@ -1381,10 +1435,29 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
>          .parent        = TYPE_PNV9_CHIP,          \
>      }
>  
> +#define DEFINE_PNV_MACHINE_TYPE(cpu, class_initfn)      \
> +    {                                                   \
> +        .name          = MACHINE_TYPE_NAME(cpu),        \
> +        .parent        = TYPE_PNV_MACHINE,              \
> +        .instance_size = sizeof(PnvMachineState),       \
> +        .instance_init = pnv_machine_instance_init,     \
> +        .class_init    = class_initfn,                  \
> +        .interfaces = (InterfaceInfo[]) {               \
> +            { TYPE_XICS_FABRIC },                       \
> +            { TYPE_INTERRUPT_STATS_PROVIDER },          \
> +            { },                                        \
> +        },                                              \
> +    }
> +
>  static const TypeInfo types[] = {
> +    DEFINE_PNV_MACHINE_TYPE("powernv8e", pnv_machine_power8e_class_init),
> +    DEFINE_PNV_MACHINE_TYPE("powernv8", pnv_machine_power8_class_init),
> +    DEFINE_PNV_MACHINE_TYPE("powernv8nvl", pnv_machine_power8nvl_class_init),
> +    DEFINE_PNV_MACHINE_TYPE("powernv9", pnv_machine_power9_class_init),
>      {
>          .name          = TYPE_PNV_MACHINE,
>          .parent        = TYPE_MACHINE,
> +        .abstract       = true,
>          .instance_size = sizeof(PnvMachineState),
>          .instance_init = pnv_machine_instance_init,
>          .class_init    = pnv_machine_class_init,

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [PATCH] ppc/pnv: Introduce PowerNV machines with fixed CPU models
Posted by Cédric Le Goater 4 years, 9 months ago
On 23/07/2019 03:38, David Gibson wrote:
> On Mon, Jul 22, 2019 at 08:23:47PM +0200, Cédric Le Goater wrote:
>> Make the current "powernv" machine an abstract type and derive from it
>> new machines with specific CPU models: power8, power8e, power8nvl,
>> power9.
>>
>> The "powernv" machine is now an alias on the "powernv9" machine.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> Ah, sorry, I wasn't clear here.  I don't think we need a different
> machine type for every cpu model, I just think we should have powernv8
> and powernv9.  POWER8E and POWER8NVL don't significantly change the
> system design (IIUC) so they can still be done with "-machine powernv8
> -cpu POWER8E" or whatever.  I expect the same will be true for POWER9'
> when that comes along

I understand but I am afraid we will to have one machine per CPU family 
because POWER8E and POWER8NVL already have their own PnvChip : 

    DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8, pnv_chip_power8_class_init),
    DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8E, pnv_chip_power8e_class_init),
    DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8NVL,
                          pnv_chip_power8nvl_class_init),

C.

>> ---
>>  hw/ppc/pnv.c | 97 +++++++++++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 85 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index 8cef6d17f111..36f57479a1f5 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -600,6 +600,7 @@ static void pnv_chip_power9_pic_print_info(PnvChip *chip, Monitor *mon)
>>  static void pnv_init(MachineState *machine)
>>  {
>>      PnvMachineState *pnv = PNV_MACHINE(machine);
>> +    MachineClass *mc = MACHINE_GET_CLASS(machine);
>>      MemoryRegion *ram;
>>      char *fw_filename;
>>      long fw_size;
>> @@ -659,16 +660,25 @@ static void pnv_init(MachineState *machine)
>>          }
>>      }
>>  
>> -    /* Create the processor chips */
>> -    i = strlen(machine->cpu_type) - strlen(POWERPC_CPU_TYPE_SUFFIX);
>> -    chip_typename = g_strdup_printf(PNV_CHIP_TYPE_NAME("%.*s"),
>> -                                    i, machine->cpu_type);
>> -    if (!object_class_by_name(chip_typename)) {
>> -        error_report("invalid CPU model '%.*s' for %s machine",
>> -                     i, machine->cpu_type, MACHINE_GET_CLASS(machine)->name);
>> +    /*
>> +     * Do a sanity check on the specified CPU to check compatibility
>> +     * with the machine default. In the future, we might want to
>> +     * create the PnvChip with a compatible CPU model but for now, we
>> +     * use the machine default.
>> +     */
>> +    if (object_class_by_name(mc->default_cpu_type) !=
>> +        object_class_by_name(machine->cpu_type)) {
>> +        error_report("invalid CPU model '%s' for %s machine",
>> +                     machine->cpu_type, mc->name);
>>          exit(1);
>>      }
>>  
>> +    /* Create the processor chips */
>> +    i = strlen(mc->default_cpu_type) - strlen(POWERPC_CPU_TYPE_SUFFIX);
>> +    chip_typename = g_strdup_printf(PNV_CHIP_TYPE_NAME("%.*s"),
>> +                                    i, mc->default_cpu_type);
>> +    assert(object_class_by_name(chip_typename));
>> +
>>      pnv->chips = g_new0(PnvChip *, pnv->num_chips);
>>      for (i = 0; i < pnv->num_chips; i++) {
>>          char chip_name[32];
>> @@ -1343,25 +1353,69 @@ static void pnv_machine_class_props_init(ObjectClass *oc)
>>                                NULL);
>>  }
>>  
>> -static void pnv_machine_class_init(ObjectClass *oc, void *data)
>> +static void pnv_machine_power8e_class_init(ObjectClass *oc, void *data)
>> +{
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +    XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
>> +
>> +    mc->desc = "IBM PowerNV (Non-Virtualized) POWER8E";
>> +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8e_v2.1");
>> +
>> +    xic->icp_get = pnv_icp_get;
>> +    xic->ics_get = pnv_ics_get;
>> +    xic->ics_resend = pnv_ics_resend;
>> +}
>> +
>> +static void pnv_machine_power8_class_init(ObjectClass *oc, void *data)
>>  {
>>      MachineClass *mc = MACHINE_CLASS(oc);
>>      XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
>> +
>> +    mc->desc = "IBM PowerNV (Non-Virtualized) POWER8";
>> +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
>> +
>> +    xic->icp_get = pnv_icp_get;
>> +    xic->ics_get = pnv_ics_get;
>> +    xic->ics_resend = pnv_ics_resend;
>> +}
>> +
>> +static void pnv_machine_power8nvl_class_init(ObjectClass *oc, void *data)
>> +{
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +    XICSFabricClass *xic = XICS_FABRIC_CLASS(oc);
>> +
>> +    mc->desc = "IBM PowerNV (Non-Virtualized) POWER8NVL";
>> +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8nvl_v1.0");
>> +
>> +    xic->icp_get = pnv_icp_get;
>> +    xic->ics_get = pnv_ics_get;
>> +    xic->ics_resend = pnv_ics_resend;
>> +}
>> +
>> +static void pnv_machine_power9_class_init(ObjectClass *oc, void *data)
>> +{
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>> +
>> +    mc->desc = "IBM PowerNV (Non-Virtualized) POWER9";
>> +    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
>> +
>> +    mc->alias = "powernv";
>> +}
>> +
>> +static void pnv_machine_class_init(ObjectClass *oc, void *data)
>> +{
>> +    MachineClass *mc = MACHINE_CLASS(oc);
>>      InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc);
>>  
>>      mc->desc = "IBM PowerNV (Non-Virtualized)";
>>      mc->init = pnv_init;
>>      mc->reset = pnv_reset;
>>      mc->max_cpus = MAX_CPUS;
>> -    mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power8_v2.0");
>>      mc->block_default_type = IF_IDE; /* Pnv provides a AHCI device for
>>                                        * storage */
>>      mc->no_parallel = 1;
>>      mc->default_boot_order = NULL;
>>      mc->default_ram_size = 2 * GiB;
>> -    xic->icp_get = pnv_icp_get;
>> -    xic->ics_get = pnv_ics_get;
>> -    xic->ics_resend = pnv_ics_resend;
>>      ispc->print_info = pnv_pic_print_info;
>>  
>>      pnv_machine_class_props_init(oc);
>> @@ -1381,10 +1435,29 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data)
>>          .parent        = TYPE_PNV9_CHIP,          \
>>      }
>>  
>> +#define DEFINE_PNV_MACHINE_TYPE(cpu, class_initfn)      \
>> +    {                                                   \
>> +        .name          = MACHINE_TYPE_NAME(cpu),        \
>> +        .parent        = TYPE_PNV_MACHINE,              \
>> +        .instance_size = sizeof(PnvMachineState),       \
>> +        .instance_init = pnv_machine_instance_init,     \
>> +        .class_init    = class_initfn,                  \
>> +        .interfaces = (InterfaceInfo[]) {               \
>> +            { TYPE_XICS_FABRIC },                       \
>> +            { TYPE_INTERRUPT_STATS_PROVIDER },          \
>> +            { },                                        \
>> +        },                                              \
>> +    }
>> +
>>  static const TypeInfo types[] = {
>> +    DEFINE_PNV_MACHINE_TYPE("powernv8e", pnv_machine_power8e_class_init),
>> +    DEFINE_PNV_MACHINE_TYPE("powernv8", pnv_machine_power8_class_init),
>> +    DEFINE_PNV_MACHINE_TYPE("powernv8nvl", pnv_machine_power8nvl_class_init),
>> +    DEFINE_PNV_MACHINE_TYPE("powernv9", pnv_machine_power9_class_init),
>>      {
>>          .name          = TYPE_PNV_MACHINE,
>>          .parent        = TYPE_MACHINE,
>> +        .abstract       = true,
>>          .instance_size = sizeof(PnvMachineState),
>>          .instance_init = pnv_machine_instance_init,
>>          .class_init    = pnv_machine_class_init,
> 


Re: [Qemu-devel] [PATCH] ppc/pnv: Introduce PowerNV machines with fixed CPU models
Posted by David Gibson 4 years, 9 months ago
On Tue, Jul 23, 2019 at 08:00:27AM +0200, Cédric Le Goater wrote:
> On 23/07/2019 03:38, David Gibson wrote:
> > On Mon, Jul 22, 2019 at 08:23:47PM +0200, Cédric Le Goater wrote:
> >> Make the current "powernv" machine an abstract type and derive from it
> >> new machines with specific CPU models: power8, power8e, power8nvl,
> >> power9.
> >>
> >> The "powernv" machine is now an alias on the "powernv9" machine.
> >>
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > 
> > Ah, sorry, I wasn't clear here.  I don't think we need a different
> > machine type for every cpu model, I just think we should have powernv8
> > and powernv9.  POWER8E and POWER8NVL don't significantly change the
> > system design (IIUC) so they can still be done with "-machine powernv8
> > -cpu POWER8E" or whatever.  I expect the same will be true for POWER9'
> > when that comes along
> 
> I understand but I am afraid we will to have one machine per CPU family 
> because POWER8E and POWER8NVL already have their own PnvChip : 
> 
>     DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8, pnv_chip_power8_class_init),
>     DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8E, pnv_chip_power8e_class_init),
>     DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8NVL,
>                           pnv_chip_power8nvl_class_init),

Hrm.  Is there an actual reason we need different chip classes for
these?  Even if there is, I don't see an inherent reason that implies
separate machine classes as well.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [PATCH] ppc/pnv: Introduce PowerNV machines with fixed CPU models
Posted by Cédric Le Goater 4 years, 9 months ago
On 23/07/2019 08:37, David Gibson wrote:
> On Tue, Jul 23, 2019 at 08:00:27AM +0200, Cédric Le Goater wrote:
>> On 23/07/2019 03:38, David Gibson wrote:
>>> On Mon, Jul 22, 2019 at 08:23:47PM +0200, Cédric Le Goater wrote:
>>>> Make the current "powernv" machine an abstract type and derive from it
>>>> new machines with specific CPU models: power8, power8e, power8nvl,
>>>> power9.
>>>>
>>>> The "powernv" machine is now an alias on the "powernv9" machine.
>>>>
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>
>>> Ah, sorry, I wasn't clear here.  I don't think we need a different
>>> machine type for every cpu model, I just think we should have powernv8
>>> and powernv9.  POWER8E and POWER8NVL don't significantly change the
>>> system design (IIUC) so they can still be done with "-machine powernv8
>>> -cpu POWER8E" or whatever.  I expect the same will be true for POWER9'
>>> when that comes along
>>
>> I understand but I am afraid we will to have one machine per CPU family 
>> because POWER8E and POWER8NVL already have their own PnvChip : 
>>
>>     DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8, pnv_chip_power8_class_init),
>>     DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8E, pnv_chip_power8e_class_init),
>>     DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8NVL,
>>                           pnv_chip_power8nvl_class_init),
> 
> Hrm.  Is there an actual reason we need different chip classes for
> these?  
The POWER8 (8E, 8, 8NVL) chips differ by the core : 

    target/ppc/cpu-models.h:    CPU_POWERPC_POWER8E_BASE       = 0x004B0000,
    target/ppc/cpu-models.h:    CPU_POWERPC_POWER8_BASE        = 0x004D0000,
    target/ppc/cpu-models.h:    CPU_POWERPC_POWER8NVL_BASE     = 0x004C0000,

The chip model is different :

    k->chip_cfam_id = 0x221ef04980000000ull; /* P8 Murano DD2.1 */
    k->chip_cfam_id = 0x220ea04980000000ull; /* P8 Venice DD2.0 */
    k->chip_cfam_id = 0x120d304980000000ull; /* P8 Naples DD1.0 */
 
The chiplets are different also. NVL has a NPU and a different LPC bus which
is modeled. There are different number of PHBs.

> Even if there is, I don't see an inherent reason that implies separate
> machine classes as well. 

I will see if we can have a single powernv8 machine.

C.

Re: [Qemu-devel] [PATCH] ppc/pnv: Introduce PowerNV machines with fixed CPU models
Posted by David Gibson 4 years, 9 months ago
On Wed, Jul 24, 2019 at 11:32:00AM +0200, Cédric Le Goater wrote:
> On 23/07/2019 08:37, David Gibson wrote:
> > On Tue, Jul 23, 2019 at 08:00:27AM +0200, Cédric Le Goater wrote:
> >> On 23/07/2019 03:38, David Gibson wrote:
> >>> On Mon, Jul 22, 2019 at 08:23:47PM +0200, Cédric Le Goater wrote:
> >>>> Make the current "powernv" machine an abstract type and derive from it
> >>>> new machines with specific CPU models: power8, power8e, power8nvl,
> >>>> power9.
> >>>>
> >>>> The "powernv" machine is now an alias on the "powernv9" machine.
> >>>>
> >>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >>>
> >>> Ah, sorry, I wasn't clear here.  I don't think we need a different
> >>> machine type for every cpu model, I just think we should have powernv8
> >>> and powernv9.  POWER8E and POWER8NVL don't significantly change the
> >>> system design (IIUC) so they can still be done with "-machine powernv8
> >>> -cpu POWER8E" or whatever.  I expect the same will be true for POWER9'
> >>> when that comes along
> >>
> >> I understand but I am afraid we will to have one machine per CPU family 
> >> because POWER8E and POWER8NVL already have their own PnvChip : 
> >>
> >>     DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8, pnv_chip_power8_class_init),
> >>     DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8E, pnv_chip_power8e_class_init),
> >>     DEFINE_PNV8_CHIP_TYPE(TYPE_PNV_CHIP_POWER8NVL,
> >>                           pnv_chip_power8nvl_class_init),
> > 
> > Hrm.  Is there an actual reason we need different chip classes for
> > these?  
> The POWER8 (8E, 8, 8NVL) chips differ by the core : 
> 
>     target/ppc/cpu-models.h:    CPU_POWERPC_POWER8E_BASE       = 0x004B0000,
>     target/ppc/cpu-models.h:    CPU_POWERPC_POWER8_BASE        = 0x004D0000,
>     target/ppc/cpu-models.h:    CPU_POWERPC_POWER8NVL_BASE     = 0x004C0000,
> 
> The chip model is different :
> 
>     k->chip_cfam_id = 0x221ef04980000000ull; /* P8 Murano DD2.1 */
>     k->chip_cfam_id = 0x220ea04980000000ull; /* P8 Venice DD2.0 */
>     k->chip_cfam_id = 0x120d304980000000ull; /* P8 Naples DD1.0 */
>  
> The chiplets are different also. NVL has a NPU and a different LPC bus which
> is modeled. There are different number of PHBs.

Ah, ok.  That seems like enough reason.

> 
> > Even if there is, I don't see an inherent reason that implies separate
> > machine classes as well. 
> 
> I will see if we can have a single powernv8 machine.
> 
> C.
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson