[Qemu-devel] [PATCHv2 00/10] Clean up pseries machine versioning

[Qemu-devel] [PATCHv2 00/10] Clean up pseries machine versioning

[David Gibson]

Thomas Huth recently posted a patches to switch the default USB
controller for the pseries machine type to xHCI, which necessitated
creating a pseries-2.6 machine type version.

That raised a bunch of comments suggesting better ways we could manage
the machine versioning for pseries.

This is an attempt to implement those suggestions.  For the most part
it makes the handling of machine versions for pseries more similar to
the handling of PC machine versions.

Eduardo,
  - Any opinions on how 5/10 (the only patch which isn't spapr
    specific) should be merged?

Otherwise I'm hoping to get some ACKs from outside Red Hat, at which
point I'll merge these into my ppc-for-2.6 tree, to be pushed upstream
once the 2.6 tree opens.

Changes in v2:
 * General rework
 * Moved most actual fixes to existing behaviour earlier in the series
   for clarity
 * Now also avoid redundant calls to the base class instance_init function

David Gibson (10):
  pseries: Remove redundant setting of mc->name for pseries-2.5 machine
  pseries: Rearrange versioned machine type code
  pseries: Remove redundant calls to spapr_machine_initfn()
  pseries: Remove versions from mc->desc
  Move SET_MACHINE_COMPAT macro to boards.h
  pseries: Use SET_MACHINE_COMPAT
  pseries: DEFINE_SPAPR_MACHINE
  pseries: Restructure class_options functions
  pseries: Improve setting of default machine version
  pseries: Add pseries-2.6 machine type

 hw/ppc/spapr.c       | 238 +++++++++++++++++++++++++--------------------------
 include/hw/boards.h  |   9 ++
 include/hw/i386/pc.h |   8 --
 3 files changed, 127 insertions(+), 128 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCHv2 01/10] pseries: Remove redundant setting of mc->name for pseries-2.5 machine

[David Gibson]

98cec76 "machine: Set MachineClass::name automatically" removed the setting
of mc->name for the pseries machine types, since it can be derived
automatically from the type names constructed with MACHINE_TYPE_NAME().

Unfortunately fb0fc8f "spapr: Create pseries-2.5 machine" went in later and
brought one of them back.

This removes it again.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/spapr.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 29a16b7..a69856f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2440,7 +2440,6 @@ static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
     MachineClass *mc = MACHINE_CLASS(oc);
     sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
 
-    mc->name = "pseries-2.5";
     mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
     mc->alias = "pseries";
     mc->is_default = 1;
-- 
2.5.0

Re: [Qemu-devel] [PATCHv2 01/10] pseries: Remove redundant setting of mc->name for pseries-2.5 machine

[Alexey Kardashevskiy]

On 12/07/2015 02:34 PM, David Gibson wrote:
> 98cec76 "machine: Set MachineClass::name automatically" removed the setting
> of mc->name for the pseries machine types, since it can be derived
> automatically from the type names constructed with MACHINE_TYPE_NAME().
>
> Unfortunately fb0fc8f "spapr: Create pseries-2.5 machine" went in later and
> brought one of them back.
>
> This removes it again.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> ---
>   hw/ppc/spapr.c | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 29a16b7..a69856f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2440,7 +2440,6 @@ static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
>       MachineClass *mc = MACHINE_CLASS(oc);
>       sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
>
> -    mc->name = "pseries-2.5";
>       mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
>       mc->alias = "pseries";
>       mc->is_default = 1;
>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>	

-- 
Alexey

[Qemu-devel] [PATCHv2 02/10] pseries: Rearrange versioned machine type code

[David Gibson]

hw/ppc/spapr.c has a number of definitions related to the various versioned
machine types ("pseries-2.1" .. "pseries-2.5") it defines.  These are
mostly arranged by type of function first, then machine version second, and
it's not consistent about whether it goes in increasing or decreasing
version order.

This rearranges the code to keep all the definitions for a particular
machine version together, and arrange then consistently in order most
recent to least recent.

This brings us closer to matching the way PC does things, and makes later
cleanups easier to follow.

Apart from adding some comments marking each section, this is a pure
mechanical rearrangement with no semantic changes.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 180 +++++++++++++++++++++++++++++++--------------------------
 1 file changed, 98 insertions(+), 82 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a69856f..c126e10 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2301,9 +2301,53 @@ static const TypeInfo spapr_machine_info = {
     },
 };
 
+/*
+ * pseries-2.5
+ */
+static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
+
+    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
+    mc->alias = "pseries";
+    mc->is_default = 1;
+    smc->dr_lmb_enabled = true;
+}
+
+static const TypeInfo spapr_machine_2_5_info = {
+    .name          = MACHINE_TYPE_NAME("pseries-2.5"),
+    .parent        = TYPE_SPAPR_MACHINE,
+    .class_init    = spapr_machine_2_5_class_init,
+};
+
+/*
+ * pseries-2.4
+ */
 #define SPAPR_COMPAT_2_4 \
         HW_COMPAT_2_4
 
+static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
+{
+    static GlobalProperty compat_props[] = {
+        SPAPR_COMPAT_2_4
+        { /* end of list */ }
+    };
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
+    mc->compat_props = compat_props;
+}
+
+static const TypeInfo spapr_machine_2_4_info = {
+    .name          = MACHINE_TYPE_NAME("pseries-2.4"),
+    .parent        = TYPE_SPAPR_MACHINE,
+    .class_init    = spapr_machine_2_4_class_init,
+};
+
+/*
+ * pseries-2.3
+ */
 #define SPAPR_COMPAT_2_3 \
         SPAPR_COMPAT_2_4 \
         HW_COMPAT_2_3 \
@@ -2313,72 +2357,61 @@ static const TypeInfo spapr_machine_info = {
             .value    = "off",\
         },
 
-#define SPAPR_COMPAT_2_2 \
-        SPAPR_COMPAT_2_3 \
-        HW_COMPAT_2_2 \
-        {\
-            .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,\
-            .property = "mem_win_size",\
-            .value    = "0x20000000",\
-        },
-
-#define SPAPR_COMPAT_2_1 \
-        SPAPR_COMPAT_2_2 \
-        HW_COMPAT_2_1
-
 static void spapr_compat_2_3(Object *obj)
 {
     savevm_skip_section_footers();
     global_state_set_optional();
 }
 
-static void spapr_compat_2_2(Object *obj)
-{
-    spapr_compat_2_3(obj);
-}
-
-static void spapr_compat_2_1(Object *obj)
-{
-    spapr_compat_2_2(obj);
-}
-
 static void spapr_machine_2_3_instance_init(Object *obj)
 {
     spapr_compat_2_3(obj);
     spapr_machine_initfn(obj);
 }
 
-static void spapr_machine_2_2_instance_init(Object *obj)
-{
-    spapr_compat_2_2(obj);
-    spapr_machine_initfn(obj);
-}
-
-static void spapr_machine_2_1_instance_init(Object *obj)
-{
-    spapr_compat_2_1(obj);
-    spapr_machine_initfn(obj);
-}
-
-static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
+static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
 {
-    MachineClass *mc = MACHINE_CLASS(oc);
     static GlobalProperty compat_props[] = {
-        SPAPR_COMPAT_2_1
+        SPAPR_COMPAT_2_3
         { /* end of list */ }
     };
+    MachineClass *mc = MACHINE_CLASS(oc);
 
-    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
+    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
     mc->compat_props = compat_props;
 }
 
