diff mbox series

[18/23] ppc: pnv: use generic cpu_model parsing

Message ID 1507220690-265042-19-git-send-email-imammedo@redhat.com
State New
Headers show
Series generalize parsing of cpu_model (part 3/PPC) | expand

Commit Message

Igor Mammedov Oct. 5, 2017, 4:24 p.m. UTC
use common cpu_model prasing in vl.c and set default cpu_model
using generic MachineClass::default_cpu_type.

Beside of switching to generic infrastructure it solves several
issues.

 * ppc_cpu_class_by_name() is used to deal with lower/upper case
   and alias translations into actual cpu type, which fixes
    '-M powernv -cpu power8' and '-M powernv -cpu power9_v1.0'
   usecases which error out with:
    'invalid CPU model 'FOO' for powernv machine'
 * allows to switch to lower-case typenames in pnv chip/core name
   (by convention typnames should be lower-case)
 * replace aliased names /power8, power9, .../ with exact cpu model
   names (i.e. typenames should be stable but aliases might decide to
   point to other cpu model withi family or changed by kvm). It will
   also help to simplify pnv_chip/core code and get rid of dependency
   on cpu_model parsing.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/ppc/pnv.h |  8 ++++----
 hw/ppc/pnv.c         | 22 ++++++++++------------
 hw/ppc/pnv_core.c    |  2 +-
 3 files changed, 15 insertions(+), 17 deletions(-)

Comments

