[Qemu-devel] [PATCH 0/5] create the mach-virt 2.7 machine type

[Qemu-devel] [PATCH 0/5] create the mach-virt 2.7 machine type

[Andrew Jones]

This is the first new machine type mach-virt has received
(2.6 being the first versioned machine type), so we need to
do a bit more than the average "new machine type" patch.
The first four patches prepare for easy-adding of machine
types. The last patch adds the 2.7 type.


Andrew Jones (5):
  hw/arm/virt: separate versioned type-init code
  hw/arm/virt: set is_default
  hw/arm/virt: introduce DEFINE_VIRT_MACHINE
  hw/arm/virt: introduce DEFINE_VIRT_MACHINE_AS_LATEST
  hw/arm/virt: create the 2.7 machine type

 hw/arm/virt.c | 66 ++++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 50 insertions(+), 16 deletions(-)

-- 
2.4.11

[Qemu-devel] [PATCH 1/5] hw/arm/virt: separate versioned type-init code

[Andrew Jones]

Rename machvirt_info (which is specifically for 2.6 TypeInfo)
to machvirt_2_6_info, and separate the type registration of the
abstract machine type from the versioned type.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 8e46137e9be71..a0d35d0826590 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1387,6 +1387,12 @@ static const TypeInfo virt_machine_info = {
     .class_init    = virt_machine_class_init,
 };
 
+static void machvirt_machine_init(void)
+{
+    type_register_static(&virt_machine_info);
+}
+type_init(machvirt_machine_init);
+
 static void virt_2_6_instance_init(Object *obj)
 {
     VirtMachineState *vms = VIRT_MACHINE(obj);
@@ -1428,17 +1434,15 @@ static void virt_2_6_class_init(ObjectClass *oc, void *data)
     mc->alias = "virt";
 }
 