-static const TypeInfo spapr_machine_2_1_info = {
-    .name          = MACHINE_TYPE_NAME("pseries-2.1"),
+static const TypeInfo spapr_machine_2_3_info = {
+    .name          = MACHINE_TYPE_NAME("pseries-2.3"),
     .parent        = TYPE_SPAPR_MACHINE,
-    .class_init    = spapr_machine_2_1_class_init,
-    .instance_init = spapr_machine_2_1_instance_init,
+    .class_init    = spapr_machine_2_3_class_init,
+    .instance_init = spapr_machine_2_3_instance_init,
 };
 
+/*
+ * pseries-2.2
+ */
+
+#define SPAPR_COMPAT_2_2 \
+        SPAPR_COMPAT_2_3 \
+        HW_COMPAT_2_2 \
+        {\
+            .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,\
+            .property = "mem_win_size",\
+            .value    = "0x20000000",\
+        },
+
+static void spapr_compat_2_2(Object *obj)
+{
+    spapr_compat_2_3(obj);
+}
+
+static void spapr_machine_2_2_instance_init(Object *obj)
+{
+    spapr_compat_2_2(obj);
+    spapr_machine_initfn(obj);
+}
+
 static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
 {
     static GlobalProperty compat_props[] = {
@@ -2398,58 +2431,41 @@ static const TypeInfo spapr_machine_2_2_info = {
     .instance_init = spapr_machine_2_2_instance_init,
 };
 
-static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
-{
-    static GlobalProperty compat_props[] = {
-        SPAPR_COMPAT_2_3
-        { /* end of list */ }
-    };
-    MachineClass *mc = MACHINE_CLASS(oc);
+/*
+ * pseries-2.1
+ */
+#define SPAPR_COMPAT_2_1 \
+        SPAPR_COMPAT_2_2 \
+        HW_COMPAT_2_1
 
-    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
-    mc->compat_props = compat_props;
+static void spapr_compat_2_1(Object *obj)
+{
+    spapr_compat_2_2(obj);
 }
 
-static const TypeInfo spapr_machine_2_3_info = {
-    .name          = MACHINE_TYPE_NAME("pseries-2.3"),
-    .parent        = TYPE_SPAPR_MACHINE,
-    .class_init    = spapr_machine_2_3_class_init,
-    .instance_init = spapr_machine_2_3_instance_init,
-};
+static void spapr_machine_2_1_instance_init(Object *obj)
+{
+    spapr_compat_2_1(obj);
+    spapr_machine_initfn(obj);
+}
 
-static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
+static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
 {
+    MachineClass *mc = MACHINE_CLASS(oc);
     static GlobalProperty compat_props[] = {
-        SPAPR_COMPAT_2_4
+        SPAPR_COMPAT_2_1
         { /* end of list */ }
     };
-    MachineClass *mc = MACHINE_CLASS(oc);
 
-    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
+    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
     mc->compat_props = compat_props;
 }
 
-static const TypeInfo spapr_machine_2_4_info = {
-    .name          = MACHINE_TYPE_NAME("pseries-2.4"),
-    .parent        = TYPE_SPAPR_MACHINE,
-    .class_init    = spapr_machine_2_4_class_init,
-};
-
-static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
-{
-    MachineClass *mc = MACHINE_CLASS(oc);
-    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
-
-    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
-    mc->alias = "pseries";
-    mc->is_default = 1;
-    smc->dr_lmb_enabled = true;
-}
-
-static const TypeInfo spapr_machine_2_5_info = {
-    .name          = MACHINE_TYPE_NAME("pseries-2.5"),
+static const TypeInfo spapr_machine_2_1_info = {
+    .name          = MACHINE_TYPE_NAME("pseries-2.1"),
     .parent        = TYPE_SPAPR_MACHINE,
-    .class_init    = spapr_machine_2_5_class_init,
+    .class_init    = spapr_machine_2_1_class_init,
+    .instance_init = spapr_machine_2_1_instance_init,
 };
 
 static void spapr_machine_register_types(void)
-- 
2.5.0

Re: [Qemu-devel] [PATCHv2 02/10] pseries: Rearrange versioned machine type code

[Thomas Huth]

On 07/12/15 04:34, David Gibson wrote:
> hw/ppc/spapr.c has a number of definitions related to the various versioned
> machine types ("pseries-2.1" .. "pseries-2.5") it defines.  These are
> mostly arranged by type of function first, then machine version second, and
> it's not consistent about whether it goes in increasing or decreasing
> version order.
> 
> This rearranges the code to keep all the definitions for a particular
> machine version together, and arrange then consistently in order most
> recent to least recent.
> 
> This brings us closer to matching the way PC does things, and makes later
> cleanups easier to follow.
> 
> Apart from adding some comments marking each section, this is a pure
> mechanical rearrangement with no semantic changes.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr.c | 180 +++++++++++++++++++++++++++++++--------------------------
>  1 file changed, 98 insertions(+), 82 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a69856f..c126e10 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2301,9 +2301,53 @@ static const TypeInfo spapr_machine_info = {
>      },
>  };
>  
> +/*
> + * pseries-2.5
> + */
> +static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
> +
> +    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
> +    mc->alias = "pseries";
> +    mc->is_default = 1;
> +    smc->dr_lmb_enabled = true;
> +}
> +
> +static const TypeInfo spapr_machine_2_5_info = {
> +    .name          = MACHINE_TYPE_NAME("pseries-2.5"),
> +    .parent        = TYPE_SPAPR_MACHINE,
> +    .class_init    = spapr_machine_2_5_class_init,
> +};
> +
> +/*
> + * pseries-2.4
> + */
>  #define SPAPR_COMPAT_2_4 \
>          HW_COMPAT_2_4
>  
> +static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
> +{
> +    static GlobalProperty compat_props[] = {
> +        SPAPR_COMPAT_2_4
> +        { /* end of list */ }
> +    };
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
> +    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
> +    mc->compat_props = compat_props;
> +}
> +
> +static const TypeInfo spapr_machine_2_4_info = {
> +    .name          = MACHINE_TYPE_NAME("pseries-2.4"),
> +    .parent        = TYPE_SPAPR_MACHINE,
> +    .class_init    = spapr_machine_2_4_class_init,
> +};
> +
> +/*
> + * pseries-2.3
> + */
>  #define SPAPR_COMPAT_2_3 \
>          SPAPR_COMPAT_2_4 \
>          HW_COMPAT_2_3 \
> @@ -2313,72 +2357,61 @@ static const TypeInfo spapr_machine_info = {
>              .value    = "off",\
>          },
>  
> -#define SPAPR_COMPAT_2_2 \
> -        SPAPR_COMPAT_2_3 \
> -        HW_COMPAT_2_2 \
> -        {\
> -            .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,\
> -            .property = "mem_win_size",\
> -            .value    = "0x20000000",\
> -        },
> -
> -#define SPAPR_COMPAT_2_1 \
> -        SPAPR_COMPAT_2_2 \
> -        HW_COMPAT_2_1
> -
>  static void spapr_compat_2_3(Object *obj)
>  {
>      savevm_skip_section_footers();
>      global_state_set_optional();
>  }
>  
> -static void spapr_compat_2_2(Object *obj)
> -{
> -    spapr_compat_2_3(obj);
> -}
> -
> -static void spapr_compat_2_1(Object *obj)
> -{
> -    spapr_compat_2_2(obj);
> -}
> -
>  static void spapr_machine_2_3_instance_init(Object *obj)
>  {
>      spapr_compat_2_3(obj);
>      spapr_machine_initfn(obj);
>  }
>  
> -static void spapr_machine_2_2_instance_init(Object *obj)
> -{
> -    spapr_compat_2_2(obj);
> -    spapr_machine_initfn(obj);
> -}
> -
> -static void spapr_machine_2_1_instance_init(Object *obj)
> -{
> -    spapr_compat_2_1(obj);
> -    spapr_machine_initfn(obj);
> -}
> -
> -static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
> +static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
>  {
> -    MachineClass *mc = MACHINE_CLASS(oc);
>      static GlobalProperty compat_props[] = {
> -        SPAPR_COMPAT_2_1
> +        SPAPR_COMPAT_2_3
>          { /* end of list */ }
>      };
> +    MachineClass *mc = MACHINE_CLASS(oc);
>  
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
> +    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
>      mc->compat_props = compat_props;
>  }
>  
> -static const TypeInfo spapr_machine_2_1_info = {
> -    .name          = MACHINE_TYPE_NAME("pseries-2.1"),
> +static const TypeInfo spapr_machine_2_3_info = {
> +    .name          = MACHINE_TYPE_NAME("pseries-2.3"),
>      .parent        = TYPE_SPAPR_MACHINE,
> -    .class_init    = spapr_machine_2_1_class_init,
> -    .instance_init = spapr_machine_2_1_instance_init,
> +    .class_init    = spapr_machine_2_3_class_init,
> +    .instance_init = spapr_machine_2_3_instance_init,
>  };
>  
> +/*
> + * pseries-2.2
> + */
> +
> +#define SPAPR_COMPAT_2_2 \
> +        SPAPR_COMPAT_2_3 \
> +        HW_COMPAT_2_2 \
> +        {\
> +            .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,\
> +            .property = "mem_win_size",\
> +            .value    = "0x20000000",\
> +        },
> +
> +static void spapr_compat_2_2(Object *obj)
> +{
> +    spapr_compat_2_3(obj);
> +}
> +
> +static void spapr_machine_2_2_instance_init(Object *obj)
> +{
> +    spapr_compat_2_2(obj);
> +    spapr_machine_initfn(obj);
> +}
> +
>  static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
>  {
>      static GlobalProperty compat_props[] = {
> @@ -2398,58 +2431,41 @@ static const TypeInfo spapr_machine_2_2_info = {
>      .instance_init = spapr_machine_2_2_instance_init,
>  };
>  
> -static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
> -{
> -    static GlobalProperty compat_props[] = {
> -        SPAPR_COMPAT_2_3
> -        { /* end of list */ }
> -    };
> -    MachineClass *mc = MACHINE_CLASS(oc);
> +/*
> + * pseries-2.1
> + */
> +#define SPAPR_COMPAT_2_1 \
> +        SPAPR_COMPAT_2_2 \
> +        HW_COMPAT_2_1
>  
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
> -    mc->compat_props = compat_props;
> +static void spapr_compat_2_1(Object *obj)
> +{
> +    spapr_compat_2_2(obj);
>  }
>  
> -static const TypeInfo spapr_machine_2_3_info = {
> -    .name          = MACHINE_TYPE_NAME("pseries-2.3"),
> -    .parent        = TYPE_SPAPR_MACHINE,
> -    .class_init    = spapr_machine_2_3_class_init,
> -    .instance_init = spapr_machine_2_3_instance_init,
> -};
> +static void spapr_machine_2_1_instance_init(Object *obj)
> +{
> +    spapr_compat_2_1(obj);
> +    spapr_machine_initfn(obj);
> +}
>  
> -static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
> +static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
>  {
> +    MachineClass *mc = MACHINE_CLASS(oc);
>      static GlobalProperty compat_props[] = {
> -        SPAPR_COMPAT_2_4
> +        SPAPR_COMPAT_2_1
>          { /* end of list */ }
>      };
> -    MachineClass *mc = MACHINE_CLASS(oc);
>  
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
> +    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
>      mc->compat_props = compat_props;
>  }
>  
> -static const TypeInfo spapr_machine_2_4_info = {
> -    .name          = MACHINE_TYPE_NAME("pseries-2.4"),
> -    .parent        = TYPE_SPAPR_MACHINE,
> -    .class_init    = spapr_machine_2_4_class_init,
> -};
> -
> -static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
> -{
> -    MachineClass *mc = MACHINE_CLASS(oc);
> -    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
> -
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
> -    mc->alias = "pseries";
> -    mc->is_default = 1;
> -    smc->dr_lmb_enabled = true;
> -}
> -
> -static const TypeInfo spapr_machine_2_5_info = {
> -    .name          = MACHINE_TYPE_NAME("pseries-2.5"),
> +static const TypeInfo spapr_machine_2_1_info = {
> +    .name          = MACHINE_TYPE_NAME("pseries-2.1"),
>      .parent        = TYPE_SPAPR_MACHINE,
> -    .class_init    = spapr_machine_2_5_class_init,
> +    .class_init    = spapr_machine_2_1_class_init,
> +    .instance_init = spapr_machine_2_1_instance_init,
>  };
>  
>  static void spapr_machine_register_types(void)

The patch might have been a little bit easier to read if you'd moved the
next two cleanup patches before this one in the series, but as far as I
can see, it's all clean code movement, without further modification, and
IMHO it also makes sense, so:

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCHv2 02/10] pseries: Rearrange versioned machine type code

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 9795 bytes --]

On Mon, Dec 07, 2015 at 11:21:26AM +0100, Thomas Huth wrote:
> On 07/12/15 04:34, David Gibson wrote:
> > hw/ppc/spapr.c has a number of definitions related to the various versioned
> > machine types ("pseries-2.1" .. "pseries-2.5") it defines.  These are
> > mostly arranged by type of function first, then machine version second, and
> > it's not consistent about whether it goes in increasing or decreasing
> > version order.
> > 
> > This rearranges the code to keep all the definitions for a particular
> > machine version together, and arrange then consistently in order most
> > recent to least recent.
> > 
> > This brings us closer to matching the way PC does things, and makes later
> > cleanups easier to follow.
> > 
> > Apart from adding some comments marking each section, this is a pure
> > mechanical rearrangement with no semantic changes.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/ppc/spapr.c | 180 +++++++++++++++++++++++++++++++--------------------------
> >  1 file changed, 98 insertions(+), 82 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index a69856f..c126e10 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2301,9 +2301,53 @@ static const TypeInfo spapr_machine_info = {
> >      },
> >  };
> >  
> > +/*
> > + * pseries-2.5
> > + */
> > +static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
> > +{
> > +    MachineClass *mc = MACHINE_CLASS(oc);
> > +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
> > +
> > +    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
> > +    mc->alias = "pseries";
> > +    mc->is_default = 1;
> > +    smc->dr_lmb_enabled = true;
> > +}
> > +
> > +static const TypeInfo spapr_machine_2_5_info = {
> > +    .name          = MACHINE_TYPE_NAME("pseries-2.5"),
> > +    .parent        = TYPE_SPAPR_MACHINE,
> > +    .class_init    = spapr_machine_2_5_class_init,
> > +};
> > +
> > +/*
> > + * pseries-2.4
> > + */
> >  #define SPAPR_COMPAT_2_4 \
> >          HW_COMPAT_2_4
> >  
> > +static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
> > +{
> > +    static GlobalProperty compat_props[] = {
> > +        SPAPR_COMPAT_2_4
> > +        { /* end of list */ }
> > +    };
> > +    MachineClass *mc = MACHINE_CLASS(oc);
> > +
> > +    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
> > +    mc->compat_props = compat_props;
> > +}
> > +
> > +static const TypeInfo spapr_machine_2_4_info = {
> > +    .name          = MACHINE_TYPE_NAME("pseries-2.4"),
> > +    .parent        = TYPE_SPAPR_MACHINE,
> > +    .class_init    = spapr_machine_2_4_class_init,
> > +};
> > +
> > +/*
> > + * pseries-2.3
> > + */
> >  #define SPAPR_COMPAT_2_3 \
> >          SPAPR_COMPAT_2_4 \
> >          HW_COMPAT_2_3 \
> > @@ -2313,72 +2357,61 @@ static const TypeInfo spapr_machine_info = {
> >              .value    = "off",\
> >          },
> >  
> > -#define SPAPR_COMPAT_2_2 \
> > -        SPAPR_COMPAT_2_3 \
> > -        HW_COMPAT_2_2 \
> > -        {\
> > -            .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,\
> > -            .property = "mem_win_size",\
> > -            .value    = "0x20000000",\
> > -        },
> > -
> > -#define SPAPR_COMPAT_2_1 \
> > -        SPAPR_COMPAT_2_2 \
> > -        HW_COMPAT_2_1
> > -
> >  static void spapr_compat_2_3(Object *obj)
> >  {
> >      savevm_skip_section_footers();
> >      global_state_set_optional();
> >  }
> >  
> > -static void spapr_compat_2_2(Object *obj)
> > -{
> > -    spapr_compat_2_3(obj);
> > -}
> > -
> > -static void spapr_compat_2_1(Object *obj)
> > -{
> > -    spapr_compat_2_2(obj);
> > -}
> > -
> >  static void spapr_machine_2_3_instance_init(Object *obj)
> >  {
> >      spapr_compat_2_3(obj);
> >      spapr_machine_initfn(obj);
> >  }
> >  
> > -static void spapr_machine_2_2_instance_init(Object *obj)
> > -{
> > -    spapr_compat_2_2(obj);
> > -    spapr_machine_initfn(obj);
> > -}
> > -
> > -static void spapr_machine_2_1_instance_init(Object *obj)
> > -{
> > -    spapr_compat_2_1(obj);
> > -    spapr_machine_initfn(obj);
> > -}
> > -
> > -static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
> > +static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
> >  {
> > -    MachineClass *mc = MACHINE_CLASS(oc);
> >      static GlobalProperty compat_props[] = {
> > -        SPAPR_COMPAT_2_1
> > +        SPAPR_COMPAT_2_3
> >          { /* end of list */ }
> >      };
> > +    MachineClass *mc = MACHINE_CLASS(oc);
> >  
> > -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
> > +    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
> >      mc->compat_props = compat_props;
> >  }
> >  
> > -static const TypeInfo spapr_machine_2_1_info = {
> > -    .name          = MACHINE_TYPE_NAME("pseries-2.1"),
> > +static const TypeInfo spapr_machine_2_3_info = {
> > +    .name          = MACHINE_TYPE_NAME("pseries-2.3"),
> >      .parent        = TYPE_SPAPR_MACHINE,
> > -    .class_init    = spapr_machine_2_1_class_init,
> > -    .instance_init = spapr_machine_2_1_instance_init,
> > +    .class_init    = spapr_machine_2_3_class_init,
> > +    .instance_init = spapr_machine_2_3_instance_init,
> >  };
> >  
> > +/*
> > + * pseries-2.2
> > + */
> > +
> > +#define SPAPR_COMPAT_2_2 \
> > +        SPAPR_COMPAT_2_3 \
> > +        HW_COMPAT_2_2 \
> > +        {\
> > +            .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,\
> > +            .property = "mem_win_size",\
> > +            .value    = "0x20000000",\
> > +        },
> > +
> > +static void spapr_compat_2_2(Object *obj)
> > +{
> > +    spapr_compat_2_3(obj);
> > +}
> > +
> > +static void spapr_machine_2_2_instance_init(Object *obj)
> > +{
> > +    spapr_compat_2_2(obj);
> > +    spapr_machine_initfn(obj);
> > +}
> > +
> >  static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
> >  {
> >      static GlobalProperty compat_props[] = {
> > @@ -2398,58 +2431,41 @@ static const TypeInfo spapr_machine_2_2_info = {
> >      .instance_init = spapr_machine_2_2_instance_init,
> >  };
> >  
> > -static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
> > -{
> > -    static GlobalProperty compat_props[] = {
> > -        SPAPR_COMPAT_2_3
> > -        { /* end of list */ }
> > -    };
> > -    MachineClass *mc = MACHINE_CLASS(oc);
> > +/*
> > + * pseries-2.1
> > + */
> > +#define SPAPR_COMPAT_2_1 \
> > +        SPAPR_COMPAT_2_2 \
> > +        HW_COMPAT_2_1
> >  
> > -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
> > -    mc->compat_props = compat_props;
> > +static void spapr_compat_2_1(Object *obj)
> > +{
> > +    spapr_compat_2_2(obj);
> >  }
> >  
> > -static const TypeInfo spapr_machine_2_3_info = {
> > -    .name          = MACHINE_TYPE_NAME("pseries-2.3"),
> > -    .parent        = TYPE_SPAPR_MACHINE,
> > -    .class_init    = spapr_machine_2_3_class_init,
> > -    .instance_init = spapr_machine_2_3_instance_init,
> > -};
> > +static void spapr_machine_2_1_instance_init(Object *obj)
> > +{
> > +    spapr_compat_2_1(obj);
> > +    spapr_machine_initfn(obj);
> > +}
> >  
> > -static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
> > +static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
> >  {
> > +    MachineClass *mc = MACHINE_CLASS(oc);
> >      static GlobalProperty compat_props[] = {
> > -        SPAPR_COMPAT_2_4
> > +        SPAPR_COMPAT_2_1
> >          { /* end of list */ }
> >      };
> > -    MachineClass *mc = MACHINE_CLASS(oc);
> >  
> > -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
> > +    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
> >      mc->compat_props = compat_props;
> >  }
> >  
> > -static const TypeInfo spapr_machine_2_4_info = {
> > -    .name          = MACHINE_TYPE_NAME("pseries-2.4"),
> > -    .parent        = TYPE_SPAPR_MACHINE,
> > -    .class_init    = spapr_machine_2_4_class_init,
> > -};
> > -
> > -static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
> > -{
> > -    MachineClass *mc = MACHINE_CLASS(oc);
> > -    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
> > -
> > -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
> > -    mc->alias = "pseries";
> > -    mc->is_default = 1;
> > -    smc->dr_lmb_enabled = true;
> > -}
> > -
> > -static const TypeInfo spapr_machine_2_5_info = {
> > -    .name          = MACHINE_TYPE_NAME("pseries-2.5"),
> > +static const TypeInfo spapr_machine_2_1_info = {
> > +    .name          = MACHINE_TYPE_NAME("pseries-2.1"),
> >      .parent        = TYPE_SPAPR_MACHINE,
> > -    .class_init    = spapr_machine_2_5_class_init,
> > +    .class_init    = spapr_machine_2_1_class_init,
> > +    .instance_init = spapr_machine_2_1_instance_init,
> >  };
> >  
> >  static void spapr_machine_register_types(void)
> 
> The patch might have been a little bit easier to read if you'd moved the
> next two cleanup patches before this one in the series, but as far as I
> can see, it's all clean code movement, without further modification, and
> IMHO it also makes sense, so:

Yeah, maybe. But because this is a big code move, doing so would have
meant basically rewriting both patches from scratch, which was more
hassle than I wanted.

> Reviewed-by: Thomas Huth <thuth@redhat.com>
> 
> 

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCHv2 02/10] pseries: Rearrange versioned machine type code

[Alexey Kardashevskiy]

On 12/07/2015 02:34 PM, David Gibson wrote:
> hw/ppc/spapr.c has a number of definitions related to the various versioned
> machine types ("pseries-2.1" .. "pseries-2.5") it defines.  These are
> mostly arranged by type of function first, then machine version second, and
> it's not consistent about whether it goes in increasing or decreasing
> version order.
>
> This rearranges the code to keep all the definitions for a particular
> machine version together, and arrange then consistently in order most
> recent to least recent.
>
> This brings us closer to matching the way PC does things, and makes later
> cleanups easier to follow.
>
> Apart from adding some comments marking each section, this is a pure
> mechanical rearrangement with no semantic changes.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   hw/ppc/spapr.c | 180 +++++++++++++++++++++++++++++++--------------------------
>   1 file changed, 98 insertions(+), 82 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a69856f..c126e10 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2301,9 +2301,53 @@ static const TypeInfo spapr_machine_info = {
>       },
>   };
>
> +/*
> + * pseries-2.5
> + */
> +static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
> +
> +    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
> +    mc->alias = "pseries";
> +    mc->is_default = 1;
> +    smc->dr_lmb_enabled = true;
> +}
> +
> +static const TypeInfo spapr_machine_2_5_info = {
> +    .name          = MACHINE_TYPE_NAME("pseries-2.5"),
> +    .parent        = TYPE_SPAPR_MACHINE,
> +    .class_init    = spapr_machine_2_5_class_init,
> +};
> +
> +/*
> + * pseries-2.4
> + */
>   #define SPAPR_COMPAT_2_4 \
>           HW_COMPAT_2_4
>
> +static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
> +{
> +    static GlobalProperty compat_props[] = {
> +        SPAPR_COMPAT_2_4
> +        { /* end of list */ }
> +    };
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
> +    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
> +    mc->compat_props = compat_props;
> +}
> +
> +static const TypeInfo spapr_machine_2_4_info = {
> +    .name          = MACHINE_TYPE_NAME("pseries-2.4"),
> +    .parent        = TYPE_SPAPR_MACHINE,
> +    .class_init    = spapr_machine_2_4_class_init,
> +};
> +
> +/*
> + * pseries-2.3
> + */
>   #define SPAPR_COMPAT_2_3 \
>           SPAPR_COMPAT_2_4 \
>           HW_COMPAT_2_3 \
> @@ -2313,72 +2357,61 @@ static const TypeInfo spapr_machine_info = {
>               .value    = "off",\
>           },
>
> -#define SPAPR_COMPAT_2_2 \
> -        SPAPR_COMPAT_2_3 \
> -        HW_COMPAT_2_2 \
> -        {\
> -            .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,\
> -            .property = "mem_win_size",\
> -            .value    = "0x20000000",\
> -        },
> -
> -#define SPAPR_COMPAT_2_1 \
> -        SPAPR_COMPAT_2_2 \
> -        HW_COMPAT_2_1
> -
>   static void spapr_compat_2_3(Object *obj)
>   {
>       savevm_skip_section_footers();
>       global_state_set_optional();
>   }
>
> -static void spapr_compat_2_2(Object *obj)
> -{
> -    spapr_compat_2_3(obj);
> -}
> -
> -static void spapr_compat_2_1(Object *obj)
> -{
> -    spapr_compat_2_2(obj);
> -}
> -
>   static void spapr_machine_2_3_instance_init(Object *obj)
>   {
>       spapr_compat_2_3(obj);
>       spapr_machine_initfn(obj);
>   }
>
> -static void spapr_machine_2_2_instance_init(Object *obj)
> -{
> -    spapr_compat_2_2(obj);
> -    spapr_machine_initfn(obj);
> -}
> -
> -static void spapr_machine_2_1_instance_init(Object *obj)
> -{
> -    spapr_compat_2_1(obj);
> -    spapr_machine_initfn(obj);
> -}
> -
> -static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
> +static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
>   {
> -    MachineClass *mc = MACHINE_CLASS(oc);
>       static GlobalProperty compat_props[] = {
> -        SPAPR_COMPAT_2_1
> +        SPAPR_COMPAT_2_3
>           { /* end of list */ }
>       };
> +    MachineClass *mc = MACHINE_CLASS(oc);
>
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
> +    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
>       mc->compat_props = compat_props;
>   }
>
> -static const TypeInfo spapr_machine_2_1_info = {
> -    .name          = MACHINE_TYPE_NAME("pseries-2.1"),
> +static const TypeInfo spapr_machine_2_3_info = {
> +    .name          = MACHINE_TYPE_NAME("pseries-2.3"),
>       .parent        = TYPE_SPAPR_MACHINE,
> -    .class_init    = spapr_machine_2_1_class_init,
> -    .instance_init = spapr_machine_2_1_instance_init,
> +    .class_init    = spapr_machine_2_3_class_init,
> +    .instance_init = spapr_machine_2_3_instance_init,
>   };
>
> +/*
> + * pseries-2.2
> + */
> +
> +#define SPAPR_COMPAT_2_2 \
> +        SPAPR_COMPAT_2_3 \
> +        HW_COMPAT_2_2 \
> +        {\
> +            .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,\
> +            .property = "mem_win_size",\
> +            .value    = "0x20000000",\
> +        },
> +
> +static void spapr_compat_2_2(Object *obj)
> +{
> +    spapr_compat_2_3(obj);
> +}
> +
> +static void spapr_machine_2_2_instance_init(Object *obj)
> +{
> +    spapr_compat_2_2(obj);
> +    spapr_machine_initfn(obj);
> +}
> +
>   static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
>   {
>       static GlobalProperty compat_props[] = {
> @@ -2398,58 +2431,41 @@ static const TypeInfo spapr_machine_2_2_info = {
>       .instance_init = spapr_machine_2_2_instance_init,
>   };
>
> -static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
> -{
> -    static GlobalProperty compat_props[] = {
> -        SPAPR_COMPAT_2_3
> -        { /* end of list */ }
> -    };
> -    MachineClass *mc = MACHINE_CLASS(oc);
> +/*
> + * pseries-2.1
> + */
> +#define SPAPR_COMPAT_2_1 \
> +        SPAPR_COMPAT_2_2 \
> +        HW_COMPAT_2_1
>
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
> -    mc->compat_props = compat_props;
> +static void spapr_compat_2_1(Object *obj)
> +{
> +    spapr_compat_2_2(obj);
>   }
>
> -static const TypeInfo spapr_machine_2_3_info = {
> -    .name          = MACHINE_TYPE_NAME("pseries-2.3"),
> -    .parent        = TYPE_SPAPR_MACHINE,
> -    .class_init    = spapr_machine_2_3_class_init,
> -    .instance_init = spapr_machine_2_3_instance_init,
> -};
> +static void spapr_machine_2_1_instance_init(Object *obj)
> +{
> +    spapr_compat_2_1(obj);
> +    spapr_machine_initfn(obj);
> +}
>
> -static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
> +static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
>   {
> +    MachineClass *mc = MACHINE_CLASS(oc);
>       static GlobalProperty compat_props[] = {
> -        SPAPR_COMPAT_2_4
> +        SPAPR_COMPAT_2_1
>           { /* end of list */ }
>       };
> -    MachineClass *mc = MACHINE_CLASS(oc);
>
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
> +    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
>       mc->compat_props = compat_props;
>   }
>
> -static const TypeInfo spapr_machine_2_4_info = {
> -    .name          = MACHINE_TYPE_NAME("pseries-2.4"),
> -    .parent        = TYPE_SPAPR_MACHINE,
> -    .class_init    = spapr_machine_2_4_class_init,
> -};
> -
> -static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
> -{
> -    MachineClass *mc = MACHINE_CLASS(oc);
> -    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
> -
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
> -    mc->alias = "pseries";
> -    mc->is_default = 1;
> -    smc->dr_lmb_enabled = true;
> -}
> -
> -static const TypeInfo spapr_machine_2_5_info = {
> -    .name          = MACHINE_TYPE_NAME("pseries-2.5"),
> +static const TypeInfo spapr_machine_2_1_info = {
> +    .name          = MACHINE_TYPE_NAME("pseries-2.1"),
>       .parent        = TYPE_SPAPR_MACHINE,
> -    .class_init    = spapr_machine_2_5_class_init,
> +    .class_init    = spapr_machine_2_1_class_init,
> +    .instance_init = spapr_machine_2_1_instance_init,
>   };
>
>   static void spapr_machine_register_types(void)
>

Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>	

-- 
Alexey

[Qemu-devel] [PATCHv2 03/10] pseries: Remove redundant calls to spapr_machine_initfn()

[David Gibson]

The instance_init() functions for several of the pseries-x.y versioned
machine types explicitly call spapr_machine_initfn().  But that's the
instance_init function for the common parent of all those machine types,
so will already have been called beforehand by the QOM infrastructure.

Remove the redundant calls.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c126e10..c7c6517 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2366,7 +2366,6 @@ static void spapr_compat_2_3(Object *obj)
 static void spapr_machine_2_3_instance_init(Object *obj)
 {
     spapr_compat_2_3(obj);
-    spapr_machine_initfn(obj);
 }
 
 static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
@@ -2409,7 +2408,6 @@ static void spapr_compat_2_2(Object *obj)
 static void spapr_machine_2_2_instance_init(Object *obj)
 {
     spapr_compat_2_2(obj);
-    spapr_machine_initfn(obj);
 }
 
 static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
@@ -2446,7 +2444,6 @@ static void spapr_compat_2_1(Object *obj)
 static void spapr_machine_2_1_instance_init(Object *obj)
 {
     spapr_compat_2_1(obj);
-    spapr_machine_initfn(obj);
 }
 
 static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
-- 
2.5.0

Re: [Qemu-devel] [PATCHv2 03/10] pseries: Remove redundant calls to spapr_machine_initfn()

[Thomas Huth]

On 07/12/15 04:34, David Gibson wrote:
> The instance_init() functions for several of the pseries-x.y versioned
> machine types explicitly call spapr_machine_initfn().  But that's the
> instance_init function for the common parent of all those machine types,
> so will already have been called beforehand by the QOM infrastructure.
> 
> Remove the redundant calls.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c126e10..c7c6517 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2366,7 +2366,6 @@ static void spapr_compat_2_3(Object *obj)
>  static void spapr_machine_2_3_instance_init(Object *obj)
>  {
>      spapr_compat_2_3(obj);
> -    spapr_machine_initfn(obj);
>  }
>  
>  static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
> @@ -2409,7 +2408,6 @@ static void spapr_compat_2_2(Object *obj)
>  static void spapr_machine_2_2_instance_init(Object *obj)
>  {
>      spapr_compat_2_2(obj);
> -    spapr_machine_initfn(obj);
>  }
>  
>  static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
> @@ -2446,7 +2444,6 @@ static void spapr_compat_2_1(Object *obj)
>  static void spapr_machine_2_1_instance_init(Object *obj)
>  {
>      spapr_compat_2_1(obj);
> -    spapr_machine_initfn(obj);
>  }
>  
>  static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)

Right, I just checked, and without this patch, spapr_machine_initfn() is
indeed called twice.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCHv2 03/10] pseries: Remove redundant calls to spapr_machine_initfn()

[Alexey Kardashevskiy]

On 12/07/2015 02:34 PM, David Gibson wrote:
> The instance_init() functions for several of the pseries-x.y versioned
> machine types explicitly call spapr_machine_initfn().  But that's the
> instance_init function for the common parent of all those machine types,
> so will already have been called beforehand by the QOM infrastructure.
>
> Remove the redundant calls.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   hw/ppc/spapr.c | 3 ---
>   1 file changed, 3 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c126e10..c7c6517 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2366,7 +2366,6 @@ static void spapr_compat_2_3(Object *obj)
>   static void spapr_machine_2_3_instance_init(Object *obj)
>   {
>       spapr_compat_2_3(obj);
> -    spapr_machine_initfn(obj);
>   }
>
>   static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
> @@ -2409,7 +2408,6 @@ static void spapr_compat_2_2(Object *obj)
>   static void spapr_machine_2_2_instance_init(Object *obj)
>   {
>       spapr_compat_2_2(obj);
> -    spapr_machine_initfn(obj);
>   }
>
>   static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
> @@ -2446,7 +2444,6 @@ static void spapr_compat_2_1(Object *obj)
>   static void spapr_machine_2_1_instance_init(Object *obj)
>   {
>       spapr_compat_2_1(obj);
> -    spapr_machine_initfn(obj);
>   }
>
>   static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
>

Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>	

-- 
Alexey

[Qemu-devel] [PATCHv2 04/10] pseries: Remove versions from mc->desc

[David Gibson]

Currently, the versioned spapr machine types put the machine type version
into the description string.  PC does not do this, using just the name
itself to distinguish.  Doing the same lets us move setting the description
into the common base class, simplifying the code slightly.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c7c6517..b08d338 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2265,6 +2265,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     NMIClass *nc = NMI_CLASS(oc);
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
+    mc->desc = "pSeries Logical Partition (PAPR compliant)";
     mc->init = ppc_spapr_init;
     mc->reset = ppc_spapr_reset;
     mc->block_default_type = IF_SCSI;
@@ -2309,7 +2310,6 @@ static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
     MachineClass *mc = MACHINE_CLASS(oc);
     sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
 
-    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
     mc->alias = "pseries";
     mc->is_default = 1;
     smc->dr_lmb_enabled = true;
@@ -2335,7 +2335,6 @@ static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
     };
     MachineClass *mc = MACHINE_CLASS(oc);
 
-    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
     mc->compat_props = compat_props;
 }
 
@@ -2376,7 +2375,6 @@ static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
     };
     MachineClass *mc = MACHINE_CLASS(oc);
 
-    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
     mc->compat_props = compat_props;
 }
 
@@ -2418,7 +2416,6 @@ static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
     };
     MachineClass *mc = MACHINE_CLASS(oc);
 
-    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.2";
     mc->compat_props = compat_props;
 }
 
@@ -2454,7 +2451,6 @@ static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
         { /* end of list */ }
     };
 
-    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
     mc->compat_props = compat_props;
 }
 
-- 
2.5.0

Re: [Qemu-devel] [PATCHv2 04/10] pseries: Remove versions from mc->desc

[Thomas Huth]

On 07/12/15 04:34, David Gibson wrote:
> Currently, the versioned spapr machine types put the machine type version
> into the description string.  PC does not do this, using just the name
> itself to distinguish.  Doing the same lets us move setting the description
> into the common base class, simplifying the code slightly.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c7c6517..b08d338 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2265,6 +2265,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>      NMIClass *nc = NMI_CLASS(oc);
>      HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>  
> +    mc->desc = "pSeries Logical Partition (PAPR compliant)";
>      mc->init = ppc_spapr_init;
>      mc->reset = ppc_spapr_reset;
>      mc->block_default_type = IF_SCSI;
> @@ -2309,7 +2310,6 @@ static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
>      MachineClass *mc = MACHINE_CLASS(oc);
>      sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
>  
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
>      mc->alias = "pseries";
>      mc->is_default = 1;
>      smc->dr_lmb_enabled = true;
> @@ -2335,7 +2335,6 @@ static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
>      };
>      MachineClass *mc = MACHINE_CLASS(oc);
>  
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
>      mc->compat_props = compat_props;
>  }
>  
> @@ -2376,7 +2375,6 @@ static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
>      };
>      MachineClass *mc = MACHINE_CLASS(oc);
>  
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
>      mc->compat_props = compat_props;
>  }
>  
> @@ -2418,7 +2416,6 @@ static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
>      };
>      MachineClass *mc = MACHINE_CLASS(oc);
>  
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.2";
>      mc->compat_props = compat_props;
>  }
>  
> @@ -2454,7 +2451,6 @@ static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
>          { /* end of list */ }
>      };
>  
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
>      mc->compat_props = compat_props;
>  }

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCHv2 04/10] pseries: Remove versions from mc->desc