Cédric Le Goater Oct. 6, 2017, 6:21 a.m. UTC | #1
On 10/05/2017 06:24 PM, Igor Mammedov wrote:
> use common cpu_model prasing in vl.c and set default cpu_model
> using generic MachineClass::default_cpu_type.
> 
> Beside of switching to generic infrastructure it solves several
> issues.
> 
>  * ppc_cpu_class_by_name() is used to deal with lower/upper case
>    and alias translations into actual cpu type, which fixes
>     '-M powernv -cpu power8' and '-M powernv -cpu power9_v1.0'
>    usecases which error out with:
>     'invalid CPU model 'FOO' for powernv machine'
>  * allows to switch to lower-case typenames in pnv chip/core name
>    (by convention typnames should be lower-case)
>  * replace aliased names /power8, power9, .../ with exact cpu model
>    names (i.e. typenames should be stable but aliases might decide to
>    point to other cpu model withi family or changed by kvm). It will
>    also help to simplify pnv_chip/core code and get rid of dependency
>    on cpu_model parsing.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>  include/hw/ppc/pnv.h |  8 ++++----
>  hw/ppc/pnv.c         | 22 ++++++++++------------
>  hw/ppc/pnv_core.c    |  2 +-
>  3 files changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 9c5437d..2525f7f 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -80,19 +80,19 @@ typedef struct PnvChipClass {
>      uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
>  } PnvChipClass;
>  
> -#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-POWER8E"
> +#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-power8e_v2.1"
>  #define PNV_CHIP_POWER8E(obj) \
>      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8E)
>  
> -#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-POWER8"
> +#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-power8_v2.0"
>  #define PNV_CHIP_POWER8(obj) \
>      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8)
>  
> -#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-POWER8NVL"
> +#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-power8nvl_v1.0"
>  #define PNV_CHIP_POWER8NVL(obj) \
>      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8NVL)
>  
> -#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-POWER9"
> +#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-power9_v1.0"
>  #define PNV_CHIP_POWER9(obj) \
>      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER9)
>  
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index d46d91c..4169837 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -607,16 +607,13 @@ static void ppc_powernv_init(MachineState *machine)
>          }
>      }
>  
> -    /* We need some cpu model to instantiate the PnvChip class */
> -    if (machine->cpu_model == NULL) {
> -        machine->cpu_model = "POWER8";
> -    }
> -
>      /* Create the processor chips */
> -    chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%s", machine->cpu_model);
> +    i = strlen(machine->cpu_type) - strlen(POWERPC_CPU_TYPE_SUFFIX);
> +    chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%.*s",
> +                                    i, machine->cpu_type);
>      if (!object_class_by_name(chip_typename)) {
> -        error_report("invalid CPU model '%s' for %s machine",
> -                     machine->cpu_model, MACHINE_GET_CLASS(machine)->name);
> +        error_report("invalid CPU model '%.*s' for %s machine",
> +                     i, machine->cpu_type, MACHINE_GET_CLASS(machine)->name);
>          exit(1);
>      }
>  
> @@ -716,7 +713,7 @@ static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -    k->cpu_model = "POWER8E";
> +    k->cpu_model = "power8e_v2.1";
>      k->chip_type = PNV_CHIP_POWER8E;
>      k->chip_cfam_id = 0x221ef04980000000ull;  /* P8 Murano DD2.1 */
>      k->cores_mask = POWER8E_CORE_MASK;
> @@ -738,7 +735,7 @@ static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -    k->cpu_model = "POWER8";
> +    k->cpu_model = "power8_v2.0";
>      k->chip_type = PNV_CHIP_POWER8;
>      k->chip_cfam_id = 0x220ea04980000000ull; /* P8 Venice DD2.0 */
>      k->cores_mask = POWER8_CORE_MASK;
> @@ -760,7 +757,7 @@ static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -    k->cpu_model = "POWER8NVL";
> +    k->cpu_model = "power8nvl_v1.0";
>      k->chip_type = PNV_CHIP_POWER8NVL;
>      k->chip_cfam_id = 0x120d304980000000ull;  /* P8 Naples DD1.0 */
>      k->cores_mask = POWER8_CORE_MASK;
> @@ -782,7 +779,7 @@ static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -    k->cpu_model = "POWER9";
> +    k->cpu_model = "power9_v1.0";
>      k->chip_type = PNV_CHIP_POWER9;
>      k->chip_cfam_id = 0x100d104980000000ull; /* P9 Nimbus DD1.0 */
>      k->cores_mask = POWER9_CORE_MASK;
> @@ -1133,6 +1130,7 @@ static void powernv_machine_class_init(ObjectClass *oc, void *data)
>      mc->init = ppc_powernv_init;
>      mc->reset = ppc_powernv_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;
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index 6726483..44b0b24 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -227,7 +227,7 @@ static const TypeInfo pnv_core_info = {
>  };
>  
>  static const char *pnv_core_models[] = {
> -    "POWER8E", "POWER8", "POWER8NVL", "POWER9"
> +    "power8e_v2.1", "power8_v2.0", "power8nvl_v1.0", "power9_v1.0"
>  };
>  
>  static void pnv_core_register_types(void)
>
David Gibson Oct. 6, 2017, 8:34 a.m. UTC | #2
On Thu, Oct 05, 2017 at 06:24:45PM +0200, Igor Mammedov wrote:
> use common cpu_model prasing in vl.c and set default cpu_model
> using generic MachineClass::default_cpu_type.
> 
> Beside of switching to generic infrastructure it solves several
> issues.
> 
>  * ppc_cpu_class_by_name() is used to deal with lower/upper case
>    and alias translations into actual cpu type, which fixes
>     '-M powernv -cpu power8' and '-M powernv -cpu power9_v1.0'
>    usecases which error out with:
>     'invalid CPU model 'FOO' for powernv machine'
>  * allows to switch to lower-case typenames in pnv chip/core name
>    (by convention typnames should be lower-case)
>  * replace aliased names /power8, power9, .../ with exact cpu model
>    names (i.e. typenames should be stable but aliases might decide to
>    point to other cpu model withi family or changed by kvm). It will
>    also help to simplify pnv_chip/core code and get rid of dependency
>    on cpu_model parsing.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/hw/ppc/pnv.h |  8 ++++----
>  hw/ppc/pnv.c         | 22 ++++++++++------------
>  hw/ppc/pnv_core.c    |  2 +-
>  3 files changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> index 9c5437d..2525f7f 100644
> --- a/include/hw/ppc/pnv.h
> +++ b/include/hw/ppc/pnv.h
> @@ -80,19 +80,19 @@ typedef struct PnvChipClass {
>      uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
>  } PnvChipClass;
>  
> -#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-POWER8E"
> +#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-power8e_v2.1"
>  #define PNV_CHIP_POWER8E(obj) \
>      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8E)
>  
> -#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-POWER8"
> +#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-power8_v2.0"
>  #define PNV_CHIP_POWER8(obj) \
>      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8)
>  
> -#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-POWER8NVL"
> +#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-power8nvl_v1.0"
>  #define PNV_CHIP_POWER8NVL(obj) \
>      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8NVL)
>  
> -#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-POWER9"
> +#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-power9_v1.0"