-static const TypeInfo machvirt_info = {
+static const TypeInfo machvirt_2_6_info = {
     .name = MACHINE_TYPE_NAME("virt-2.6"),
     .parent = TYPE_VIRT_MACHINE,
     .instance_init = virt_2_6_instance_init,
     .class_init = virt_2_6_class_init,
 };
 
-static void machvirt_machine_init(void)
+static void machvirt_machine_2_6_init(void)
 {
-    type_register_static(&virt_machine_info);
-    type_register_static(&machvirt_info);
+    type_register_static(&machvirt_2_6_info);
 }
-
-type_init(machvirt_machine_init);
+type_init(machvirt_machine_2_6_init);
-- 
2.4.11

[Qemu-devel] [PATCH 2/5] hw/arm/virt: set is_default

[Andrew Jones]

Make the latest machine type (currently only one) the
default.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a0d35d0826590..09afbafde025d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1432,6 +1432,7 @@ static void virt_2_6_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "QEMU 2.6 ARM Virtual Machine";
     mc->alias = "virt";
+    mc->is_default = 1;
 }
 
 static const TypeInfo machvirt_2_6_info = {
-- 
2.4.11

[Qemu-devel] [PATCH 3/5] hw/arm/virt: introduce DEFINE_VIRT_MACHINE

[Andrew Jones]

Use DEFINE_VIRT_MACHINE to generate versioned machine type info.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt.c | 40 +++++++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 09afbafde025d..9a3289d2c422c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -98,6 +98,27 @@ typedef struct {
 #define VIRT_MACHINE_CLASS(klass) \
     OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
 
+
+#define DEFINE_VIRT_MACHINE(major, minor) \
+    static void virt_##major##_##minor##_class_init(ObjectClass *oc, void *data) \
+    { \
+        MachineClass *mc = MACHINE_CLASS(oc); \
+        virt_machine_##major##_##minor##_options(mc); \
+        mc->desc = "QEMU " # major "." # minor " ARM Virtual Machine"; \
+    } \
+    static const TypeInfo machvirt_##major##_##minor##_info = { \
+        .name = MACHINE_TYPE_NAME("virt-" # major "." # minor), \
+        .parent = TYPE_VIRT_MACHINE, \
+        .instance_init = virt_##major##_##minor##_instance_init, \
+        .class_init = virt_##major##_##minor##_class_init, \
+    }; \
+    static void machvirt_machine_##major##_##minor##_init(void) \
+    { \
+        type_register_static(&machvirt_##major##_##minor##_info); \
+    } \
+    type_init(machvirt_machine_##major##_##minor##_init);
+
+
 /* RAM limit in GB. Since VIRT_MEM starts at the 1GB mark, this means
  * RAM can go up to the 256GB mark, leaving 256GB of the physical
  * address space unallocated and free for future use between 256G and 512G.
@@ -1426,24 +1447,9 @@ static void virt_2_6_instance_init(Object *obj)
                                     "Valid values are 2, 3 and host", NULL);
 }
 
-static void virt_2_6_class_init(ObjectClass *oc, void *data)
+static void virt_machine_2_6_options(MachineClass *mc)
 {
-    MachineClass *mc = MACHINE_CLASS(oc);
-
-    mc->desc = "QEMU 2.6 ARM Virtual Machine";
     mc->alias = "virt";
     mc->is_default = 1;
 }
-
-static const TypeInfo machvirt_2_6_info = {
-    .name = MACHINE_TYPE_NAME("virt-2.6"),
-    .parent = TYPE_VIRT_MACHINE,
-    .instance_init = virt_2_6_instance_init,
-    .class_init = virt_2_6_class_init,
-};
-
-static void machvirt_machine_2_6_init(void)
-{
-    type_register_static(&machvirt_2_6_info);
-}
-type_init(machvirt_machine_2_6_init);
+DEFINE_VIRT_MACHINE(2, 6)
-- 
2.4.11

[Qemu-devel] [PATCH 4/5] hw/arm/virt: introduce DEFINE_VIRT_MACHINE_AS_LATEST

[Andrew Jones]

Create two variants of DEFINE_VIRT_MACHINE. One, just called
DEFINE_VIRT_MACHINE, that does not set properties that only
the latest machine type should have, and another that does.
This will hopefully reduce potential for errors when adding
new versions.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 9a3289d2c422c..017c244a46f41 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -99,12 +99,16 @@ typedef struct {
     OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
 
 
-#define DEFINE_VIRT_MACHINE(major, minor) \
+#define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
     static void virt_##major##_##minor##_class_init(ObjectClass *oc, void *data) \
     { \
         MachineClass *mc = MACHINE_CLASS(oc); \
         virt_machine_##major##_##minor##_options(mc); \
         mc->desc = "QEMU " # major "." # minor " ARM Virtual Machine"; \
+        if (latest) { \
+            mc->alias = "virt"; \
+            mc->is_default = 1; \
+        } \
     } \
     static const TypeInfo machvirt_##major##_##minor##_info = { \
         .name = MACHINE_TYPE_NAME("virt-" # major "." # minor), \
@@ -118,6 +122,11 @@ typedef struct {
     } \
     type_init(machvirt_machine_##major##_##minor##_init);
 
+#define DEFINE_VIRT_MACHINE_AS_LATEST(major, minor) \
+    DEFINE_VIRT_MACHINE_LATEST(major, minor, true)
+#define DEFINE_VIRT_MACHINE(major, minor) \
+    DEFINE_VIRT_MACHINE_LATEST(major, minor, false)
+
 
 /* RAM limit in GB. Since VIRT_MEM starts at the 1GB mark, this means
  * RAM can go up to the 256GB mark, leaving 256GB of the physical
@@ -1449,7 +1458,5 @@ static void virt_2_6_instance_init(Object *obj)
 
 static void virt_machine_2_6_options(MachineClass *mc)
 {
-    mc->alias = "virt";
-    mc->is_default = 1;
 }
-DEFINE_VIRT_MACHINE(2, 6)
+DEFINE_VIRT_MACHINE_AS_LATEST(2, 6)
-- 
2.4.11

[Qemu-devel] [PATCH 5/5] hw/arm/virt: create the 2.7 machine type

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/arm/virt.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 017c244a46f41..323ffd4689641 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -42,6 +42,7 @@
 #include "sysemu/sysemu.h"
 #include "sysemu/kvm.h"
 #include "hw/boards.h"
+#include "hw/compat.h"
 #include "hw/loader.h"
 #include "exec/address-spaces.h"
 #include "qemu/bitops.h"
@@ -1423,7 +1424,7 @@ static void machvirt_machine_init(void)
 }
 type_init(machvirt_machine_init);
 
-static void virt_2_6_instance_init(Object *obj)
+static void virt_2_7_instance_init(Object *obj)
 {
     VirtMachineState *vms = VIRT_MACHINE(obj);
 
@@ -1456,7 +1457,22 @@ static void virt_2_6_instance_init(Object *obj)
                                     "Valid values are 2, 3 and host", NULL);
 }
 
+static void virt_machine_2_7_options(MachineClass *mc)
+{
+}
+DEFINE_VIRT_MACHINE_AS_LATEST(2, 7)
+
+#define VIRT_COMPAT_2_6 \
+    HW_COMPAT_2_6
+
+static void virt_2_6_instance_init(Object *obj)
+{
+    virt_2_7_instance_init(obj);
+}
+
 static void virt_machine_2_6_options(MachineClass *mc)
 {
+    virt_machine_2_7_options(mc);
+    SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_6);
 }
-DEFINE_VIRT_MACHINE_AS_LATEST(2, 6)
+DEFINE_VIRT_MACHINE(2, 6)
-- 
2.4.11

Re: [Qemu-devel] [PATCH 5/5] hw/arm/virt: create the 2.7 machine type

[Alex Bennée]

Andrew Jones <drjones@redhat.com> writes:

> Signed-off-by: Andrew Jones <drjones@redhat.com>


I'm confused. These steps have introduced a bunch of boiler plate to set
the default virt machine type so we can introduce 2.7 which is an alias
of the existing 2.6? Wouldn't this be better of waiting until there is a
concrete difference or have I missed something subtle here?

> ---
>  hw/arm/virt.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 017c244a46f41..323ffd4689641 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -42,6 +42,7 @@
>  #include "sysemu/sysemu.h"
>  #include "sysemu/kvm.h"
>  #include "hw/boards.h"
> +#include "hw/compat.h"
>  #include "hw/loader.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/bitops.h"
> @@ -1423,7 +1424,7 @@ static void machvirt_machine_init(void)
>  }
>  type_init(machvirt_machine_init);
>
> -static void virt_2_6_instance_init(Object *obj)
> +static void virt_2_7_instance_init(Object *obj)
>  {
>      VirtMachineState *vms = VIRT_MACHINE(obj);
>
> @@ -1456,7 +1457,22 @@ static void virt_2_6_instance_init(Object *obj)
>                                      "Valid values are 2, 3 and host", NULL);
>  }
>
> +static void virt_machine_2_7_options(MachineClass *mc)
> +{
> +}
> +DEFINE_VIRT_MACHINE_AS_LATEST(2, 7)
> +
> +#define VIRT_COMPAT_2_6 \
> +    HW_COMPAT_2_6
> +
> +static void virt_2_6_instance_init(Object *obj)
> +{
> +    virt_2_7_instance_init(obj);
> +}
> +
>  static void virt_machine_2_6_options(MachineClass *mc)
>  {
> +    virt_machine_2_7_options(mc);
> +    SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_6);
>  }
> -DEFINE_VIRT_MACHINE_AS_LATEST(2, 6)
> +DEFINE_VIRT_MACHINE(2, 6)


--
Alex Bennée

Re: [Qemu-devel] [PATCH 5/5] hw/arm/virt: create the 2.7 machine type

[Andrew Jones]

On Thu, Jun 09, 2016 at 05:06:43PM +0100, Alex Bennée wrote:
> 
> Andrew Jones <drjones@redhat.com> writes:
> 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> 
> 
> I'm confused. These steps have introduced a bunch of boiler plate to set
> the default virt machine type so we can introduce 2.7 which is an alias
> of the existing 2.6? Wouldn't this be better of waiting until there is a
> concrete difference or have I missed something subtle here?

The concrete difference is the minor number; 7 != 6.

Take a look at how hw/i386/pc and hw/ppc/spapr do this.
I basically just stole from them.

Now, on differences (besides the version); I'm trying to decide
what warrants versioning. There's the obvious migrateable state,
but what about changes to DT/ACPI? We've added NUMA info to the
hardware description for 2.7, but there's no easy way to version
that with the way we currently do it (like this series). Thoughts
on that?

Thanks,
drew

> 
> > ---
> >  hw/arm/virt.c | 20 ++++++++++++++++++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 017c244a46f41..323ffd4689641 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -42,6 +42,7 @@
> >  #include "sysemu/sysemu.h"
> >  #include "sysemu/kvm.h"
> >  #include "hw/boards.h"
> > +#include "hw/compat.h"
> >  #include "hw/loader.h"
> >  #include "exec/address-spaces.h"
> >  #include "qemu/bitops.h"
> > @@ -1423,7 +1424,7 @@ static void machvirt_machine_init(void)
> >  }
> >  type_init(machvirt_machine_init);
> >
> > -static void virt_2_6_instance_init(Object *obj)
> > +static void virt_2_7_instance_init(Object *obj)
> >  {
> >      VirtMachineState *vms = VIRT_MACHINE(obj);
> >
> > @@ -1456,7 +1457,22 @@ static void virt_2_6_instance_init(Object *obj)
> >                                      "Valid values are 2, 3 and host", NULL);
> >  }
> >
> > +static void virt_machine_2_7_options(MachineClass *mc)
> > +{
> > +}
> > +DEFINE_VIRT_MACHINE_AS_LATEST(2, 7)
> > +
> > +#define VIRT_COMPAT_2_6 \
> > +    HW_COMPAT_2_6
> > +
> > +static void virt_2_6_instance_init(Object *obj)
> > +{
> > +    virt_2_7_instance_init(obj);
> > +}
> > +
> >  static void virt_machine_2_6_options(MachineClass *mc)
> >  {
> > +    virt_machine_2_7_options(mc);
> > +    SET_MACHINE_COMPAT(mc, VIRT_COMPAT_2_6);
> >  }
> > -DEFINE_VIRT_MACHINE_AS_LATEST(2, 6)
> > +DEFINE_VIRT_MACHINE(2, 6)
> 
> 
> --
> Alex Bennée

Re: [Qemu-devel] [PATCH 2/5] hw/arm/virt: set is_default

[Peter Maydell]

On 8 June 2016 at 19:24, Andrew Jones <drjones@redhat.com> wrote:
> Make the latest machine type (currently only one) the
> default.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  hw/arm/virt.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index a0d35d0826590..09afbafde025d 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1432,6 +1432,7 @@ static void virt_2_6_class_init(ObjectClass *oc, void *data)
>
>      mc->desc = "QEMU 2.6 ARM Virtual Machine";
>      mc->alias = "virt";
> +    mc->is_default = 1;
>  }
>
>  static const TypeInfo machvirt_2_6_info = {

We don't want a default machine for 32-bit ARM, and I'm still
unconvinced we want a default machine for 64-bit ARM. At least,
this should be a separate patch/discussion from adding the
virt-2.7 machine type.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/5] hw/arm/virt: separate versioned type-init code

[Peter Maydell]

On 8 June 2016 at 19:24, Andrew Jones <drjones@redhat.com> wrote:
> Rename machvirt_info (which is specifically for 2.6 TypeInfo)
> to machvirt_2_6_info, and separate the type registration of the
> abstract machine type from the versioned type.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/5] hw/arm/virt: introduce DEFINE_VIRT_MACHINE

[Peter Maydell]

On 8 June 2016 at 19:24, Andrew Jones <drjones@redhat.com> wrote:
> Use DEFINE_VIRT_MACHINE to generate versioned machine type info.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  hw/arm/virt.c | 40 +++++++++++++++++++++++-----------------
>  1 file changed, 23 insertions(+), 17 deletions(-)

Checkpatch complains about a long line; otherwise

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [Qemu-devel] [PATCH 4/5] hw/arm/virt: introduce DEFINE_VIRT_MACHINE_AS_LATEST

[Peter Maydell]

On 8 June 2016 at 19:24, Andrew Jones <drjones@redhat.com> wrote:
> Create two variants of DEFINE_VIRT_MACHINE. One, just called
> DEFINE_VIRT_MACHINE, that does not set properties that only
> the latest machine type should have, and another that does.
> This will hopefully reduce potential for errors when adding
> new versions.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  hw/arm/virt.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 9a3289d2c422c..017c244a46f41 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -99,12 +99,16 @@ typedef struct {
>      OBJECT_CLASS_CHECK(VirtMachineClass, klass, TYPE_VIRT_MACHINE)
>
>
> -#define DEFINE_VIRT_MACHINE(major, minor) \
> +#define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>      static void virt_##major##_##minor##_class_init(ObjectClass *oc, void *data) \
>      { \
>          MachineClass *mc = MACHINE_CLASS(oc); \
>          virt_machine_##major##_##minor##_options(mc); \
>          mc->desc = "QEMU " # major "." # minor " ARM Virtual Machine"; \
> +        if (latest) { \
> +            mc->alias = "virt"; \
> +            mc->is_default = 1; \
> +        } \

See previous patch comments about not wanting a default machine.

thanks
-- PMM