[Alexey Kardashevskiy]

On 12/07/2015 02:34 PM, David Gibson wrote:
> Currently, the versioned spapr machine types put the machine type version
> into the description string.  PC does not do this, using just the name
> itself to distinguish.  Doing the same lets us move setting the description
> into the common base class, simplifying the code slightly.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   hw/ppc/spapr.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index c7c6517..b08d338 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2265,6 +2265,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>       NMIClass *nc = NMI_CLASS(oc);
>       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>
> +    mc->desc = "pSeries Logical Partition (PAPR compliant)";
>       mc->init = ppc_spapr_init;
>       mc->reset = ppc_spapr_reset;
>       mc->block_default_type = IF_SCSI;
> @@ -2309,7 +2310,6 @@ static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
>       MachineClass *mc = MACHINE_CLASS(oc);
>       sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
>
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.5";
>       mc->alias = "pseries";
>       mc->is_default = 1;
>       smc->dr_lmb_enabled = true;
> @@ -2335,7 +2335,6 @@ static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
>       };
>       MachineClass *mc = MACHINE_CLASS(oc);
>
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.4";
>       mc->compat_props = compat_props;
>   }
>
> @@ -2376,7 +2375,6 @@ static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
>       };
>       MachineClass *mc = MACHINE_CLASS(oc);
>
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.3";
>       mc->compat_props = compat_props;
>   }
>
> @@ -2418,7 +2416,6 @@ static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
>       };
>       MachineClass *mc = MACHINE_CLASS(oc);
>
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.2";
>       mc->compat_props = compat_props;
>   }
>
> @@ -2454,7 +2451,6 @@ static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
>           { /* end of list */ }
>       };
>
> -    mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
>       mc->compat_props = compat_props;
>   }
>
>

Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>	