Uh.. we really should add a DD2 power9 before we make this change.
Making a DD1.0 (read, buggy as hell) chip the default is not
sensible.  Especially since we don't implement the various DD1 bugs
and differences in qemu.

>  #define PNV_CHIP_POWER9(obj) \
>      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER9)
>  
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index d46d91c..4169837 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -607,16 +607,13 @@ static void ppc_powernv_init(MachineState *machine)
>          }
>      }
>  
> -    /* We need some cpu model to instantiate the PnvChip class */
> -    if (machine->cpu_model == NULL) {
> -        machine->cpu_model = "POWER8";
> -    }
> -
>      /* Create the processor chips */
> -    chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%s", machine->cpu_model);
> +    i = strlen(machine->cpu_type) - strlen(POWERPC_CPU_TYPE_SUFFIX);
> +    chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%.*s",
> +                                    i, machine->cpu_type);
>      if (!object_class_by_name(chip_typename)) {
> -        error_report("invalid CPU model '%s' for %s machine",
> -                     machine->cpu_model, MACHINE_GET_CLASS(machine)->name);
> +        error_report("invalid CPU model '%.*s' for %s machine",
> +                     i, machine->cpu_type, MACHINE_GET_CLASS(machine)->name);
>          exit(1);
>      }
>  
> @@ -716,7 +713,7 @@ static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -    k->cpu_model = "POWER8E";
> +    k->cpu_model = "power8e_v2.1";
>      k->chip_type = PNV_CHIP_POWER8E;
>      k->chip_cfam_id = 0x221ef04980000000ull;  /* P8 Murano DD2.1 */
>      k->cores_mask = POWER8E_CORE_MASK;
> @@ -738,7 +735,7 @@ static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -    k->cpu_model = "POWER8";
> +    k->cpu_model = "power8_v2.0";
>      k->chip_type = PNV_CHIP_POWER8;
>      k->chip_cfam_id = 0x220ea04980000000ull; /* P8 Venice DD2.0 */
>      k->cores_mask = POWER8_CORE_MASK;
> @@ -760,7 +757,7 @@ static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -    k->cpu_model = "POWER8NVL";
> +    k->cpu_model = "power8nvl_v1.0";
>      k->chip_type = PNV_CHIP_POWER8NVL;
>      k->chip_cfam_id = 0x120d304980000000ull;  /* P8 Naples DD1.0 */
>      k->cores_mask = POWER8_CORE_MASK;
> @@ -782,7 +779,7 @@ static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PnvChipClass *k = PNV_CHIP_CLASS(klass);
>  
> -    k->cpu_model = "POWER9";
> +    k->cpu_model = "power9_v1.0";
>      k->chip_type = PNV_CHIP_POWER9;
>      k->chip_cfam_id = 0x100d104980000000ull; /* P9 Nimbus DD1.0 */
>      k->cores_mask = POWER9_CORE_MASK;
> @@ -1133,6 +1130,7 @@ static void powernv_machine_class_init(ObjectClass *oc, void *data)
>      mc->init = ppc_powernv_init;
>      mc->reset = ppc_powernv_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;
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index 6726483..44b0b24 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -227,7 +227,7 @@ static const TypeInfo pnv_core_info = {
>  };
>  
>  static const char *pnv_core_models[] = {
> -    "POWER8E", "POWER8", "POWER8NVL", "POWER9"
> +    "power8e_v2.1", "power8_v2.0", "power8nvl_v1.0", "power9_v1.0"
>  };
>  
>  static void pnv_core_register_types(void)
Igor Mammedov Oct. 6, 2017, 9:30 a.m. UTC | #3
On Fri, 6 Oct 2017 19:34:19 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Oct 05, 2017 at 06:24:45PM +0200, Igor Mammedov wrote:
> > use common cpu_model prasing in vl.c and set default cpu_model
> > using generic MachineClass::default_cpu_type.
> > 
> > Beside of switching to generic infrastructure it solves several
> > issues.
> > 
> >  * ppc_cpu_class_by_name() is used to deal with lower/upper case
> >    and alias translations into actual cpu type, which fixes
> >     '-M powernv -cpu power8' and '-M powernv -cpu power9_v1.0'
> >    usecases which error out with:
> >     'invalid CPU model 'FOO' for powernv machine'
> >  * allows to switch to lower-case typenames in pnv chip/core name
> >    (by convention typnames should be lower-case)
> >  * replace aliased names /power8, power9, .../ with exact cpu model
> >    names (i.e. typenames should be stable but aliases might decide to
> >    point to other cpu model withi family or changed by kvm). It will
> >    also help to simplify pnv_chip/core code and get rid of dependency
> >    on cpu_model parsing.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  include/hw/ppc/pnv.h |  8 ++++----
> >  hw/ppc/pnv.c         | 22 ++++++++++------------
> >  hw/ppc/pnv_core.c    |  2 +-
> >  3 files changed, 15 insertions(+), 17 deletions(-)
> > 
> > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> > index 9c5437d..2525f7f 100644
> > --- a/include/hw/ppc/pnv.h
> > +++ b/include/hw/ppc/pnv.h
> > @@ -80,19 +80,19 @@ typedef struct PnvChipClass {
> >      uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
> >  } PnvChipClass;
> >  
> > -#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-POWER8E"
> > +#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-power8e_v2.1"
> >  #define PNV_CHIP_POWER8E(obj) \
> >      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8E)
> >  
> > -#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-POWER8"
> > +#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-power8_v2.0"
> >  #define PNV_CHIP_POWER8(obj) \
> >      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8)
> >  
> > -#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-POWER8NVL"
> > +#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-power8nvl_v1.0"
> >  #define PNV_CHIP_POWER8NVL(obj) \
> >      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8NVL)
> >  
> > -#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-POWER9"
> > +#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-power9_v1.0"  
> 
> Uh.. we really should add a DD2 power9 before we make this change.
> Making a DD1.0 (read, buggy as hell) chip the default is not
> sensible.  Especially since we don't implement the various DD1 bugs
> and differences in qemu.
I guess pnv owner will have to it,
I can't help here /me uses whatever is in code right now/