-- 
Alexey

[Qemu-devel] [PATCHv2 05/10] Move SET_MACHINE_COMPAT macro to boards.h

[David Gibson]

pc.h defines a SET_MACHINE_COMPAT macro to make setting up compat_props
for the various PC machine versions less verbose.  There's nothing
inherently PC specific about it, though, so move it to boards.h where other
versioned machine types (like pseries-*) can use it.

While we're doing that, change it's indentation to be a bit more regular.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/hw/boards.h  | 9 +++++++++
 include/hw/i386/pc.h | 8 --------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 5da4fb0..9bf4ec8 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -153,4 +153,13 @@ struct MachineState {
     } \
     machine_init(machine_initfn##_register_types)
 
+#define SET_MACHINE_COMPAT(m, COMPAT) \
+    do {                              \
+        static GlobalProperty props[] = {       \
+            COMPAT                              \
+            { /* end of list */ }               \
+        };                                      \
+        (m)->compat_props = props;              \
+    } while (0)
+
 #endif
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 854c330..071a0d7 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -795,13 +795,5 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
     } \
     machine_init(pc_machine_init_##suffix)
 
-#define SET_MACHINE_COMPAT(m, COMPAT) do { \
-    static GlobalProperty props[] = { \
-        COMPAT \
-        { /* end of list */ } \
-    }; \
-    (m)->compat_props = props; \
-} while (0)
-
 extern void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id);
 #endif
-- 
2.5.0

Re: [Qemu-devel] [PATCHv2 05/10] Move SET_MACHINE_COMPAT macro to boards.h

[Alexey Kardashevskiy]

On 12/07/2015 02:34 PM, David Gibson wrote:
> pc.h defines a SET_MACHINE_COMPAT macro to make setting up compat_props
> for the various PC machine versions less verbose.  There's nothing
> inherently PC specific about it, though, so move it to boards.h where other
> versioned machine types (like pseries-*) can use it.
>
> While we're doing that, change it's indentation to be a bit more regular.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>   include/hw/boards.h  | 9 +++++++++
>   include/hw/i386/pc.h | 8 --------
>   2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 5da4fb0..9bf4ec8 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -153,4 +153,13 @@ struct MachineState {
>       } \
>       machine_init(machine_initfn##_register_types)
>
> +#define SET_MACHINE_COMPAT(m, COMPAT) \
> +    do {                              \
> +        static GlobalProperty props[] = {       \
> +            COMPAT                              \
> +            { /* end of list */ }               \
> +        };                                      \
> +        (m)->compat_props = props;              \
> +    } while (0)
> +
>   #endif
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 854c330..071a0d7 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -795,13 +795,5 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>       } \
>       machine_init(pc_machine_init_##suffix)
>
> -#define SET_MACHINE_COMPAT(m, COMPAT) do { \
> -    static GlobalProperty props[] = { \
> -        COMPAT \
> -        { /* end of list */ } \
> -    }; \
> -    (m)->compat_props = props; \
> -} while (0)
> -
>   extern void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id);
>   #endif
>

Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>	

-- 
Alexey

[Qemu-devel] [PATCHv2 06/10] pseries: Use SET_MACHINE_COMPAT

[David Gibson]

To make the spapr_machine_*_class_init() functions a little less bulky.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b08d338..3078e60 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2329,13 +2329,9 @@ static const TypeInfo spapr_machine_2_5_info = {
 
 static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
 {
-    static GlobalProperty compat_props[] = {
-        SPAPR_COMPAT_2_4
-        { /* end of list */ }
-    };
     MachineClass *mc = MACHINE_CLASS(oc);
 
-    mc->compat_props = compat_props;
+    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_4);
 }
 
 static const TypeInfo spapr_machine_2_4_info = {
@@ -2369,13 +2365,9 @@ static void spapr_machine_2_3_instance_init(Object *obj)
 
 static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
 {
-    static GlobalProperty compat_props[] = {
-        SPAPR_COMPAT_2_3
-        { /* end of list */ }
-    };
     MachineClass *mc = MACHINE_CLASS(oc);
 
-    mc->compat_props = compat_props;
+    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_3);
 }
 
 static const TypeInfo spapr_machine_2_3_info = {
@@ -2410,13 +2402,9 @@ static void spapr_machine_2_2_instance_init(Object *obj)
 
 static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
 {
-    static GlobalProperty compat_props[] = {
-        SPAPR_COMPAT_2_2
-        { /* end of list */ }
-    };
     MachineClass *mc = MACHINE_CLASS(oc);
 
-    mc->compat_props = compat_props;
+    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_2);
 }
 
 static const TypeInfo spapr_machine_2_2_info = {
@@ -2446,12 +2434,8 @@ static void spapr_machine_2_1_instance_init(Object *obj)
 static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
-    static GlobalProperty compat_props[] = {
-        SPAPR_COMPAT_2_1
-        { /* end of list */ }
-    };
 
-    mc->compat_props = compat_props;
+    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_1);
 }
 
 static const TypeInfo spapr_machine_2_1_info = {
-- 
2.5.0

Re: [Qemu-devel] [PATCHv2 06/10] pseries: Use SET_MACHINE_COMPAT

[Thomas Huth]

On 07/12/15 04:34, David Gibson wrote:
> To make the spapr_machine_*_class_init() functions a little less bulky.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr.c | 24 ++++--------------------
>  1 file changed, 4 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index b08d338..3078e60 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2329,13 +2329,9 @@ static const TypeInfo spapr_machine_2_5_info = {
>  
>  static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
>  {
> -    static GlobalProperty compat_props[] = {
> -        SPAPR_COMPAT_2_4
> -        { /* end of list */ }
> -    };
>      MachineClass *mc = MACHINE_CLASS(oc);
>  
> -    mc->compat_props = compat_props;
> +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_4);
>  }
>  
>  static const TypeInfo spapr_machine_2_4_info = {
> @@ -2369,13 +2365,9 @@ static void spapr_machine_2_3_instance_init(Object *obj)
>  
>  static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
>  {
> -    static GlobalProperty compat_props[] = {
> -        SPAPR_COMPAT_2_3
> -        { /* end of list */ }
> -    };
>      MachineClass *mc = MACHINE_CLASS(oc);
>  
> -    mc->compat_props = compat_props;
> +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_3);
>  }
>  
>  static const TypeInfo spapr_machine_2_3_info = {
> @@ -2410,13 +2402,9 @@ static void spapr_machine_2_2_instance_init(Object *obj)
>  
>  static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
>  {
> -    static GlobalProperty compat_props[] = {
> -        SPAPR_COMPAT_2_2
> -        { /* end of list */ }
> -    };
>      MachineClass *mc = MACHINE_CLASS(oc);
>  
> -    mc->compat_props = compat_props;
> +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_2);
>  }
>  
>  static const TypeInfo spapr_machine_2_2_info = {
> @@ -2446,12 +2434,8 @@ static void spapr_machine_2_1_instance_init(Object *obj)
>  static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
>  {
>      MachineClass *mc = MACHINE_CLASS(oc);
> -    static GlobalProperty compat_props[] = {
> -        SPAPR_COMPAT_2_1
> -        { /* end of list */ }
> -    };
>  
> -    mc->compat_props = compat_props;
> +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_1);
>  }
>  
>  static const TypeInfo spapr_machine_2_1_info = {

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCHv2 06/10] pseries: Use SET_MACHINE_COMPAT

[Alexey Kardashevskiy]

On 12/07/2015 02:34 PM, David Gibson wrote:
> To make the spapr_machine_*_class_init() functions a little less bulky.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   hw/ppc/spapr.c | 24 ++++--------------------
>   1 file changed, 4 insertions(+), 20 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index b08d338..3078e60 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2329,13 +2329,9 @@ static const TypeInfo spapr_machine_2_5_info = {
>
>   static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
>   {
> -    static GlobalProperty compat_props[] = {
> -        SPAPR_COMPAT_2_4
> -        { /* end of list */ }
> -    };
>       MachineClass *mc = MACHINE_CLASS(oc);
>
> -    mc->compat_props = compat_props;
> +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_4);
>   }
>
>   static const TypeInfo spapr_machine_2_4_info = {
> @@ -2369,13 +2365,9 @@ static void spapr_machine_2_3_instance_init(Object *obj)
>
>   static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
>   {
> -    static GlobalProperty compat_props[] = {
> -        SPAPR_COMPAT_2_3
> -        { /* end of list */ }
> -    };
>       MachineClass *mc = MACHINE_CLASS(oc);
>
> -    mc->compat_props = compat_props;
> +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_3);
>   }
>
>   static const TypeInfo spapr_machine_2_3_info = {
> @@ -2410,13 +2402,9 @@ static void spapr_machine_2_2_instance_init(Object *obj)
>
>   static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
>   {
> -    static GlobalProperty compat_props[] = {
> -        SPAPR_COMPAT_2_2
> -        { /* end of list */ }
> -    };
>       MachineClass *mc = MACHINE_CLASS(oc);
>
> -    mc->compat_props = compat_props;
> +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_2);
>   }
>
>   static const TypeInfo spapr_machine_2_2_info = {
> @@ -2446,12 +2434,8 @@ static void spapr_machine_2_1_instance_init(Object *obj)
>   static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
>   {
>       MachineClass *mc = MACHINE_CLASS(oc);
> -    static GlobalProperty compat_props[] = {
> -        SPAPR_COMPAT_2_1
> -        { /* end of list */ }
> -    };
>
> -    mc->compat_props = compat_props;
> +    SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_1);
>   }
>
>   static const TypeInfo spapr_machine_2_1_info = {
>

Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>	

-- 
Alexey

[Qemu-devel] [PATCHv2 07/10] pseries: DEFINE_SPAPR_MACHINE

[David Gibson]

At the moment all the class_init functions and TypeInfo structures for the
various versioned pseries machine types are open-coded.  As more versions
are created this is getting increasingly clumsy.

This patch borrows the approach used in PC, using a DEFINE_SPAPR_MACHINE()
macro to construct most of the boilerplate from simpler 'class_options' and
'instance_options' functions.

This patch makes a small semantic change - the versioned machine types are
now registered through machine_init() instead of type_init().  Since the
new way is how PC already did it, I'm assuming that's correct.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 119 ++++++++++++++++++++++++---------------------------------
 1 file changed, 49 insertions(+), 70 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3078e60..4f645f3 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2302,24 +2302,47 @@ static const TypeInfo spapr_machine_info = {
     },
 };
 
+#define DEFINE_SPAPR_MACHINE(suffix, verstr)                         \
+    static void spapr_machine_##suffix##_class_init(ObjectClass *oc, \
+                                                    void *data)      \
+    {                                                                \
+        MachineClass *mc = MACHINE_CLASS(oc);                        \
+        spapr_machine_##suffix##_class_options(mc);                  \
+    }                                                                \
+    static void spapr_machine_##suffix##_instance_init(Object *obj)  \
+    {                                                                \
+        MachineState *machine = MACHINE(obj);                        \
+        spapr_machine_##suffix##_instance_options(machine);          \
+    }                                                                \
+    static const TypeInfo spapr_machine_##suffix##_info = {          \
+        .name = MACHINE_TYPE_NAME("pseries-" verstr),                \
+        .parent = TYPE_SPAPR_MACHINE,                                \
+        .class_init = spapr_machine_##suffix##_class_init,           \
+        .instance_init = spapr_machine_##suffix##_instance_init,     \
+    };                                                               \
+    static void spapr_machine_register_##suffix(void)                \
+    {                                                                \
+        type_register(&spapr_machine_##suffix##_info);               \
+    }                                                                \
+    machine_init(spapr_machine_register_##suffix)
+
 /*
  * pseries-2.5
  */
-static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
+static void spapr_machine_2_5_instance_options(MachineState *machine)
 {
-    MachineClass *mc = MACHINE_CLASS(oc);
-    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
+}
+
+static void spapr_machine_2_5_class_options(MachineClass *mc)
+{
+    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
 
     mc->alias = "pseries";
     mc->is_default = 1;
     smc->dr_lmb_enabled = true;
 }
 
-static const TypeInfo spapr_machine_2_5_info = {
-    .name          = MACHINE_TYPE_NAME("pseries-2.5"),
-    .parent        = TYPE_SPAPR_MACHINE,
-    .class_init    = spapr_machine_2_5_class_init,
-};
+DEFINE_SPAPR_MACHINE(2_5, "2.5");
 
 /*
  * pseries-2.4
@@ -2327,18 +2350,17 @@ static const TypeInfo spapr_machine_2_5_info = {
 #define SPAPR_COMPAT_2_4 \
         HW_COMPAT_2_4
 
-static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
+static void spapr_machine_2_4_instance_options(MachineState *machine)
 {
-    MachineClass *mc = MACHINE_CLASS(oc);
+    spapr_machine_2_5_instance_options(machine);
+}
 
+static void spapr_machine_2_4_class_options(MachineClass *mc)
+{
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_4);
 }
 
-static const TypeInfo spapr_machine_2_4_info = {
-    .name          = MACHINE_TYPE_NAME("pseries-2.4"),
-    .parent        = TYPE_SPAPR_MACHINE,
-    .class_init    = spapr_machine_2_4_class_init,
-};
+DEFINE_SPAPR_MACHINE(2_4, "2.4");
 
 /*
  * pseries-2.3
@@ -2352,30 +2374,18 @@ static const TypeInfo spapr_machine_2_4_info = {
             .value    = "off",\
         },
 
-static void spapr_compat_2_3(Object *obj)
+static void spapr_machine_2_3_instance_options(MachineState *machine)
 {
+    spapr_machine_2_4_instance_options(machine);
     savevm_skip_section_footers();
     global_state_set_optional();
 }
 
-static void spapr_machine_2_3_instance_init(Object *obj)
-{
-    spapr_compat_2_3(obj);
-}
-
-static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
+static void spapr_machine_2_3_class_options(MachineClass *mc)
 {
-    MachineClass *mc = MACHINE_CLASS(oc);
-
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_3);
 }
-
-static const TypeInfo spapr_machine_2_3_info = {
-    .name          = MACHINE_TYPE_NAME("pseries-2.3"),
-    .parent        = TYPE_SPAPR_MACHINE,
-    .class_init    = spapr_machine_2_3_class_init,
-    .instance_init = spapr_machine_2_3_instance_init,
-};
+DEFINE_SPAPR_MACHINE(2_3, "2.3");
 
 /*
  * pseries-2.2
@@ -2390,29 +2400,16 @@ static const TypeInfo spapr_machine_2_3_info = {
             .value    = "0x20000000",\
         },
 
-static void spapr_compat_2_2(Object *obj)
-{
-    spapr_compat_2_3(obj);
-}
-
-static void spapr_machine_2_2_instance_init(Object *obj)
+static void spapr_machine_2_2_instance_options(MachineState *machine)
 {
-    spapr_compat_2_2(obj);
+    spapr_machine_2_3_instance_options(machine);
 }
 
-static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
+static void spapr_machine_2_2_class_options(MachineClass *mc)
 {
-    MachineClass *mc = MACHINE_CLASS(oc);
-
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_2);
 }
-
-static const TypeInfo spapr_machine_2_2_info = {
-    .name          = MACHINE_TYPE_NAME("pseries-2.2"),
-    .parent        = TYPE_SPAPR_MACHINE,
-    .class_init    = spapr_machine_2_2_class_init,
-    .instance_init = spapr_machine_2_2_instance_init,
-};
+DEFINE_SPAPR_MACHINE(2_2, "2.2");
 
 /*
  * pseries-2.1
@@ -2421,38 +2418,20 @@ static const TypeInfo spapr_machine_2_2_info = {
         SPAPR_COMPAT_2_2 \
         HW_COMPAT_2_1
 
-static void spapr_compat_2_1(Object *obj)
-{
-    spapr_compat_2_2(obj);
-}
-
-static void spapr_machine_2_1_instance_init(Object *obj)
+static void spapr_machine_2_1_instance_options(MachineState *machine)
 {
-    spapr_compat_2_1(obj);
+    spapr_machine_2_2_instance_options(machine);
 }
 
-static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
+static void spapr_machine_2_1_class_options(MachineClass *mc)
 {
-    MachineClass *mc = MACHINE_CLASS(oc);
-
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_1);
 }
-
-static const TypeInfo spapr_machine_2_1_info = {
-    .name          = MACHINE_TYPE_NAME("pseries-2.1"),
-    .parent        = TYPE_SPAPR_MACHINE,
-    .class_init    = spapr_machine_2_1_class_init,
-    .instance_init = spapr_machine_2_1_instance_init,
-};
+DEFINE_SPAPR_MACHINE(2_1, "2.1");
 
 static void spapr_machine_register_types(void)
 {
     type_register_static(&spapr_machine_info);
-    type_register_static(&spapr_machine_2_1_info);
-    type_register_static(&spapr_machine_2_2_info);
-    type_register_static(&spapr_machine_2_3_info);
-    type_register_static(&spapr_machine_2_4_info);
-    type_register_static(&spapr_machine_2_5_info);
 }
 
 type_init(spapr_machine_register_types)
-- 
2.5.0

Re: [Qemu-devel] [Qemu-ppc] [PATCHv2 07/10] pseries: DEFINE_SPAPR_MACHINE

[Sam Bobroff]

On Mon, Dec 07, 2015 at 02:34:37PM +1100, David Gibson wrote:
> At the moment all the class_init functions and TypeInfo structures for the
> various versioned pseries machine types are open-coded.  As more versions
> are created this is getting increasingly clumsy.
> 
> This patch borrows the approach used in PC, using a DEFINE_SPAPR_MACHINE()
> macro to construct most of the boilerplate from simpler 'class_options' and
> 'instance_options' functions.
> 
> This patch makes a small semantic change - the versioned machine types are
> now registered through machine_init() instead of type_init().  Since the
> new way is how PC already did it, I'm assuming that's correct.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr.c | 119 ++++++++++++++++++++++++---------------------------------
>  1 file changed, 49 insertions(+), 70 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3078e60..4f645f3 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2302,24 +2302,47 @@ static const TypeInfo spapr_machine_info = {
>      },
>  };
>  
> +#define DEFINE_SPAPR_MACHINE(suffix, verstr)                         \
> +    static void spapr_machine_##suffix##_class_init(ObjectClass *oc, \
> +                                                    void *data)      \
> +    {                                                                \
> +        MachineClass *mc = MACHINE_CLASS(oc);                        \
> +        spapr_machine_##suffix##_class_options(mc);                  \
> +    }                                                                \
> +    static void spapr_machine_##suffix##_instance_init(Object *obj)  \
> +    {                                                                \
> +        MachineState *machine = MACHINE(obj);                        \
> +        spapr_machine_##suffix##_instance_options(machine);          \
> +    }                                                                \
> +    static const TypeInfo spapr_machine_##suffix##_info = {          \
> +        .name = MACHINE_TYPE_NAME("pseries-" verstr),                \
> +        .parent = TYPE_SPAPR_MACHINE,                                \
> +        .class_init = spapr_machine_##suffix##_class_init,           \
> +        .instance_init = spapr_machine_##suffix##_instance_init,     \
> +    };                                                               \
> +    static void spapr_machine_register_##suffix(void)                \
> +    {                                                                \
> +        type_register(&spapr_machine_##suffix##_info);               \
> +    }                                                                \
> +    machine_init(spapr_machine_register_##suffix)
> +
>  /*
>   * pseries-2.5
>   */
> -static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
> +static void spapr_machine_2_5_instance_options(MachineState *machine)
>  {
> -    MachineClass *mc = MACHINE_CLASS(oc);
> -    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
> +}
> +
> +static void spapr_machine_2_5_class_options(MachineClass *mc)
> +{
> +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
>  
>      mc->alias = "pseries";
>      mc->is_default = 1;
>      smc->dr_lmb_enabled = true;
>  }
>  
> -static const TypeInfo spapr_machine_2_5_info = {
> -    .name          = MACHINE_TYPE_NAME("pseries-2.5"),
> -    .parent        = TYPE_SPAPR_MACHINE,
> -    .class_init    = spapr_machine_2_5_class_init,
> -};
> +DEFINE_SPAPR_MACHINE(2_5, "2.5");
>  
>  /*
>   * pseries-2.4
> @@ -2327,18 +2350,17 @@ static const TypeInfo spapr_machine_2_5_info = {
>  #define SPAPR_COMPAT_2_4 \
>          HW_COMPAT_2_4
>  
> -static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
> +static void spapr_machine_2_4_instance_options(MachineState *machine)
>  {
> -    MachineClass *mc = MACHINE_CLASS(oc);
> +    spapr_machine_2_5_instance_options(machine);
> +}
>  
> +static void spapr_machine_2_4_class_options(MachineClass *mc)
> +{
>      SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_4);
>  }
>  
> -static const TypeInfo spapr_machine_2_4_info = {
> -    .name          = MACHINE_TYPE_NAME("pseries-2.4"),
> -    .parent        = TYPE_SPAPR_MACHINE,
> -    .class_init    = spapr_machine_2_4_class_init,
> -};
> +DEFINE_SPAPR_MACHINE(2_4, "2.4");
>  
>  /*
>   * pseries-2.3
> @@ -2352,30 +2374,18 @@ static const TypeInfo spapr_machine_2_4_info = {
>              .value    = "off",\
>          },
>  
> -static void spapr_compat_2_3(Object *obj)
> +static void spapr_machine_2_3_instance_options(MachineState *machine)
>  {
> +    spapr_machine_2_4_instance_options(machine);
>      savevm_skip_section_footers();
>      global_state_set_optional();
>  }
>  
> -static void spapr_machine_2_3_instance_init(Object *obj)
> -{
> -    spapr_compat_2_3(obj);
> -}
> -
> -static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
> +static void spapr_machine_2_3_class_options(MachineClass *mc)
>  {
> -    MachineClass *mc = MACHINE_CLASS(oc);
> -
>      SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_3);
>  }
> -
> -static const TypeInfo spapr_machine_2_3_info = {
> -    .name          = MACHINE_TYPE_NAME("pseries-2.3"),
> -    .parent        = TYPE_SPAPR_MACHINE,
> -    .class_init    = spapr_machine_2_3_class_init,
> -    .instance_init = spapr_machine_2_3_instance_init,
> -};
> +DEFINE_SPAPR_MACHINE(2_3, "2.3");
>  
>  /*
>   * pseries-2.2
> @@ -2390,29 +2400,16 @@ static const TypeInfo spapr_machine_2_3_info = {
>              .value    = "0x20000000",\
>          },
>  
> -static void spapr_compat_2_2(Object *obj)
> -{
> -    spapr_compat_2_3(obj);
> -}
> -
> -static void spapr_machine_2_2_instance_init(Object *obj)
> +static void spapr_machine_2_2_instance_options(MachineState *machine)
>  {
> -    spapr_compat_2_2(obj);
> +    spapr_machine_2_3_instance_options(machine);
>  }
>  
> -static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
> +static void spapr_machine_2_2_class_options(MachineClass *mc)
>  {
> -    MachineClass *mc = MACHINE_CLASS(oc);
> -
>      SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_2);
>  }
> -
> -static const TypeInfo spapr_machine_2_2_info = {
> -    .name          = MACHINE_TYPE_NAME("pseries-2.2"),
> -    .parent        = TYPE_SPAPR_MACHINE,
> -    .class_init    = spapr_machine_2_2_class_init,
> -    .instance_init = spapr_machine_2_2_instance_init,
> -};
> +DEFINE_SPAPR_MACHINE(2_2, "2.2");
>  
>  /*
>   * pseries-2.1
> @@ -2421,38 +2418,20 @@ static const TypeInfo spapr_machine_2_2_info = {
>          SPAPR_COMPAT_2_2 \
>          HW_COMPAT_2_1
>  
> -static void spapr_compat_2_1(Object *obj)
> -{
> -    spapr_compat_2_2(obj);
> -}
> -
> -static void spapr_machine_2_1_instance_init(Object *obj)
> +static void spapr_machine_2_1_instance_options(MachineState *machine)
>  {
> -    spapr_compat_2_1(obj);
> +    spapr_machine_2_2_instance_options(machine);
>  }
>  
> -static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
> +static void spapr_machine_2_1_class_options(MachineClass *mc)
>  {
> -    MachineClass *mc = MACHINE_CLASS(oc);
> -
>      SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_1);
>  }
> -
> -static const TypeInfo spapr_machine_2_1_info = {
> -    .name          = MACHINE_TYPE_NAME("pseries-2.1"),
> -    .parent        = TYPE_SPAPR_MACHINE,
> -    .class_init    = spapr_machine_2_1_class_init,
> -    .instance_init = spapr_machine_2_1_instance_init,
> -};
> +DEFINE_SPAPR_MACHINE(2_1, "2.1");
>  
>  static void spapr_machine_register_types(void)
>  {
>      type_register_static(&spapr_machine_info);
> -    type_register_static(&spapr_machine_2_1_info);
> -    type_register_static(&spapr_machine_2_2_info);
> -    type_register_static(&spapr_machine_2_3_info);
> -    type_register_static(&spapr_machine_2_4_info);
> -    type_register_static(&spapr_machine_2_5_info);
>  }
>  
>  type_init(spapr_machine_register_types)
> -- 
> 2.5.0

Looks good to me. Just one nit: I couldn't help noticing that the suffix and
verstr parameters to the macro were both encoding the same information.

Did you consider a macro with the major and minor version as the parameters?

e.g.
#define DEFINE_SPAPR_MACHINE(major, minor)

So usage would be:
DEFINE_SPAPR_MACHINE(2, 1);

Reviewed-by: Sam Bobroff <sam.bobroff@au1.ibm.com>

Re: [Qemu-devel] [Qemu-ppc] [PATCHv2 07/10] pseries: DEFINE_SPAPR_MACHINE

[Alexey Kardashevskiy]

On 12/08/2015 01:38 PM, Sam Bobroff wrote:
> On Mon, Dec 07, 2015 at 02:34:37PM +1100, David Gibson wrote:
>> At the moment all the class_init functions and TypeInfo structures for the
>> various versioned pseries machine types are open-coded.  As more versions
>> are created this is getting increasingly clumsy.
>>
>> This patch borrows the approach used in PC, using a DEFINE_SPAPR_MACHINE()
>> macro to construct most of the boilerplate from simpler 'class_options' and
>> 'instance_options' functions.
>>
>> This patch makes a small semantic change - the versioned machine types are
>> now registered through machine_init() instead of type_init().  Since the
>> new way is how PC already did it, I'm assuming that's correct.
>>
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>>   hw/ppc/spapr.c | 119 ++++++++++++++++++++++++---------------------------------
>>   1 file changed, 49 insertions(+), 70 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 3078e60..4f645f3 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -2302,24 +2302,47 @@ static const TypeInfo spapr_machine_info = {
>>       },
>>   };
>>
>> +#define DEFINE_SPAPR_MACHINE(suffix, verstr)                         \
>> +    static void spapr_machine_##suffix##_class_init(ObjectClass *oc, \
>> +                                                    void *data)      \
>> +    {                                                                \
>> +        MachineClass *mc = MACHINE_CLASS(oc);                        \
>> +        spapr_machine_##suffix##_class_options(mc);                  \
>> +    }                                                                \
>> +    static void spapr_machine_##suffix##_instance_init(Object *obj)  \
>> +    {                                                                \
>> +        MachineState *machine = MACHINE(obj);                        \
>> +        spapr_machine_##suffix##_instance_options(machine);          \
>> +    }                                                                \
>> +    static const TypeInfo spapr_machine_##suffix##_info = {          \
>> +        .name = MACHINE_TYPE_NAME("pseries-" verstr),                \
>> +        .parent = TYPE_SPAPR_MACHINE,                                \
>> +        .class_init = spapr_machine_##suffix##_class_init,           \
>> +        .instance_init = spapr_machine_##suffix##_instance_init,     \
>> +    };                                                               \
>> +    static void spapr_machine_register_##suffix(void)                \
>> +    {                                                                \
>> +        type_register(&spapr_machine_##suffix##_info);               \
>> +    }                                                                \
>> +    machine_init(spapr_machine_register_##suffix)
>> +
>>   /*
>>    * pseries-2.5
>>    */
>> -static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
>> +static void spapr_machine_2_5_instance_options(MachineState *machine)
>>   {
>> -    MachineClass *mc = MACHINE_CLASS(oc);
>> -    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
>> +}
>> +
>> +static void spapr_machine_2_5_class_options(MachineClass *mc)
>> +{
>> +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
>>
>>       mc->alias = "pseries";
>>       mc->is_default = 1;
>>       smc->dr_lmb_enabled = true;
>>   }
>>
>> -static const TypeInfo spapr_machine_2_5_info = {
>> -    .name          = MACHINE_TYPE_NAME("pseries-2.5"),
>> -    .parent        = TYPE_SPAPR_MACHINE,
>> -    .class_init    = spapr_machine_2_5_class_init,
>> -};
>> +DEFINE_SPAPR_MACHINE(2_5, "2.5");
>>
>>   /*
>>    * pseries-2.4
>> @@ -2327,18 +2350,17 @@ static const TypeInfo spapr_machine_2_5_info = {
>>   #define SPAPR_COMPAT_2_4 \
>>           HW_COMPAT_2_4
>>
>> -static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
>> +static void spapr_machine_2_4_instance_options(MachineState *machine)
>>   {
>> -    MachineClass *mc = MACHINE_CLASS(oc);
>> +    spapr_machine_2_5_instance_options(machine);
>> +}
>>
>> +static void spapr_machine_2_4_class_options(MachineClass *mc)
>> +{
>>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_4);
>>   }
>>
>> -static const TypeInfo spapr_machine_2_4_info = {
>> -    .name          = MACHINE_TYPE_NAME("pseries-2.4"),
>> -    .parent        = TYPE_SPAPR_MACHINE,
>> -    .class_init    = spapr_machine_2_4_class_init,
>> -};
>> +DEFINE_SPAPR_MACHINE(2_4, "2.4");
>>
>>   /*
>>    * pseries-2.3
>> @@ -2352,30 +2374,18 @@ static const TypeInfo spapr_machine_2_4_info = {
>>               .value    = "off",\
>>           },
>>
>> -static void spapr_compat_2_3(Object *obj)
>> +static void spapr_machine_2_3_instance_options(MachineState *machine)
>>   {
>> +    spapr_machine_2_4_instance_options(machine);
>>       savevm_skip_section_footers();
>>       global_state_set_optional();
>>   }
>>
>> -static void spapr_machine_2_3_instance_init(Object *obj)
>> -{
>> -    spapr_compat_2_3(obj);
>> -}
>> -
>> -static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
>> +static void spapr_machine_2_3_class_options(MachineClass *mc)
>>   {
>> -    MachineClass *mc = MACHINE_CLASS(oc);
>> -
>>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_3);
>>   }
>> -
>> -static const TypeInfo spapr_machine_2_3_info = {
>> -    .name          = MACHINE_TYPE_NAME("pseries-2.3"),
>> -    .parent        = TYPE_SPAPR_MACHINE,
>> -    .class_init    = spapr_machine_2_3_class_init,
>> -    .instance_init = spapr_machine_2_3_instance_init,
>> -};
>> +DEFINE_SPAPR_MACHINE(2_3, "2.3");
>>
>>   /*
>>    * pseries-2.2
>> @@ -2390,29 +2400,16 @@ static const TypeInfo spapr_machine_2_3_info = {
>>               .value    = "0x20000000",\
>>           },
>>
>> -static void spapr_compat_2_2(Object *obj)
>> -{
>> -    spapr_compat_2_3(obj);
>> -}
>> -
>> -static void spapr_machine_2_2_instance_init(Object *obj)
>> +static void spapr_machine_2_2_instance_options(MachineState *machine)
>>   {
>> -    spapr_compat_2_2(obj);
>> +    spapr_machine_2_3_instance_options(machine);
>>   }
>>
>> -static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
>> +static void spapr_machine_2_2_class_options(MachineClass *mc)
>>   {
>> -    MachineClass *mc = MACHINE_CLASS(oc);
>> -
>>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_2);
>>   }
>> -
>> -static const TypeInfo spapr_machine_2_2_info = {
>> -    .name          = MACHINE_TYPE_NAME("pseries-2.2"),
>> -    .parent        = TYPE_SPAPR_MACHINE,
>> -    .class_init    = spapr_machine_2_2_class_init,
>> -    .instance_init = spapr_machine_2_2_instance_init,
>> -};
>> +DEFINE_SPAPR_MACHINE(2_2, "2.2");
>>
>>   /*
>>    * pseries-2.1
>> @@ -2421,38 +2418,20 @@ static const TypeInfo spapr_machine_2_2_info = {
>>           SPAPR_COMPAT_2_2 \
>>           HW_COMPAT_2_1
>>
>> -static void spapr_compat_2_1(Object *obj)
>> -{
>> -    spapr_compat_2_2(obj);
>> -}
>> -
>> -static void spapr_machine_2_1_instance_init(Object *obj)
>> +static void spapr_machine_2_1_instance_options(MachineState *machine)
>>   {
>> -    spapr_compat_2_1(obj);
>> +    spapr_machine_2_2_instance_options(machine);
>>   }
>>
>> -static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
>> +static void spapr_machine_2_1_class_options(MachineClass *mc)
>>   {
>> -    MachineClass *mc = MACHINE_CLASS(oc);
>> -
>>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_1);
>>   }
>> -
>> -static const TypeInfo spapr_machine_2_1_info = {
>> -    .name          = MACHINE_TYPE_NAME("pseries-2.1"),
>> -    .parent        = TYPE_SPAPR_MACHINE,
>> -    .class_init    = spapr_machine_2_1_class_init,
>> -    .instance_init = spapr_machine_2_1_instance_init,
>> -};
>> +DEFINE_SPAPR_MACHINE(2_1, "2.1");
>>
>>   static void spapr_machine_register_types(void)
>>   {
>>       type_register_static(&spapr_machine_info);
>> -    type_register_static(&spapr_machine_2_1_info);
>> -    type_register_static(&spapr_machine_2_2_info);
>> -    type_register_static(&spapr_machine_2_3_info);
>> -    type_register_static(&spapr_machine_2_4_info);
>> -    type_register_static(&spapr_machine_2_5_info);
>>   }
>>
>>   type_init(spapr_machine_register_types)
>> --
>> 2.5.0
>
> Looks good to me. Just one nit: I couldn't help noticing that the suffix and
> verstr parameters to the macro were both encoding the same information.
>
> Did you consider a macro with the major and minor version as the parameters?
>
> e.g.
> #define DEFINE_SPAPR_MACHINE(major, minor)
>
> So usage would be:
> DEFINE_SPAPR_MACHINE(2, 1);


I suggested this earlier but for unknown reason this was rejected :)


>
> Reviewed-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
>


-- 
Alexey

Re: [Qemu-devel] [Qemu-ppc] [PATCHv2 07/10] pseries: DEFINE_SPAPR_MACHINE

[Alexey Kardashevskiy]

On 12/09/2015 02:30 PM, Alexey Kardashevskiy wrote:
> On 12/08/2015 01:38 PM, Sam Bobroff wrote:
>> On Mon, Dec 07, 2015 at 02:34:37PM +1100, David Gibson wrote:
>>> At the moment all the class_init functions and TypeInfo structures for the
>>> various versioned pseries machine types are open-coded.  As more versions
>>> are created this is getting increasingly clumsy.
>>>
>>> This patch borrows the approach used in PC, using a DEFINE_SPAPR_MACHINE()
>>> macro to construct most of the boilerplate from simpler 'class_options' and
>>> 'instance_options' functions.
>>>
>>> This patch makes a small semantic change - the versioned machine types are
>>> now registered through machine_init() instead of type_init().  Since the
>>> new way is how PC already did it, I'm assuming that's correct.
>>>
>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>> ---
>>>   hw/ppc/spapr.c | 119
>>> ++++++++++++++++++++++++---------------------------------
>>>   1 file changed, 49 insertions(+), 70 deletions(-)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index 3078e60..4f645f3 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -2302,24 +2302,47 @@ static const TypeInfo spapr_machine_info = {
>>>       },
>>>   };
>>>
>>> +#define DEFINE_SPAPR_MACHINE(suffix, verstr)                         \
>>> +    static void spapr_machine_##suffix##_class_init(ObjectClass *oc, \
>>> +                                                    void *data)      \
>>> +    {                                                                \
>>> +        MachineClass *mc = MACHINE_CLASS(oc);                        \
>>> +        spapr_machine_##suffix##_class_options(mc);                  \
>>> +    }                                                                \
>>> +    static void spapr_machine_##suffix##_instance_init(Object *obj)  \
>>> +    {                                                                \
>>> +        MachineState *machine = MACHINE(obj);                        \
>>> +        spapr_machine_##suffix##_instance_options(machine);          \
>>> +    }                                                                \
>>> +    static const TypeInfo spapr_machine_##suffix##_info = {          \
>>> +        .name = MACHINE_TYPE_NAME("pseries-" verstr),                \
>>> +        .parent = TYPE_SPAPR_MACHINE,                                \
>>> +        .class_init = spapr_machine_##suffix##_class_init,           \
>>> +        .instance_init = spapr_machine_##suffix##_instance_init,     \
>>> +    };                                                               \
>>> +    static void spapr_machine_register_##suffix(void)                \
>>> +    {                                                                \
>>> +        type_register(&spapr_machine_##suffix##_info);               \
>>> +    }                                                                \
>>> +    machine_init(spapr_machine_register_##suffix)
>>> +
>>>   /*
>>>    * pseries-2.5
>>>    */
>>> -static void spapr_machine_2_5_class_init(ObjectClass *oc, void *data)
>>> +static void spapr_machine_2_5_instance_options(MachineState *machine)
>>>   {
>>> -    MachineClass *mc = MACHINE_CLASS(oc);
>>> -    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(oc);
>>> +}
>>> +
>>> +static void spapr_machine_2_5_class_options(MachineClass *mc)
>>> +{
>>> +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
>>>
>>>       mc->alias = "pseries";
>>>       mc->is_default = 1;
>>>       smc->dr_lmb_enabled = true;
>>>   }
>>>
>>> -static const TypeInfo spapr_machine_2_5_info = {
>>> -    .name          = MACHINE_TYPE_NAME("pseries-2.5"),
>>> -    .parent        = TYPE_SPAPR_MACHINE,
>>> -    .class_init    = spapr_machine_2_5_class_init,
>>> -};
>>> +DEFINE_SPAPR_MACHINE(2_5, "2.5");
>>>
>>>   /*
>>>    * pseries-2.4
>>> @@ -2327,18 +2350,17 @@ static const TypeInfo spapr_machine_2_5_info = {
>>>   #define SPAPR_COMPAT_2_4 \
>>>           HW_COMPAT_2_4
>>>
>>> -static void spapr_machine_2_4_class_init(ObjectClass *oc, void *data)
>>> +static void spapr_machine_2_4_instance_options(MachineState *machine)
>>>   {
>>> -    MachineClass *mc = MACHINE_CLASS(oc);
>>> +    spapr_machine_2_5_instance_options(machine);
>>> +}
>>>
>>> +static void spapr_machine_2_4_class_options(MachineClass *mc)
>>> +{
>>>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_4);
>>>   }
>>>
>>> -static const TypeInfo spapr_machine_2_4_info = {
>>> -    .name          = MACHINE_TYPE_NAME("pseries-2.4"),
>>> -    .parent        = TYPE_SPAPR_MACHINE,
>>> -    .class_init    = spapr_machine_2_4_class_init,
>>> -};
>>> +DEFINE_SPAPR_MACHINE(2_4, "2.4");
>>>
>>>   /*
>>>    * pseries-2.3
>>> @@ -2352,30 +2374,18 @@ static const TypeInfo spapr_machine_2_4_info = {
>>>               .value    = "off",\
>>>           },
>>>
>>> -static void spapr_compat_2_3(Object *obj)
>>> +static void spapr_machine_2_3_instance_options(MachineState *machine)
>>>   {
>>> +    spapr_machine_2_4_instance_options(machine);
>>>       savevm_skip_section_footers();
>>>       global_state_set_optional();
>>>   }
>>>
>>> -static void spapr_machine_2_3_instance_init(Object *obj)
>>> -{
>>> -    spapr_compat_2_3(obj);
>>> -}
>>> -
>>> -static void spapr_machine_2_3_class_init(ObjectClass *oc, void *data)
>>> +static void spapr_machine_2_3_class_options(MachineClass *mc)
>>>   {
>>> -    MachineClass *mc = MACHINE_CLASS(oc);
>>> -
>>>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_3);
>>>   }
>>> -
>>> -static const TypeInfo spapr_machine_2_3_info = {
>>> -    .name          = MACHINE_TYPE_NAME("pseries-2.3"),
>>> -    .parent        = TYPE_SPAPR_MACHINE,
>>> -    .class_init    = spapr_machine_2_3_class_init,
>>> -    .instance_init = spapr_machine_2_3_instance_init,
>>> -};
>>> +DEFINE_SPAPR_MACHINE(2_3, "2.3");
>>>
>>>   /*
>>>    * pseries-2.2
>>> @@ -2390,29 +2400,16 @@ static const TypeInfo spapr_machine_2_3_info = {
>>>               .value    = "0x20000000",\
>>>           },
>>>
>>> -static void spapr_compat_2_2(Object *obj)
>>> -{
>>> -    spapr_compat_2_3(obj);
>>> -}
>>> -
>>> -static void spapr_machine_2_2_instance_init(Object *obj)
>>> +static void spapr_machine_2_2_instance_options(MachineState *machine)
>>>   {
>>> -    spapr_compat_2_2(obj);
>>> +    spapr_machine_2_3_instance_options(machine);
>>>   }
>>>
>>> -static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
>>> +static void spapr_machine_2_2_class_options(MachineClass *mc)
>>>   {
>>> -    MachineClass *mc = MACHINE_CLASS(oc);
>>> -
>>>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_2);
>>>   }
>>> -
>>> -static const TypeInfo spapr_machine_2_2_info = {
>>> -    .name          = MACHINE_TYPE_NAME("pseries-2.2"),
>>> -    .parent        = TYPE_SPAPR_MACHINE,
>>> -    .class_init    = spapr_machine_2_2_class_init,
>>> -    .instance_init = spapr_machine_2_2_instance_init,
>>> -};
>>> +DEFINE_SPAPR_MACHINE(2_2, "2.2");
>>>
>>>   /*
>>>    * pseries-2.1
>>> @@ -2421,38 +2418,20 @@ static const TypeInfo spapr_machine_2_2_info = {
>>>           SPAPR_COMPAT_2_2 \
>>>           HW_COMPAT_2_1
>>>
>>> -static void spapr_compat_2_1(Object *obj)
>>> -{
>>> -    spapr_compat_2_2(obj);
>>> -}
>>> -
>>> -static void spapr_machine_2_1_instance_init(Object *obj)
>>> +static void spapr_machine_2_1_instance_options(MachineState *machine)
>>>   {
>>> -    spapr_compat_2_1(obj);
>>> +    spapr_machine_2_2_instance_options(machine);
>>>   }
>>>
>>> -static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
>>> +static void spapr_machine_2_1_class_options(MachineClass *mc)
>>>   {
>>> -    MachineClass *mc = MACHINE_CLASS(oc);
>>> -
>>>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_1);
>>>   }
>>> -
>>> -static const TypeInfo spapr_machine_2_1_info = {
>>> -    .name          = MACHINE_TYPE_NAME("pseries-2.1"),
>>> -    .parent        = TYPE_SPAPR_MACHINE,
>>> -    .class_init    = spapr_machine_2_1_class_init,
>>> -    .instance_init = spapr_machine_2_1_instance_init,
>>> -};
>>> +DEFINE_SPAPR_MACHINE(2_1, "2.1");
>>>
>>>   static void spapr_machine_register_types(void)
>>>   {
>>>       type_register_static(&spapr_machine_info);
>>> -    type_register_static(&spapr_machine_2_1_info);
>>> -    type_register_static(&spapr_machine_2_2_info);
>>> -    type_register_static(&spapr_machine_2_3_info);
>>> -    type_register_static(&spapr_machine_2_4_info);
>>> -    type_register_static(&spapr_machine_2_5_info);
>>>   }
>>>
>>>   type_init(spapr_machine_register_types)
>>> --
>>> 2.5.0
>>
>> Looks good to me. Just one nit: I couldn't help noticing that the suffix and
>> verstr parameters to the macro were both encoding the same information.
>>
>> Did you consider a macro with the major and minor version as the parameters?
>>
>> e.g.
>> #define DEFINE_SPAPR_MACHINE(major, minor)
>>
>> So usage would be:
>> DEFINE_SPAPR_MACHINE(2, 1);
>
>
> I suggested this earlier but for unknown reason this was rejected :)

ah, also:

Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>	


>
>
>>
>> Reviewed-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
>>
>
>


-- 
Alexey

[Qemu-devel] [PATCHv2 08/10] pseries: Restructure class_options functions

[David Gibson]

Currently each of the *_class_options() functions for the pseries-2.1 ..
pseries-2.5 machine types are standalone.  This will become harder to
maintain as new versions are added.

This patch restructures them similarly to x86 where each function calls
the one from the next version, then overrides anything necessary for
compatibility with the specific version and older.

The default behaviour - that for the most recent machine are set up in
the base class initializer spapr_machine_class_init().  Previously it had
some things set up to default to older behaviour with the more recent
machines overriding it.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4f645f3..5af3d13 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2266,6 +2266,12 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
 
     mc->desc = "pSeries Logical Partition (PAPR compliant)";
+
+    /*
+     * We set up the default / latest behaviour here.  The class_init
+     * functions for the specific versioned machine types can override
+     * these details for backwards compatibility
+     */
     mc->init = ppc_spapr_init;
     mc->reset = ppc_spapr_reset;
     mc->block_default_type = IF_SCSI;
@@ -2281,7 +2287,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     hc->unplug = spapr_machine_device_unplug;
     mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
 
-    smc->dr_lmb_enabled = false;
+    smc->dr_lmb_enabled = true;
     fwc->get_dev_path = spapr_get_fw_dev_path;
     nc->nmi_monitor_handler = spapr_nmi;
 }
@@ -2335,11 +2341,9 @@ static void spapr_machine_2_5_instance_options(MachineState *machine)
 
 static void spapr_machine_2_5_class_options(MachineClass *mc)
 {
-    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
-
+    /* Defaults for the latest behaviour inherited from the base class */
     mc->alias = "pseries";
     mc->is_default = 1;
-    smc->dr_lmb_enabled = true;
 }
 
 DEFINE_SPAPR_MACHINE(2_5, "2.5");
@@ -2357,6 +2361,12 @@ static void spapr_machine_2_4_instance_options(MachineState *machine)
 
 static void spapr_machine_2_4_class_options(MachineClass *mc)
 {
+    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
+
+    spapr_machine_2_5_class_options(mc);
+    mc->alias = NULL;
+    mc->is_default = 0;
+    smc->dr_lmb_enabled = false;
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_4);
 }
 