btw: in my tests when booting machine with power9, qemu crashes
(it happens on master as well, so this series in not to blame)

> 
> >  #define PNV_CHIP_POWER9(obj) \
> >      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER9)
> >  
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index d46d91c..4169837 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -607,16 +607,13 @@ static void ppc_powernv_init(MachineState *machine)
> >          }
> >      }
> >  
> > -    /* We need some cpu model to instantiate the PnvChip class */
> > -    if (machine->cpu_model == NULL) {
> > -        machine->cpu_model = "POWER8";
> > -    }
> > -
> >      /* Create the processor chips */
> > -    chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%s", machine->cpu_model);
> > +    i = strlen(machine->cpu_type) - strlen(POWERPC_CPU_TYPE_SUFFIX);
> > +    chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%.*s",
> > +                                    i, machine->cpu_type);
> >      if (!object_class_by_name(chip_typename)) {
> > -        error_report("invalid CPU model '%s' for %s machine",
> > -                     machine->cpu_model, MACHINE_GET_CLASS(machine)->name);
> > +        error_report("invalid CPU model '%.*s' for %s machine",
> > +                     i, machine->cpu_type, MACHINE_GET_CLASS(machine)->name);
> >          exit(1);
> >      }
> >  
> > @@ -716,7 +713,7 @@ static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
> >      DeviceClass *dc = DEVICE_CLASS(klass);
> >      PnvChipClass *k = PNV_CHIP_CLASS(klass);
> >  
> > -    k->cpu_model = "POWER8E";
> > +    k->cpu_model = "power8e_v2.1";
> >      k->chip_type = PNV_CHIP_POWER8E;
> >      k->chip_cfam_id = 0x221ef04980000000ull;  /* P8 Murano DD2.1 */
> >      k->cores_mask = POWER8E_CORE_MASK;
> > @@ -738,7 +735,7 @@ static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
> >      DeviceClass *dc = DEVICE_CLASS(klass);
> >      PnvChipClass *k = PNV_CHIP_CLASS(klass);
> >  
> > -    k->cpu_model = "POWER8";
> > +    k->cpu_model = "power8_v2.0";
> >      k->chip_type = PNV_CHIP_POWER8;
> >      k->chip_cfam_id = 0x220ea04980000000ull; /* P8 Venice DD2.0 */
> >      k->cores_mask = POWER8_CORE_MASK;
> > @@ -760,7 +757,7 @@ static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
> >      DeviceClass *dc = DEVICE_CLASS(klass);
> >      PnvChipClass *k = PNV_CHIP_CLASS(klass);
> >  
> > -    k->cpu_model = "POWER8NVL";
> > +    k->cpu_model = "power8nvl_v1.0";
> >      k->chip_type = PNV_CHIP_POWER8NVL;
> >      k->chip_cfam_id = 0x120d304980000000ull;  /* P8 Naples DD1.0 */
> >      k->cores_mask = POWER8_CORE_MASK;
> > @@ -782,7 +779,7 @@ static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
> >      DeviceClass *dc = DEVICE_CLASS(klass);
> >      PnvChipClass *k = PNV_CHIP_CLASS(klass);
> >  
> > -    k->cpu_model = "POWER9";
> > +    k->cpu_model = "power9_v1.0";
> >      k->chip_type = PNV_CHIP_POWER9;
> >      k->chip_cfam_id = 0x100d104980000000ull; /* P9 Nimbus DD1.0 */
> >      k->cores_mask = POWER9_CORE_MASK;
> > @@ -1133,6 +1130,7 @@ static void powernv_machine_class_init(ObjectClass *oc, void *data)
> >      mc->init = ppc_powernv_init;
> >      mc->reset = ppc_powernv_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;
> > diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> > index 6726483..44b0b24 100644
> > --- a/hw/ppc/pnv_core.c
> > +++ b/hw/ppc/pnv_core.c
> > @@ -227,7 +227,7 @@ static const TypeInfo pnv_core_info = {
> >  };
> >  
> >  static const char *pnv_core_models[] = {
> > -    "POWER8E", "POWER8", "POWER8NVL", "POWER9"
> > +    "power8e_v2.1", "power8_v2.0", "power8nvl_v1.0", "power9_v1.0"
> >  };
> >  
> >  static void pnv_core_register_types(void)  
>
David Gibson Oct. 6, 2017, 11:25 a.m. UTC | #4
On Fri, Oct 06, 2017 at 11:30:54AM +0200, Igor Mammedov wrote:
> On Fri, 6 Oct 2017 19:34:19 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Thu, Oct 05, 2017 at 06:24:45PM +0200, Igor Mammedov wrote:
> > > use common cpu_model prasing in vl.c and set default cpu_model
> > > using generic MachineClass::default_cpu_type.
> > > 
> > > Beside of switching to generic infrastructure it solves several
> > > issues.
> > > 
> > >  * ppc_cpu_class_by_name() is used to deal with lower/upper case
> > >    and alias translations into actual cpu type, which fixes
> > >     '-M powernv -cpu power8' and '-M powernv -cpu power9_v1.0'
> > >    usecases which error out with:
> > >     'invalid CPU model 'FOO' for powernv machine'
> > >  * allows to switch to lower-case typenames in pnv chip/core name
> > >    (by convention typnames should be lower-case)
> > >  * replace aliased names /power8, power9, .../ with exact cpu model
> > >    names (i.e. typenames should be stable but aliases might decide to
> > >    point to other cpu model withi family or changed by kvm). It will
> > >    also help to simplify pnv_chip/core code and get rid of dependency
> > >    on cpu_model parsing.
> > > 
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > >  include/hw/ppc/pnv.h |  8 ++++----
> > >  hw/ppc/pnv.c         | 22 ++++++++++------------
> > >  hw/ppc/pnv_core.c    |  2 +-
> > >  3 files changed, 15 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> > > index 9c5437d..2525f7f 100644
> > > --- a/include/hw/ppc/pnv.h
> > > +++ b/include/hw/ppc/pnv.h
> > > @@ -80,19 +80,19 @@ typedef struct PnvChipClass {
> > >      uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
> > >  } PnvChipClass;
> > >  
> > > -#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-POWER8E"
> > > +#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-power8e_v2.1"
> > >  #define PNV_CHIP_POWER8E(obj) \
> > >      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8E)
> > >  
> > > -#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-POWER8"
> > > +#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-power8_v2.0"
> > >  #define PNV_CHIP_POWER8(obj) \
> > >      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8)
> > >  
> > > -#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-POWER8NVL"
> > > +#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-power8nvl_v1.0"
> > >  #define PNV_CHIP_POWER8NVL(obj) \
> > >      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8NVL)
> > >  
> > > -#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-POWER9"
> > > +#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-power9_v1.0"  
> > 
> > Uh.. we really should add a DD2 power9 before we make this change.
> > Making a DD1.0 (read, buggy as hell) chip the default is not
> > sensible.  Especially since we don't implement the various DD1 bugs
> > and differences in qemu.
> I guess pnv owner will have to it,
> I can't help here /me uses whatever is in code right now/

I just committed a patch to ppc-for-2.11 that adds POWER9 v2.0 to the
code (and makes it the default).  Sorry, this will probably require a
rebase of your stuff.
Igor Mammedov Oct. 9, 2017, 5:44 a.m. UTC | #5
On Fri, 6 Oct 2017 22:25:03 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Oct 06, 2017 at 11:30:54AM +0200, Igor Mammedov wrote:
> > On Fri, 6 Oct 2017 19:34:19 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> >   
> > > On Thu, Oct 05, 2017 at 06:24:45PM +0200, Igor Mammedov wrote:  
> > > > use common cpu_model prasing in vl.c and set default cpu_model
> > > > using generic MachineClass::default_cpu_type.
> > > > 
> > > > Beside of switching to generic infrastructure it solves several
> > > > issues.
> > > > 
> > > >  * ppc_cpu_class_by_name() is used to deal with lower/upper case
> > > >    and alias translations into actual cpu type, which fixes
> > > >     '-M powernv -cpu power8' and '-M powernv -cpu power9_v1.0'
> > > >    usecases which error out with:
> > > >     'invalid CPU model 'FOO' for powernv machine'
> > > >  * allows to switch to lower-case typenames in pnv chip/core name
> > > >    (by convention typnames should be lower-case)
> > > >  * replace aliased names /power8, power9, .../ with exact cpu model
> > > >    names (i.e. typenames should be stable but aliases might decide to
> > > >    point to other cpu model withi family or changed by kvm). It will
> > > >    also help to simplify pnv_chip/core code and get rid of dependency
> > > >    on cpu_model parsing.
> > > > 
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > ---
> > > >  include/hw/ppc/pnv.h |  8 ++++----
> > > >  hw/ppc/pnv.c         | 22 ++++++++++------------
> > > >  hw/ppc/pnv_core.c    |  2 +-
> > > >  3 files changed, 15 insertions(+), 17 deletions(-)
> > > > 
> > > > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> > > > index 9c5437d..2525f7f 100644
> > > > --- a/include/hw/ppc/pnv.h
> > > > +++ b/include/hw/ppc/pnv.h
> > > > @@ -80,19 +80,19 @@ typedef struct PnvChipClass {
> > > >      uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
> > > >  } PnvChipClass;
> > > >  
> > > > -#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-POWER8E"
> > > > +#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-power8e_v2.1"
> > > >  #define PNV_CHIP_POWER8E(obj) \
> > > >      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8E)
> > > >  
> > > > -#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-POWER8"
> > > > +#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-power8_v2.0"
> > > >  #define PNV_CHIP_POWER8(obj) \
> > > >      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8)
> > > >  
> > > > -#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-POWER8NVL"
> > > > +#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-power8nvl_v1.0"
> > > >  #define PNV_CHIP_POWER8NVL(obj) \
> > > >      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8NVL)
> > > >  
> > > > -#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-POWER9"
> > > > +#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-power9_v1.0"    
> > > 
> > > Uh.. we really should add a DD2 power9 before we make this change.
> > > Making a DD1.0 (read, buggy as hell) chip the default is not
> > > sensible.  Especially since we don't implement the various DD1 bugs
> > > and differences in qemu.  
> > I guess pnv owner will have to it,
> > I can't help here /me uses whatever is in code right now/  
> 
> I just committed a patch to ppc-for-2.11 that adds POWER9 v2.0 to the
> code (and makes it the default).  Sorry, this will probably require a
> rebase of your stuff.
Do you have a pointer to the patch or even better ppc staging tree to rebase on?
David Gibson Oct. 9, 2017, 6:59 a.m. UTC | #6
On Mon, Oct 09, 2017 at 07:44:15AM +0200, Igor Mammedov wrote:
> On Fri, 6 Oct 2017 22:25:03 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Fri, Oct 06, 2017 at 11:30:54AM +0200, Igor Mammedov wrote:
> > > On Fri, 6 Oct 2017 19:34:19 +1100
> > > David Gibson <david@gibson.dropbear.id.au> wrote:
> > >   
> > > > On Thu, Oct 05, 2017 at 06:24:45PM +0200, Igor Mammedov wrote:  
> > > > > use common cpu_model prasing in vl.c and set default cpu_model
> > > > > using generic MachineClass::default_cpu_type.
> > > > > 
> > > > > Beside of switching to generic infrastructure it solves several
> > > > > issues.
> > > > > 
> > > > >  * ppc_cpu_class_by_name() is used to deal with lower/upper case
> > > > >    and alias translations into actual cpu type, which fixes
> > > > >     '-M powernv -cpu power8' and '-M powernv -cpu power9_v1.0'
> > > > >    usecases which error out with:
> > > > >     'invalid CPU model 'FOO' for powernv machine'
> > > > >  * allows to switch to lower-case typenames in pnv chip/core name
> > > > >    (by convention typnames should be lower-case)
> > > > >  * replace aliased names /power8, power9, .../ with exact cpu model
> > > > >    names (i.e. typenames should be stable but aliases might decide to
> > > > >    point to other cpu model withi family or changed by kvm). It will
> > > > >    also help to simplify pnv_chip/core code and get rid of dependency
> > > > >    on cpu_model parsing.
> > > > > 
> > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > > ---
> > > > >  include/hw/ppc/pnv.h |  8 ++++----
> > > > >  hw/ppc/pnv.c         | 22 ++++++++++------------
> > > > >  hw/ppc/pnv_core.c    |  2 +-
> > > > >  3 files changed, 15 insertions(+), 17 deletions(-)
> > > > > 
> > > > > diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
> > > > > index 9c5437d..2525f7f 100644
> > > > > --- a/include/hw/ppc/pnv.h
> > > > > +++ b/include/hw/ppc/pnv.h
> > > > > @@ -80,19 +80,19 @@ typedef struct PnvChipClass {
> > > > >      uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
> > > > >  } PnvChipClass;
> > > > >  
> > > > > -#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-POWER8E"
> > > > > +#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-power8e_v2.1"
> > > > >  #define PNV_CHIP_POWER8E(obj) \
> > > > >      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8E)
> > > > >  
> > > > > -#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-POWER8"
> > > > > +#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-power8_v2.0"
> > > > >  #define PNV_CHIP_POWER8(obj) \
> > > > >      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8)
> > > > >  
> > > > > -#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-POWER8NVL"
> > > > > +#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-power8nvl_v1.0"
> > > > >  #define PNV_CHIP_POWER8NVL(obj) \
> > > > >      OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8NVL)
> > > > >  
> > > > > -#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-POWER9"
> > > > > +#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-power9_v1.0"    
> > > > 
> > > > Uh.. we really should add a DD2 power9 before we make this change.
> > > > Making a DD1.0 (read, buggy as hell) chip the default is not
> > > > sensible.  Especially since we don't implement the various DD1 bugs
> > > > and differences in qemu.  
> > > I guess pnv owner will have to it,
> > > I can't help here /me uses whatever is in code right now/  
> > 
> > I just committed a patch to ppc-for-2.11 that adds POWER9 v2.0 to the
> > code (and makes it the default).  Sorry, this will probably require a
> > rebase of your stuff.
> Do you have a pointer to the patch or even better ppc staging tree to rebase on?