@@ -2383,6 +2393,7 @@ static void spapr_machine_2_3_instance_options(MachineState *machine)
 
 static void spapr_machine_2_3_class_options(MachineClass *mc)
 {
+    spapr_machine_2_4_class_options(mc);
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_3);
 }
 DEFINE_SPAPR_MACHINE(2_3, "2.3");
@@ -2407,6 +2418,7 @@ static void spapr_machine_2_2_instance_options(MachineState *machine)
 
 static void spapr_machine_2_2_class_options(MachineClass *mc)
 {
+    spapr_machine_2_3_class_options(mc);
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_2);
 }
 DEFINE_SPAPR_MACHINE(2_2, "2.2");
@@ -2425,6 +2437,7 @@ static void spapr_machine_2_1_instance_options(MachineState *machine)
 
 static void spapr_machine_2_1_class_options(MachineClass *mc)
 {
+    spapr_machine_2_2_class_options(mc);
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_1);
 }
 DEFINE_SPAPR_MACHINE(2_1, "2.1");
-- 
2.5.0

Re: [Qemu-devel] [PATCHv2 08/10] pseries: Restructure class_options functions

[Alexey Kardashevskiy]

On 12/07/2015 02:34 PM, David Gibson wrote:
> Currently each of the *_class_options() functions for the pseries-2.1 ..
> pseries-2.5 machine types are standalone.  This will become harder to
> maintain as new versions are added.
>
> This patch restructures them similarly to x86 where each function calls
> the one from the next version, then overrides anything necessary for
> compatibility with the specific version and older.
>
> The default behaviour - that for the most recent machine are set up in
> the base class initializer spapr_machine_class_init().  Previously it had
> some things set up to default to older behaviour with the more recent
> machines overriding it.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   hw/ppc/spapr.c | 21 +++++++++++++++++----
>   1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 4f645f3..5af3d13 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2266,6 +2266,12 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>       HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>
>       mc->desc = "pSeries Logical Partition (PAPR compliant)";
> +
> +    /*
> +     * We set up the default / latest behaviour here.  The class_init
> +     * functions for the specific versioned machine types can override
> +     * these details for backwards compatibility
> +     */
>       mc->init = ppc_spapr_init;
>       mc->reset = ppc_spapr_reset;
>       mc->block_default_type = IF_SCSI;
> @@ -2281,7 +2287,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>       hc->unplug = spapr_machine_device_unplug;
>       mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
>
> -    smc->dr_lmb_enabled = false;
> +    smc->dr_lmb_enabled = true;
>       fwc->get_dev_path = spapr_get_fw_dev_path;
>       nc->nmi_monitor_handler = spapr_nmi;
>   }
> @@ -2335,11 +2341,9 @@ static void spapr_machine_2_5_instance_options(MachineState *machine)
>
>   static void spapr_machine_2_5_class_options(MachineClass *mc)
>   {
> -    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> -
> +    /* Defaults for the latest behaviour inherited from the base class */
>       mc->alias = "pseries";
>       mc->is_default = 1;
> -    smc->dr_lmb_enabled = true;
>   }
>
>   DEFINE_SPAPR_MACHINE(2_5, "2.5");
> @@ -2357,6 +2361,12 @@ static void spapr_machine_2_4_instance_options(MachineState *machine)
>
>   static void spapr_machine_2_4_class_options(MachineClass *mc)
>   {
> +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> +
> +    spapr_machine_2_5_class_options(mc);
> +    mc->alias = NULL;
> +    mc->is_default = 0;
> +    smc->dr_lmb_enabled = false;
>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_4);
>   }
>
> @@ -2383,6 +2393,7 @@ static void spapr_machine_2_3_instance_options(MachineState *machine)
>
>   static void spapr_machine_2_3_class_options(MachineClass *mc)
>   {
> +    spapr_machine_2_4_class_options(mc);
>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_3);
>   }
>   DEFINE_SPAPR_MACHINE(2_3, "2.3");
> @@ -2407,6 +2418,7 @@ static void spapr_machine_2_2_instance_options(MachineState *machine)
>
>   static void spapr_machine_2_2_class_options(MachineClass *mc)
>   {
> +    spapr_machine_2_3_class_options(mc);
>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_2);
>   }
>   DEFINE_SPAPR_MACHINE(2_2, "2.2");
> @@ -2425,6 +2437,7 @@ static void spapr_machine_2_1_instance_options(MachineState *machine)
>
>   static void spapr_machine_2_1_class_options(MachineClass *mc)
>   {
> +    spapr_machine_2_2_class_options(mc);
>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_1);
>   }
>   DEFINE_SPAPR_MACHINE(2_1, "2.1");
>

Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>	