Oh, sorry, when I say 'ppc-for-2.11' I'm referring to that staging
tree.  You can find it at:

git://github.com/dgibson/qemu.git branch 'ppc-for-2.11'.
diff mbox series

Patch

diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 9c5437d..2525f7f 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -80,19 +80,19 @@  typedef struct PnvChipClass {
     uint32_t (*core_pir)(PnvChip *chip, uint32_t core_id);
 } PnvChipClass;
 
-#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-POWER8E"
+#define TYPE_PNV_CHIP_POWER8E TYPE_PNV_CHIP "-power8e_v2.1"
 #define PNV_CHIP_POWER8E(obj) \
     OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8E)
 
-#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-POWER8"
+#define TYPE_PNV_CHIP_POWER8 TYPE_PNV_CHIP "-power8_v2.0"
 #define PNV_CHIP_POWER8(obj) \
     OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8)
 
-#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-POWER8NVL"
+#define TYPE_PNV_CHIP_POWER8NVL TYPE_PNV_CHIP "-power8nvl_v1.0"
 #define PNV_CHIP_POWER8NVL(obj) \
     OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER8NVL)
 
-#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-POWER9"
+#define TYPE_PNV_CHIP_POWER9 TYPE_PNV_CHIP "-power9_v1.0"
 #define PNV_CHIP_POWER9(obj) \
     OBJECT_CHECK(PnvChip, (obj), TYPE_PNV_CHIP_POWER9)
 
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index d46d91c..4169837 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -607,16 +607,13 @@  static void ppc_powernv_init(MachineState *machine)
         }
     }
 