-- 
Alexey

[Qemu-devel] [PATCHv2 09/10] pseries: Improve setting of default machine version

[David Gibson]

This tweaks the way the default machine version is controlled, so that
there will be a bit less churn when each new version is introduced.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5af3d13..8b8eb18 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2308,12 +2308,16 @@ static const TypeInfo spapr_machine_info = {
     },
 };
 
-#define DEFINE_SPAPR_MACHINE(suffix, verstr)                         \
+#define DEFINE_SPAPR_MACHINE(suffix, verstr, latest)                 \
     static void spapr_machine_##suffix##_class_init(ObjectClass *oc, \
                                                     void *data)      \
     {                                                                \
         MachineClass *mc = MACHINE_CLASS(oc);                        \
         spapr_machine_##suffix##_class_options(mc);                  \
+        if (latest) {                                                \
+            mc->alias = "pseries";                                   \
+            mc->is_default = 1;                                      \
+        }                                                            \
     }                                                                \
     static void spapr_machine_##suffix##_instance_init(Object *obj)  \
     {                                                                \
@@ -2342,11 +2346,9 @@ static void spapr_machine_2_5_instance_options(MachineState *machine)
 static void spapr_machine_2_5_class_options(MachineClass *mc)
 {
     /* Defaults for the latest behaviour inherited from the base class */
-    mc->alias = "pseries";
-    mc->is_default = 1;
 }
 
-DEFINE_SPAPR_MACHINE(2_5, "2.5");
+DEFINE_SPAPR_MACHINE(2_5, "2.5", true);
 
 /*
  * pseries-2.4
@@ -2364,13 +2366,11 @@ static void spapr_machine_2_4_class_options(MachineClass *mc)
     sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
 
     spapr_machine_2_5_class_options(mc);
-    mc->alias = NULL;
-    mc->is_default = 0;
     smc->dr_lmb_enabled = false;
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_4);
 }
 
-DEFINE_SPAPR_MACHINE(2_4, "2.4");
+DEFINE_SPAPR_MACHINE(2_4, "2.4", false);
 
 /*
  * pseries-2.3
@@ -2396,7 +2396,7 @@ static void spapr_machine_2_3_class_options(MachineClass *mc)
     spapr_machine_2_4_class_options(mc);
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_3);
 }
-DEFINE_SPAPR_MACHINE(2_3, "2.3");
+DEFINE_SPAPR_MACHINE(2_3, "2.3", false);
 
 /*
  * pseries-2.2
@@ -2421,7 +2421,7 @@ static void spapr_machine_2_2_class_options(MachineClass *mc)
     spapr_machine_2_3_class_options(mc);
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_2);
 }
-DEFINE_SPAPR_MACHINE(2_2, "2.2");
+DEFINE_SPAPR_MACHINE(2_2, "2.2", false);
 
 /*
  * pseries-2.1
@@ -2440,7 +2440,7 @@ static void spapr_machine_2_1_class_options(MachineClass *mc)
     spapr_machine_2_2_class_options(mc);
     SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_1);
 }
-DEFINE_SPAPR_MACHINE(2_1, "2.1");
+DEFINE_SPAPR_MACHINE(2_1, "2.1", false);
 
 static void spapr_machine_register_types(void)
 {
-- 
2.5.0

Re: [Qemu-devel] [PATCHv2 09/10] pseries: Improve setting of default machine version

[Alexey Kardashevskiy]

On 12/07/2015 02:34 PM, David Gibson wrote:
> This tweaks the way the default machine version is controlled, so that
> there will be a bit less churn when each new version is introduced.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   hw/ppc/spapr.c | 20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5af3d13..8b8eb18 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2308,12 +2308,16 @@ static const TypeInfo spapr_machine_info = {
>       },
>   };
>
> -#define DEFINE_SPAPR_MACHINE(suffix, verstr)                         \
> +#define DEFINE_SPAPR_MACHINE(suffix, verstr, latest)                 \
>       static void spapr_machine_##suffix##_class_init(ObjectClass *oc, \
>                                                       void *data)      \
>       {                                                                \
>           MachineClass *mc = MACHINE_CLASS(oc);                        \
>           spapr_machine_##suffix##_class_options(mc);                  \
> +        if (latest) {                                                \
> +            mc->alias = "pseries";                                   \
> +            mc->is_default = 1;                                      \
> +        }                                                            \
>       }                                                                \
>       static void spapr_machine_##suffix##_instance_init(Object *obj)  \
>       {                                                                \
> @@ -2342,11 +2346,9 @@ static void spapr_machine_2_5_instance_options(MachineState *machine)
>   static void spapr_machine_2_5_class_options(MachineClass *mc)
>   {
>       /* Defaults for the latest behaviour inherited from the base class */
> -    mc->alias = "pseries";
> -    mc->is_default = 1;
>   }
>
> -DEFINE_SPAPR_MACHINE(2_5, "2.5");
> +DEFINE_SPAPR_MACHINE(2_5, "2.5", true);
>
>   /*
>    * pseries-2.4
> @@ -2364,13 +2366,11 @@ static void spapr_machine_2_4_class_options(MachineClass *mc)
>       sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
>
>       spapr_machine_2_5_class_options(mc);
> -    mc->alias = NULL;
> -    mc->is_default = 0;
>       smc->dr_lmb_enabled = false;
>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_4);
>   }
>
> -DEFINE_SPAPR_MACHINE(2_4, "2.4");
> +DEFINE_SPAPR_MACHINE(2_4, "2.4", false);
>
>   /*
>    * pseries-2.3
> @@ -2396,7 +2396,7 @@ static void spapr_machine_2_3_class_options(MachineClass *mc)
>       spapr_machine_2_4_class_options(mc);
>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_3);
>   }
> -DEFINE_SPAPR_MACHINE(2_3, "2.3");
> +DEFINE_SPAPR_MACHINE(2_3, "2.3", false);
>
>   /*
>    * pseries-2.2
> @@ -2421,7 +2421,7 @@ static void spapr_machine_2_2_class_options(MachineClass *mc)
>       spapr_machine_2_3_class_options(mc);
>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_2);
>   }
> -DEFINE_SPAPR_MACHINE(2_2, "2.2");
> +DEFINE_SPAPR_MACHINE(2_2, "2.2", false);
>
>   /*
>    * pseries-2.1
> @@ -2440,7 +2440,7 @@ static void spapr_machine_2_1_class_options(MachineClass *mc)
>       spapr_machine_2_2_class_options(mc);
>       SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_1);
>   }
> -DEFINE_SPAPR_MACHINE(2_1, "2.1");
> +DEFINE_SPAPR_MACHINE(2_1, "2.1", false);
>
>   static void spapr_machine_register_types(void)
>   {
>

Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>	

-- 
Alexey

[Qemu-devel] [PATCHv2 10/10] pseries: Add pseries-2.6 machine type

[David Gibson]

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 8b8eb18..2d57ab0 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2337,6 +2337,20 @@ static const TypeInfo spapr_machine_info = {
     machine_init(spapr_machine_register_##suffix)
 
 /*
+ * pseries-2.6
+ */
+static void spapr_machine_2_6_instance_options(MachineState *machine)
+{
+}
+
+static void spapr_machine_2_6_class_options(MachineClass *mc)
+{
+    /* Defaults for the latest behaviour inherited from the base class */
+}
+
+DEFINE_SPAPR_MACHINE(2_6, "2.6", true);
+
+/*
  * pseries-2.5
  */
 static void spapr_machine_2_5_instance_options(MachineState *machine)