-    /* We need some cpu model to instantiate the PnvChip class */
-    if (machine->cpu_model == NULL) {
-        machine->cpu_model = "POWER8";
-    }
-
     /* Create the processor chips */
-    chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%s", machine->cpu_model);
+    i = strlen(machine->cpu_type) - strlen(POWERPC_CPU_TYPE_SUFFIX);
+    chip_typename = g_strdup_printf(TYPE_PNV_CHIP "-%.*s",
+                                    i, machine->cpu_type);
     if (!object_class_by_name(chip_typename)) {
-        error_report("invalid CPU model '%s' for %s machine",
-                     machine->cpu_model, MACHINE_GET_CLASS(machine)->name);
+        error_report("invalid CPU model '%.*s' for %s machine",
+                     i, machine->cpu_type, MACHINE_GET_CLASS(machine)->name);
         exit(1);
     }
 
@@ -716,7 +713,7 @@  static void pnv_chip_power8e_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PnvChipClass *k = PNV_CHIP_CLASS(klass);
 
-    k->cpu_model = "POWER8E";
+    k->cpu_model = "power8e_v2.1";
     k->chip_type = PNV_CHIP_POWER8E;
     k->chip_cfam_id = 0x221ef04980000000ull;  /* P8 Murano DD2.1 */
     k->cores_mask = POWER8E_CORE_MASK;