@@ -2345,10 +2359,10 @@ static void spapr_machine_2_5_instance_options(MachineState *machine)
 
 static void spapr_machine_2_5_class_options(MachineClass *mc)
 {
-    /* Defaults for the latest behaviour inherited from the base class */
+    spapr_machine_2_6_class_options(mc);
 }
 
-DEFINE_SPAPR_MACHINE(2_5, "2.5", true);
+DEFINE_SPAPR_MACHINE(2_5, "2.5", false);
 
 /*
  * pseries-2.4
-- 
2.5.0

Re: [Qemu-devel] [PATCHv2 10/10] pseries: Add pseries-2.6 machine type

[Alexey Kardashevskiy]

On 12/07/2015 02:34 PM, David Gibson wrote:
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   hw/ppc/spapr.c | 18 ++++++++++++++++--
>   1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 8b8eb18..2d57ab0 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2337,6 +2337,20 @@ static const TypeInfo spapr_machine_info = {
>       machine_init(spapr_machine_register_##suffix)
>
>   /*
> + * pseries-2.6
> + */
> +static void spapr_machine_2_6_instance_options(MachineState *machine)
> +{
> +}
> +
> +static void spapr_machine_2_6_class_options(MachineClass *mc)
> +{
> +    /* Defaults for the latest behaviour inherited from the base class */
> +}
> +
> +DEFINE_SPAPR_MACHINE(2_6, "2.6", true);
> +
> +/*
>    * pseries-2.5
>    */
>   static void spapr_machine_2_5_instance_options(MachineState *machine)
> @@ -2345,10 +2359,10 @@ static void spapr_machine_2_5_instance_options(MachineState *machine)
>
>   static void spapr_machine_2_5_class_options(MachineClass *mc)
>   {
> -    /* Defaults for the latest behaviour inherited from the base class */
> +    spapr_machine_2_6_class_options(mc);
>   }
>
> -DEFINE_SPAPR_MACHINE(2_5, "2.5", true);
> +DEFINE_SPAPR_MACHINE(2_5, "2.5", false);
>
>   /*
>    * pseries-2.4
>

Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>	

-- 
Alexey

Re: [Qemu-devel] [PATCHv2 00/10] Clean up pseries machine versioning

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1120 bytes --]

On Mon, Dec 07, 2015 at 02:34:30PM +1100, David Gibson wrote:
> Thomas Huth recently posted a patches to switch the default USB
> controller for the pseries machine type to xHCI, which necessitated
> creating a pseries-2.6 machine type version.
> 
> That raised a bunch of comments suggesting better ways we could manage
> the machine versioning for pseries.
> 
> This is an attempt to implement those suggestions.  For the most part
> it makes the handling of machine versions for pseries more similar to
> the handling of PC machine versions.
> 
> Eduardo,
>   - Any opinions on how 5/10 (the only patch which isn't spapr
>     specific) should be merged?
> 
> Otherwise I'm hoping to get some ACKs from outside Red Hat, at which
> point I'll merge these into my ppc-for-2.6 tree, to be pushed upstream
> once the 2.6 tree opens.

Alexey, thanks for the reviews, I've now merged these to ppc-for-2.6.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]