@@ -738,7 +735,7 @@  static void pnv_chip_power8_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PnvChipClass *k = PNV_CHIP_CLASS(klass);
 
-    k->cpu_model = "POWER8";
+    k->cpu_model = "power8_v2.0";
     k->chip_type = PNV_CHIP_POWER8;
     k->chip_cfam_id = 0x220ea04980000000ull; /* P8 Venice DD2.0 */
     k->cores_mask = POWER8_CORE_MASK;
@@ -760,7 +757,7 @@  static void pnv_chip_power8nvl_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PnvChipClass *k = PNV_CHIP_CLASS(klass);
 
-    k->cpu_model = "POWER8NVL";
+    k->cpu_model = "power8nvl_v1.0";
     k->chip_type = PNV_CHIP_POWER8NVL;
     k->chip_cfam_id = 0x120d304980000000ull;  /* P8 Naples DD1.0 */
     k->cores_mask = POWER8_CORE_MASK;
@@ -782,7 +779,7 @@  static void pnv_chip_power9_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PnvChipClass *k = PNV_CHIP_CLASS(klass);
 
-    k->cpu_model = "POWER9";
+    k->cpu_model = "power9_v1.0";
     k->chip_type = PNV_CHIP_POWER9;
     k->chip_cfam_id = 0x100d104980000000ull; /* P9 Nimbus DD1.0 */
     k->cores_mask = POWER9_CORE_MASK;
@@ -1133,6 +1130,7 @@  static void powernv_machine_class_init(ObjectClass *oc, void *data)
     mc->init = ppc_powernv_init;
     mc->reset = ppc_powernv_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;
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 6726483..44b0b24 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -227,7 +227,7 @@  static const TypeInfo pnv_core_info = {
 };
 
 static const char *pnv_core_models[] = {
-    "POWER8E", "POWER8", "POWER8NVL", "POWER9"
+    "power8e_v2.1", "power8_v2.0", "power8nvl_v1.0", "power9_v1.0"
 };
 
 static void pnv_core_register_types(void)