All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
@ 2014-05-23 16:07 ` Stefano Stabellini
  0 siblings, 0 replies; 31+ messages in thread
From: Stefano Stabellini @ 2014-05-23 16:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Ian Campbell, Stefano Stabellini, Ian Jackson, qemu-devel,
	Paul Durrant, Anthony.Perard, tiejun.chen

Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
sure what is the machine that we are emulating.

Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
xen-platform device if requested. Choose slot 2 for the xen-platform
device for compatibility with current installations. In case of Intel
graphic passthrough, slot 2 might be needed by the grafic card. However
now that we can specify the slot explicitly, it is easy to change the
position of the xen-platform device on the PCI bus if graphic
passthrough is enabled.

Move the machine options earlier, before any other emulated devices
options. Otherwise the selected PCI slot for the xen-platform device is
not available in QEMU.

Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
differently from xenfv, the other QEMU machines do not have that option
off by default.

This patch does not change the emulated environment in the guest.

Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 8abed7b..fef684f 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -476,6 +476,29 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
         flexarray_vappend(dm_args, "-k", keymap, NULL);
     }
 
+    flexarray_append(dm_args, "-machine");
+    switch (b_info->type) {
+    case LIBXL_DOMAIN_TYPE_PV:
+        flexarray_append(dm_args, "xenpv");
+        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
+            flexarray_append(dm_args, b_info->extra_pv[i]);
+        break;
+    case LIBXL_DOMAIN_TYPE_HVM:
+        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
+        flexarray_append(dm_args, "-global");
+        flexarray_append(dm_args, "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
+        if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
+            flexarray_append(dm_args, "-device");
+            flexarray_append(dm_args, "xen-platform,addr=0x2");
+        }
+        for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
+            flexarray_append(dm_args, b_info->extra_hvm[i]);
+        break;
+    default:
+        abort();
+    }
+
+
     if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
         int ioemu_nics = 0;
 
@@ -645,29 +668,6 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
     for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++)
         flexarray_append(dm_args, b_info->extra[i]);
 
-    flexarray_append(dm_args, "-machine");
-    switch (b_info->type) {
-    case LIBXL_DOMAIN_TYPE_PV:
-        flexarray_append(dm_args, "xenpv");
-        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
-            flexarray_append(dm_args, b_info->extra_pv[i]);
-        break;
-    case LIBXL_DOMAIN_TYPE_HVM:
-        if (!libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
-            /* Switching here to the machine "pc" which does not add
-             * the xen-platform device instead of the default "xenfv" machine.
-             */
-            flexarray_append(dm_args, "pc,accel=xen");
-        } else {
-            flexarray_append(dm_args, "xenfv");
-        }
-        for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
-            flexarray_append(dm_args, b_info->extra_hvm[i]);
-        break;
-    default:
-        abort();
-    }
-
     ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb);
     flexarray_append(dm_args, "-m");
     flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size));

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
@ 2014-05-23 16:07 ` Stefano Stabellini
  0 siblings, 0 replies; 31+ messages in thread
From: Stefano Stabellini @ 2014-05-23 16:07 UTC (permalink / raw)
  To: xen-devel
  Cc: Ian Campbell, Stefano Stabellini, Ian Jackson, qemu-devel,
	Paul Durrant, Anthony.Perard, tiejun.chen

Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
sure what is the machine that we are emulating.

Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
xen-platform device if requested. Choose slot 2 for the xen-platform
device for compatibility with current installations. In case of Intel
graphic passthrough, slot 2 might be needed by the grafic card. However
now that we can specify the slot explicitly, it is easy to change the
position of the xen-platform device on the PCI bus if graphic
passthrough is enabled.

Move the machine options earlier, before any other emulated devices
options. Otherwise the selected PCI slot for the xen-platform device is
not available in QEMU.

Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
differently from xenfv, the other QEMU machines do not have that option
off by default.

This patch does not change the emulated environment in the guest.

Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 8abed7b..fef684f 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -476,6 +476,29 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
         flexarray_vappend(dm_args, "-k", keymap, NULL);
     }
 
+    flexarray_append(dm_args, "-machine");
+    switch (b_info->type) {
+    case LIBXL_DOMAIN_TYPE_PV:
+        flexarray_append(dm_args, "xenpv");
+        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
+            flexarray_append(dm_args, b_info->extra_pv[i]);
+        break;
+    case LIBXL_DOMAIN_TYPE_HVM:
+        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
+        flexarray_append(dm_args, "-global");
+        flexarray_append(dm_args, "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
+        if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
+            flexarray_append(dm_args, "-device");
+            flexarray_append(dm_args, "xen-platform,addr=0x2");
+        }
+        for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
+            flexarray_append(dm_args, b_info->extra_hvm[i]);
+        break;
+    default:
+        abort();
+    }
+
+
     if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
         int ioemu_nics = 0;
 
@@ -645,29 +668,6 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
     for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++)
         flexarray_append(dm_args, b_info->extra[i]);
 
-    flexarray_append(dm_args, "-machine");
-    switch (b_info->type) {
-    case LIBXL_DOMAIN_TYPE_PV:
-        flexarray_append(dm_args, "xenpv");
-        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
-            flexarray_append(dm_args, b_info->extra_pv[i]);
-        break;
-    case LIBXL_DOMAIN_TYPE_HVM:
-        if (!libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
-            /* Switching here to the machine "pc" which does not add
-             * the xen-platform device instead of the default "xenfv" machine.
-             */
-            flexarray_append(dm_args, "pc,accel=xen");
-        } else {
-            flexarray_append(dm_args, "xenfv");
-        }
-        for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
-            flexarray_append(dm_args, b_info->extra_hvm[i]);
-        break;
-    default:
-        abort();
-    }
-
     ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb);
     flexarray_append(dm_args, "-m");
     flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size));

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-05-23 16:07 ` Stefano Stabellini
@ 2014-05-23 17:08   ` Fabio Fantoni
  -1 siblings, 0 replies; 31+ messages in thread
From: Fabio Fantoni @ 2014-05-23 17:08 UTC (permalink / raw)
  To: Stefano Stabellini, xen-devel
  Cc: Ian Campbell, Ian Jackson, qemu-devel, Paul Durrant,
	Anthony.Perard, tiejun.chen


Il 23/05/2014 18:07, Stefano Stabellini ha scritto:
> Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
> sure what is the machine that we are emulating.
>
> Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> xen-platform device if requested. Choose slot 2 for the xen-platform
> device for compatibility with current installations. In case of Intel
> graphic passthrough, slot 2 might be needed by the grafic card. However
> now that we can specify the slot explicitly, it is easy to change the
> position of the xen-platform device on the PCI bus if graphic
> passthrough is enabled.
>
> Move the machine options earlier, before any other emulated devices
> options. Otherwise the selected PCI slot for the xen-platform device is
> not available in QEMU.
>
> Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> differently from xenfv, the other QEMU machines do not have that option
> off by default.
>
> This patch does not change the emulated environment in the guest.
>
> Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 8abed7b..fef684f 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -476,6 +476,29 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>           flexarray_vappend(dm_args, "-k", keymap, NULL);
>       }
>   
> +    flexarray_append(dm_args, "-machine");
> +    switch (b_info->type) {
> +    case LIBXL_DOMAIN_TYPE_PV:
> +        flexarray_append(dm_args, "xenpv");
> +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
> +            flexarray_append(dm_args, b_info->extra_pv[i]);
> +        break;
> +    case LIBXL_DOMAIN_TYPE_HVM:
> +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> +        flexarray_append(dm_args, "-global");
> +        flexarray_append(dm_args, "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");

I think is good add a comment for remember to remove this workaround 
when pc >=2.1 will be the default since qemu 2.1 will fix it.
https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04789.html

> +        if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
> +            flexarray_append(dm_args, "-device");
> +            flexarray_append(dm_args, "xen-platform,addr=0x2");

The fixed pci address to 0x2 probably is a problem with intel gpu 
passthrough:
http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03726.html

> +        }
> +        for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
> +            flexarray_append(dm_args, b_info->extra_hvm[i]);
> +        break;
> +    default:
> +        abort();
> +    }
> +
> +
>       if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
>           int ioemu_nics = 0;
>   
> @@ -645,29 +668,6 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>       for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++)
>           flexarray_append(dm_args, b_info->extra[i]);
>   
> -    flexarray_append(dm_args, "-machine");
> -    switch (b_info->type) {
> -    case LIBXL_DOMAIN_TYPE_PV:
> -        flexarray_append(dm_args, "xenpv");
> -        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
> -            flexarray_append(dm_args, b_info->extra_pv[i]);
> -        break;
> -    case LIBXL_DOMAIN_TYPE_HVM:
> -        if (!libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
> -            /* Switching here to the machine "pc" which does not add
> -             * the xen-platform device instead of the default "xenfv" machine.
> -             */
> -            flexarray_append(dm_args, "pc,accel=xen");
> -        } else {
> -            flexarray_append(dm_args, "xenfv");
> -        }
> -        for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
> -            flexarray_append(dm_args, b_info->extra_hvm[i]);
> -        break;
> -    default:
> -        abort();
> -    }
> -
>       ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb);
>       flexarray_append(dm_args, "-m");
>       flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size));
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
@ 2014-05-23 17:08   ` Fabio Fantoni
  0 siblings, 0 replies; 31+ messages in thread
From: Fabio Fantoni @ 2014-05-23 17:08 UTC (permalink / raw)
  To: Stefano Stabellini, xen-devel
  Cc: Ian Campbell, Ian Jackson, qemu-devel, Paul Durrant,
	Anthony.Perard, tiejun.chen


Il 23/05/2014 18:07, Stefano Stabellini ha scritto:
> Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
> sure what is the machine that we are emulating.
>
> Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> xen-platform device if requested. Choose slot 2 for the xen-platform
> device for compatibility with current installations. In case of Intel
> graphic passthrough, slot 2 might be needed by the grafic card. However
> now that we can specify the slot explicitly, it is easy to change the
> position of the xen-platform device on the PCI bus if graphic
> passthrough is enabled.
>
> Move the machine options earlier, before any other emulated devices
> options. Otherwise the selected PCI slot for the xen-platform device is
> not available in QEMU.
>
> Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> differently from xenfv, the other QEMU machines do not have that option
> off by default.
>
> This patch does not change the emulated environment in the guest.
>
> Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 8abed7b..fef684f 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -476,6 +476,29 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>           flexarray_vappend(dm_args, "-k", keymap, NULL);
>       }
>   
> +    flexarray_append(dm_args, "-machine");
> +    switch (b_info->type) {
> +    case LIBXL_DOMAIN_TYPE_PV:
> +        flexarray_append(dm_args, "xenpv");
> +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
> +            flexarray_append(dm_args, b_info->extra_pv[i]);
> +        break;
> +    case LIBXL_DOMAIN_TYPE_HVM:
> +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> +        flexarray_append(dm_args, "-global");
> +        flexarray_append(dm_args, "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");

I think is good add a comment for remember to remove this workaround 
when pc >=2.1 will be the default since qemu 2.1 will fix it.
https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04789.html

> +        if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
> +            flexarray_append(dm_args, "-device");
> +            flexarray_append(dm_args, "xen-platform,addr=0x2");

The fixed pci address to 0x2 probably is a problem with intel gpu 
passthrough:
http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03726.html

> +        }
> +        for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
> +            flexarray_append(dm_args, b_info->extra_hvm[i]);
> +        break;
> +    default:
> +        abort();
> +    }
> +
> +
>       if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
>           int ioemu_nics = 0;
>   
> @@ -645,29 +668,6 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>       for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++)
>           flexarray_append(dm_args, b_info->extra[i]);
>   
> -    flexarray_append(dm_args, "-machine");
> -    switch (b_info->type) {
> -    case LIBXL_DOMAIN_TYPE_PV:
> -        flexarray_append(dm_args, "xenpv");
> -        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
> -            flexarray_append(dm_args, b_info->extra_pv[i]);
> -        break;
> -    case LIBXL_DOMAIN_TYPE_HVM:
> -        if (!libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
> -            /* Switching here to the machine "pc" which does not add
> -             * the xen-platform device instead of the default "xenfv" machine.
> -             */
> -            flexarray_append(dm_args, "pc,accel=xen");
> -        } else {
> -            flexarray_append(dm_args, "xenfv");
> -        }
> -        for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++)
> -            flexarray_append(dm_args, b_info->extra_hvm[i]);
> -        break;
> -    default:
> -        abort();
> -    }
> -
>       ram_size = libxl__sizekb_to_mb(b_info->max_memkb - b_info->video_memkb);
>       flexarray_append(dm_args, "-m");
>       flexarray_append(dm_args, libxl__sprintf(gc, "%"PRId64, ram_size));
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-05-23 17:08   ` Fabio Fantoni
@ 2014-05-25 14:14     ` Stefano Stabellini
  -1 siblings, 0 replies; 31+ messages in thread
From: Stefano Stabellini @ 2014-05-25 14:14 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: xen-devel, Ian Campbell, Stefano Stabellini, Ian Jackson,
	qemu-devel, Paul Durrant, Anthony.Perard, tiejun.chen

On Fri, 23 May 2014, Fabio Fantoni wrote:
> Il 23/05/2014 18:07, Stefano Stabellini ha scritto:
> > Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
> > sure what is the machine that we are emulating.
> > 
> > Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> > xen-platform device if requested. Choose slot 2 for the xen-platform
> > device for compatibility with current installations. In case of Intel
> > graphic passthrough, slot 2 might be needed by the grafic card. However
> > now that we can specify the slot explicitly, it is easy to change the
> > position of the xen-platform device on the PCI bus if graphic
> > passthrough is enabled.
> > 
> > Move the machine options earlier, before any other emulated devices
> > options. Otherwise the selected PCI slot for the xen-platform device is
> > not available in QEMU.
> > 
> > Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> > differently from xenfv, the other QEMU machines do not have that option
> > off by default.
> > 
> > This patch does not change the emulated environment in the guest.
> > 
> > Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 8abed7b..fef684f 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -476,6 +476,29 @@ static char **
> > libxl__build_device_model_args_new(libxl__gc *gc,
> >           flexarray_vappend(dm_args, "-k", keymap, NULL);
> >       }
> >   +    flexarray_append(dm_args, "-machine");
> > +    switch (b_info->type) {
> > +    case LIBXL_DOMAIN_TYPE_PV:
> > +        flexarray_append(dm_args, "xenpv");
> > +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
> > +            flexarray_append(dm_args, b_info->extra_pv[i]);
> > +        break;
> > +    case LIBXL_DOMAIN_TYPE_HVM:
> > +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> > +        flexarray_append(dm_args, "-global");
> > +        flexarray_append(dm_args,
> > "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
> 
> I think is good add a comment for remember to remove this workaround when pc
> >=2.1 will be the default since qemu 2.1 will fix it.
> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04789.html

The workaround is not actually harmful, it doesn't need to be removed
when pc >= 2.1 in QEMU.


> > +        if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
> > +            flexarray_append(dm_args, "-device");
> > +            flexarray_append(dm_args, "xen-platform,addr=0x2");
> 
> The fixed pci address to 0x2 probably is a problem with intel gpu passthrough:
> http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03726.html

Right, however we cannot really change the default position of the
xen-platform device on the PCI bus, otherwise we'll end up changing the
emulated environment for all the VMs out there.

I'll leave it to Tiejun to move xen-platform to another slot when
graphic passthrough is enabled.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
@ 2014-05-25 14:14     ` Stefano Stabellini
  0 siblings, 0 replies; 31+ messages in thread
From: Stefano Stabellini @ 2014-05-25 14:14 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: xen-devel, Ian Campbell, Stefano Stabellini, Ian Jackson,
	qemu-devel, Paul Durrant, Anthony.Perard, tiejun.chen

On Fri, 23 May 2014, Fabio Fantoni wrote:
> Il 23/05/2014 18:07, Stefano Stabellini ha scritto:
> > Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
> > sure what is the machine that we are emulating.
> > 
> > Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> > xen-platform device if requested. Choose slot 2 for the xen-platform
> > device for compatibility with current installations. In case of Intel
> > graphic passthrough, slot 2 might be needed by the grafic card. However
> > now that we can specify the slot explicitly, it is easy to change the
> > position of the xen-platform device on the PCI bus if graphic
> > passthrough is enabled.
> > 
> > Move the machine options earlier, before any other emulated devices
> > options. Otherwise the selected PCI slot for the xen-platform device is
> > not available in QEMU.
> > 
> > Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> > differently from xenfv, the other QEMU machines do not have that option
> > off by default.
> > 
> > This patch does not change the emulated environment in the guest.
> > 
> > Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 8abed7b..fef684f 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -476,6 +476,29 @@ static char **
> > libxl__build_device_model_args_new(libxl__gc *gc,
> >           flexarray_vappend(dm_args, "-k", keymap, NULL);
> >       }
> >   +    flexarray_append(dm_args, "-machine");
> > +    switch (b_info->type) {
> > +    case LIBXL_DOMAIN_TYPE_PV:
> > +        flexarray_append(dm_args, "xenpv");
> > +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
> > +            flexarray_append(dm_args, b_info->extra_pv[i]);
> > +        break;
> > +    case LIBXL_DOMAIN_TYPE_HVM:
> > +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> > +        flexarray_append(dm_args, "-global");
> > +        flexarray_append(dm_args,
> > "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
> 
> I think is good add a comment for remember to remove this workaround when pc
> >=2.1 will be the default since qemu 2.1 will fix it.
> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04789.html

The workaround is not actually harmful, it doesn't need to be removed
when pc >= 2.1 in QEMU.


> > +        if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
> > +            flexarray_append(dm_args, "-device");
> > +            flexarray_append(dm_args, "xen-platform,addr=0x2");
> 
> The fixed pci address to 0x2 probably is a problem with intel gpu passthrough:
> http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03726.html

Right, however we cannot really change the default position of the
xen-platform device on the PCI bus, otherwise we'll end up changing the
emulated environment for all the VMs out there.

I'll leave it to Tiejun to move xen-platform to another slot when
graphic passthrough is enabled.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-05-25 14:14     ` Stefano Stabellini
@ 2014-05-26  8:00       ` Fabio Fantoni
  -1 siblings, 0 replies; 31+ messages in thread
From: Fabio Fantoni @ 2014-05-26  8:00 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Ian Campbell, Ian Jackson, qemu-devel, Paul Durrant,
	Anthony.Perard, tiejun.chen

Il 25/05/2014 16:14, Stefano Stabellini ha scritto:
> On Fri, 23 May 2014, Fabio Fantoni wrote:
>> Il 23/05/2014 18:07, Stefano Stabellini ha scritto:
>>> Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
>>> sure what is the machine that we are emulating.
>>>
>>> Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
>>> xen-platform device if requested. Choose slot 2 for the xen-platform
>>> device for compatibility with current installations. In case of Intel
>>> graphic passthrough, slot 2 might be needed by the grafic card. However
>>> now that we can specify the slot explicitly, it is easy to change the
>>> position of the xen-platform device on the PCI bus if graphic
>>> passthrough is enabled.
>>>
>>> Move the machine options earlier, before any other emulated devices
>>> options. Otherwise the selected PCI slot for the xen-platform device is
>>> not available in QEMU.
>>>
>>> Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
>>> differently from xenfv, the other QEMU machines do not have that option
>>> off by default.
>>>
>>> This patch does not change the emulated environment in the guest.
>>>
>>> Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>
>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>> index 8abed7b..fef684f 100644
>>> --- a/tools/libxl/libxl_dm.c
>>> +++ b/tools/libxl/libxl_dm.c
>>> @@ -476,6 +476,29 @@ static char **
>>> libxl__build_device_model_args_new(libxl__gc *gc,
>>>            flexarray_vappend(dm_args, "-k", keymap, NULL);
>>>        }
>>>    +    flexarray_append(dm_args, "-machine");
>>> +    switch (b_info->type) {
>>> +    case LIBXL_DOMAIN_TYPE_PV:
>>> +        flexarray_append(dm_args, "xenpv");
>>> +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
>>> +            flexarray_append(dm_args, b_info->extra_pv[i]);
>>> +        break;
>>> +    case LIBXL_DOMAIN_TYPE_HVM:
>>> +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");

I think that a note in README should be added: qemu >=1.6.1 is required 
for hvm domUs.

About the other xl parameter to be added I think that should be:
qemu_machine_type="i440fx"|"q35" (when also q35 will be supported on xen)
qemu_machine_version="x.y" to specify the machine version, useful for 
debug/testing and other some cases.

>>> +        flexarray_append(dm_args, "-global");
>>> +        flexarray_append(dm_args,
>>> "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
>> I think is good add a comment for remember to remove this workaround when pc
>>> =2.1 will be the default since qemu 2.1 will fix it.
>> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04789.html
> The workaround is not actually harmful, it doesn't need to be removed
> when pc >= 2.1 in QEMU.
>
>
>>> +        if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
>>> +            flexarray_append(dm_args, "-device");
>>> +            flexarray_append(dm_args, "xen-platform,addr=0x2");
>> The fixed pci address to 0x2 probably is a problem with intel gpu passthrough:
>> http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03726.html
> Right, however we cannot really change the default position of the
> xen-platform device on the PCI bus, otherwise we'll end up changing the
> emulated environment for all the VMs out there.
>
> I'll leave it to Tiejun to move xen-platform to another slot when
> graphic passthrough is enabled.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
@ 2014-05-26  8:00       ` Fabio Fantoni
  0 siblings, 0 replies; 31+ messages in thread
From: Fabio Fantoni @ 2014-05-26  8:00 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Ian Campbell, Ian Jackson, qemu-devel, Paul Durrant,
	Anthony.Perard, tiejun.chen

Il 25/05/2014 16:14, Stefano Stabellini ha scritto:
> On Fri, 23 May 2014, Fabio Fantoni wrote:
>> Il 23/05/2014 18:07, Stefano Stabellini ha scritto:
>>> Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
>>> sure what is the machine that we are emulating.
>>>
>>> Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
>>> xen-platform device if requested. Choose slot 2 for the xen-platform
>>> device for compatibility with current installations. In case of Intel
>>> graphic passthrough, slot 2 might be needed by the grafic card. However
>>> now that we can specify the slot explicitly, it is easy to change the
>>> position of the xen-platform device on the PCI bus if graphic
>>> passthrough is enabled.
>>>
>>> Move the machine options earlier, before any other emulated devices
>>> options. Otherwise the selected PCI slot for the xen-platform device is
>>> not available in QEMU.
>>>
>>> Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
>>> differently from xenfv, the other QEMU machines do not have that option
>>> off by default.
>>>
>>> This patch does not change the emulated environment in the guest.
>>>
>>> Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2
>>>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>
>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>> index 8abed7b..fef684f 100644
>>> --- a/tools/libxl/libxl_dm.c
>>> +++ b/tools/libxl/libxl_dm.c
>>> @@ -476,6 +476,29 @@ static char **
>>> libxl__build_device_model_args_new(libxl__gc *gc,
>>>            flexarray_vappend(dm_args, "-k", keymap, NULL);
>>>        }
>>>    +    flexarray_append(dm_args, "-machine");
>>> +    switch (b_info->type) {
>>> +    case LIBXL_DOMAIN_TYPE_PV:
>>> +        flexarray_append(dm_args, "xenpv");
>>> +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
>>> +            flexarray_append(dm_args, b_info->extra_pv[i]);
>>> +        break;
>>> +    case LIBXL_DOMAIN_TYPE_HVM:
>>> +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");

I think that a note in README should be added: qemu >=1.6.1 is required 
for hvm domUs.

About the other xl parameter to be added I think that should be:
qemu_machine_type="i440fx"|"q35" (when also q35 will be supported on xen)
qemu_machine_version="x.y" to specify the machine version, useful for 
debug/testing and other some cases.

>>> +        flexarray_append(dm_args, "-global");
>>> +        flexarray_append(dm_args,
>>> "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
>> I think is good add a comment for remember to remove this workaround when pc
>>> =2.1 will be the default since qemu 2.1 will fix it.
>> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04789.html
> The workaround is not actually harmful, it doesn't need to be removed
> when pc >= 2.1 in QEMU.
>
>
>>> +        if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
>>> +            flexarray_append(dm_args, "-device");
>>> +            flexarray_append(dm_args, "xen-platform,addr=0x2");
>> The fixed pci address to 0x2 probably is a problem with intel gpu passthrough:
>> http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03726.html
> Right, however we cannot really change the default position of the
> xen-platform device on the PCI bus, otherwise we'll end up changing the
> emulated environment for all the VMs out there.
>
> I'll leave it to Tiejun to move xen-platform to another slot when
> graphic passthrough is enabled.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-05-26  8:00       ` Fabio Fantoni
  (?)
@ 2014-05-28 14:45       ` Fabio Fantoni
  2014-05-28 16:18         ` Stefano Stabellini
  -1 siblings, 1 reply; 31+ messages in thread
From: Fabio Fantoni @ 2014-05-28 14:45 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Ian Campbell, Ian Jackson, qemu-devel, Paul Durrant,
	Anthony.Perard, tiejun.chen

Il 26/05/2014 10:00, Fabio Fantoni ha scritto:
> Il 25/05/2014 16:14, Stefano Stabellini ha scritto:
>> On Fri, 23 May 2014, Fabio Fantoni wrote:
>>> Il 23/05/2014 18:07, Stefano Stabellini ha scritto:
>>>> Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
>>>> sure what is the machine that we are emulating.
>>>>
>>>> Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
>>>> xen-platform device if requested. Choose slot 2 for the xen-platform
>>>> device for compatibility with current installations. In case of Intel
>>>> graphic passthrough, slot 2 might be needed by the grafic card. 
>>>> However
>>>> now that we can specify the slot explicitly, it is easy to change the
>>>> position of the xen-platform device on the PCI bus if graphic
>>>> passthrough is enabled.
>>>>
>>>> Move the machine options earlier, before any other emulated devices
>>>> options. Otherwise the selected PCI slot for the xen-platform 
>>>> device is
>>>> not available in QEMU.
>>>>
>>>> Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
>>>> differently from xenfv, the other QEMU machines do not have that 
>>>> option
>>>> off by default.
>>>>
>>>> This patch does not change the emulated environment in the guest.
>>>>
>>>> Refer to this thread: 
>>>> http://marc.info/?l=xen-devel&m=140023775929625&w=2
>>>>
>>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>>>>
>>>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>>>> index 8abed7b..fef684f 100644
>>>> --- a/tools/libxl/libxl_dm.c
>>>> +++ b/tools/libxl/libxl_dm.c
>>>> @@ -476,6 +476,29 @@ static char **
>>>> libxl__build_device_model_args_new(libxl__gc *gc,
>>>>            flexarray_vappend(dm_args, "-k", keymap, NULL);
>>>>        }
>>>>    +    flexarray_append(dm_args, "-machine");
>>>> +    switch (b_info->type) {
>>>> +    case LIBXL_DOMAIN_TYPE_PV:
>>>> +        flexarray_append(dm_args, "xenpv");
>>>> +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != 
>>>> NULL; i++)
>>>> +            flexarray_append(dm_args, b_info->extra_pv[i]);
>>>> +        break;
>>>> +    case LIBXL_DOMAIN_TYPE_HVM:
>>>> +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
>
> I think that a note in README should be added: qemu >=1.6.1 is 
> required for hvm domUs.
>
> About the other xl parameter to be added I think that should be:
> qemu_machine_type="i440fx"|"q35" (when also q35 will be supported on xen)
> qemu_machine_version="x.y" to specify the machine version, useful for 
> debug/testing and other some cases.
>
>>>> +        flexarray_append(dm_args, "-global");
>>>> +        flexarray_append(dm_args,
>>>> "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
>>> I think is good add a comment for remember to remove this workaround 
>>> when pc
>>>> =2.1 will be the default since qemu 2.1 will fix it.
>>> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04789.html
>> The workaround is not actually harmful, it doesn't need to be removed
>> when pc >= 2.1 in QEMU.
>>
>>
>>>> +        if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
>>>> +            flexarray_append(dm_args, "-device");
>>>> +            flexarray_append(dm_args, "xen-platform,addr=0x2");
>>> The fixed pci address to 0x2 probably is a problem with intel gpu 
>>> passthrough:
>>> http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03726.html
>> Right, however we cannot really change the default position of the
>> xen-platform device on the PCI bus, otherwise we'll end up changing the
>> emulated environment for all the VMs out there.
>>
>> I'll leave it to Tiejun to move xen-platform to another slot when
>> graphic passthrough is enabled.
>

I found another case of problem with xen-platform's fixed pci slot.
I tested this patch and I saw that qemu not start also in other cases, 
for example the domU of my test:
> qemu-system-i386: -device xen-platform,addr=0x2: PCI: slot 2 function 
> 0 not available for xen-platform, in use by intel-hda
> qemu-system-i386: -device xen-platform,addr=0x2: Device initialization 
> failed.
> qemu-system-i386: -device xen-platform,addr=0x2: Device 'xen-platform' 
> could not be initialized

The domU's xl cfg:
> name='W7'
> builder="hvm"
> #device_model_override="/usr/lib/xen/bin/qemu-gdb"
> #device_model_override="/usr/bin/qemu-system-x86_64"
> #bios="ovmf"
> memory=2048
> vcpus=2
> #nestedhvm=1
> #vif=['model=e1000,bridge=xenbr0']
> vif=['bridge=xenbr0,mac=00:16:3e:42:ae:8f']
> disk=['/mnt/vm/disks/W7.disk1.xm,raw,hda,rw',',raw,hdb,ro,cdrom']
> boot='dc'
> device_model_version="qemu-xen"
> viridian=1
> vnc=0
> keymap="it"
> on_crash="destroy"
> vga="qxl"
> #videoram=64
> spice=1
> spicehost='0.0.0.0'
> spiceport=6002
> spicedisable_ticketing=1
> spicevdagent=1
> spice_clipboard_sharing=0
> spice_image_compression="off"
> spice_streaming_video="filter"
> spiceusbredirection=4
> soundhw="hda"
> localtime=1
> usbversion=2

Probably there are also other cases that can create a problem with 
xen-platform fixed address, FWIK now new usb controller (with 
usbversion) is the only other with fixed pci address in libxl, all other 
emulated qemu components not.
And call it before in qemu binary starts notaffect the pci slot order 
because the xen-platform is already before audio.

Thanks for any reply and sorry for my bad english.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-05-28 14:45       ` Fabio Fantoni
@ 2014-05-28 16:18         ` Stefano Stabellini
  2014-05-28 16:41           ` Stefano Stabellini
  0 siblings, 1 reply; 31+ messages in thread
From: Stefano Stabellini @ 2014-05-28 16:18 UTC (permalink / raw)
  To: Fabio Fantoni
  Cc: xen-devel, Ian Campbell, Stefano Stabellini, Ian Jackson,
	qemu-devel, Paul Durrant, Anthony.Perard, tiejun.chen

On Wed, 28 May 2014, Fabio Fantoni wrote:
> Il 26/05/2014 10:00, Fabio Fantoni ha scritto:
> > Il 25/05/2014 16:14, Stefano Stabellini ha scritto:
> > > On Fri, 23 May 2014, Fabio Fantoni wrote:
> > > > Il 23/05/2014 18:07, Stefano Stabellini ha scritto:
> > > > > Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
> > > > > sure what is the machine that we are emulating.
> > > > > 
> > > > > Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> > > > > xen-platform device if requested. Choose slot 2 for the xen-platform
> > > > > device for compatibility with current installations. In case of Intel
> > > > > graphic passthrough, slot 2 might be needed by the grafic card.
> > > > > However
> > > > > now that we can specify the slot explicitly, it is easy to change the
> > > > > position of the xen-platform device on the PCI bus if graphic
> > > > > passthrough is enabled.
> > > > > 
> > > > > Move the machine options earlier, before any other emulated devices
> > > > > options. Otherwise the selected PCI slot for the xen-platform device
> > > > > is
> > > > > not available in QEMU.
> > > > > 
> > > > > Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> > > > > differently from xenfv, the other QEMU machines do not have that
> > > > > option
> > > > > off by default.
> > > > > 
> > > > > This patch does not change the emulated environment in the guest.
> > > > > 
> > > > > Refer to this thread:
> > > > > http://marc.info/?l=xen-devel&m=140023775929625&w=2
> > > > > 
> > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > > > 
> > > > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > > > > index 8abed7b..fef684f 100644
> > > > > --- a/tools/libxl/libxl_dm.c
> > > > > +++ b/tools/libxl/libxl_dm.c
> > > > > @@ -476,6 +476,29 @@ static char **
> > > > > libxl__build_device_model_args_new(libxl__gc *gc,
> > > > >            flexarray_vappend(dm_args, "-k", keymap, NULL);
> > > > >        }
> > > > >    +    flexarray_append(dm_args, "-machine");
> > > > > +    switch (b_info->type) {
> > > > > +    case LIBXL_DOMAIN_TYPE_PV:
> > > > > +        flexarray_append(dm_args, "xenpv");
> > > > > +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL;
> > > > > i++)
> > > > > +            flexarray_append(dm_args, b_info->extra_pv[i]);
> > > > > +        break;
> > > > > +    case LIBXL_DOMAIN_TYPE_HVM:
> > > > > +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> > 
> > I think that a note in README should be added: qemu >=1.6.1 is required for
> > hvm domUs.
> > 
> > About the other xl parameter to be added I think that should be:
> > qemu_machine_type="i440fx"|"q35" (when also q35 will be supported on xen)
> > qemu_machine_version="x.y" to specify the machine version, useful for
> > debug/testing and other some cases.
> > 
> > > > > +        flexarray_append(dm_args, "-global");
> > > > > +        flexarray_append(dm_args,
> > > > > "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
> > > > I think is good add a comment for remember to remove this workaround
> > > > when pc
> > > > > =2.1 will be the default since qemu 2.1 will fix it.
> > > > https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04789.html
> > > The workaround is not actually harmful, it doesn't need to be removed
> > > when pc >= 2.1 in QEMU.
> > > 
> > > 
> > > > > +        if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
> > > > > +            flexarray_append(dm_args, "-device");
> > > > > +            flexarray_append(dm_args, "xen-platform,addr=0x2");
> > > > The fixed pci address to 0x2 probably is a problem with intel gpu
> > > > passthrough:
> > > > http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03726.html
> > > Right, however we cannot really change the default position of the
> > > xen-platform device on the PCI bus, otherwise we'll end up changing the
> > > emulated environment for all the VMs out there.
> > > 
> > > I'll leave it to Tiejun to move xen-platform to another slot when
> > > graphic passthrough is enabled.
> > 
> 
> I found another case of problem with xen-platform's fixed pci slot.
> I tested this patch and I saw that qemu not start also in other cases, for
> example the domU of my test:
> > qemu-system-i386: -device xen-platform,addr=0x2: PCI: slot 2 function 0 not
> > available for xen-platform, in use by intel-hda
> > qemu-system-i386: -device xen-platform,addr=0x2: Device initialization
> > failed.
> > qemu-system-i386: -device xen-platform,addr=0x2: Device 'xen-platform' could
> > not be initialized
> 
> The domU's xl cfg:
> > name='W7'
> > builder="hvm"
> > #device_model_override="/usr/lib/xen/bin/qemu-gdb"
> > #device_model_override="/usr/bin/qemu-system-x86_64"
> > #bios="ovmf"
> > memory=2048
> > vcpus=2
> > #nestedhvm=1
> > #vif=['model=e1000,bridge=xenbr0']
> > vif=['bridge=xenbr0,mac=00:16:3e:42:ae:8f']
> > disk=['/mnt/vm/disks/W7.disk1.xm,raw,hda,rw',',raw,hdb,ro,cdrom']
> > boot='dc'
> > device_model_version="qemu-xen"
> > viridian=1
> > vnc=0
> > keymap="it"
> > on_crash="destroy"
> > vga="qxl"
> > #videoram=64
> > spice=1
> > spicehost='0.0.0.0'
> > spiceport=6002
> > spicedisable_ticketing=1
> > spicevdagent=1
> > spice_clipboard_sharing=0
> > spice_image_compression="off"
> > spice_streaming_video="filter"
> > spiceusbredirection=4
> > soundhw="hda"

>From the error message, it looks like soundhw is the option that
conflicts with xen-platform,addr=0x2.


> > localtime=1
> > usbversion=2
> 
> Probably there are also other cases that can create a problem with
> xen-platform fixed address, FWIK now new usb controller (with usbversion) is
> the only other with fixed pci address in libxl, all other emulated qemu
> components not.
> And call it before in qemu binary starts notaffect the pci slot order because
> the xen-platform is already before audio.

Actually it looks like that by passing -device xen-platform before the
other options, it already tries to assign slot 2 if possible. I don't
need to specify addr=0x2. That seems to be the best course of action.

Thanks for testing!

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-05-28 16:18         ` Stefano Stabellini
@ 2014-05-28 16:41           ` Stefano Stabellini
  2014-05-28 16:50             ` Paolo Bonzini
  2014-05-28 16:53             ` [Xen-devel] " Fabio Fantoni
  0 siblings, 2 replies; 31+ messages in thread
From: Stefano Stabellini @ 2014-05-28 16:41 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Ian Campbell, Ian Jackson, qemu-devel, Fabio Fantoni,
	Anthony.Perard, tiejun.chen, Paul Durrant

On Wed, 28 May 2014, Stefano Stabellini wrote:
> On Wed, 28 May 2014, Fabio Fantoni wrote:
> > Il 26/05/2014 10:00, Fabio Fantoni ha scritto:
> > > Il 25/05/2014 16:14, Stefano Stabellini ha scritto:
> > > > On Fri, 23 May 2014, Fabio Fantoni wrote:
> > > > > Il 23/05/2014 18:07, Stefano Stabellini ha scritto:
> > > > > > Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
> > > > > > sure what is the machine that we are emulating.
> > > > > > 
> > > > > > Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> > > > > > xen-platform device if requested. Choose slot 2 for the xen-platform
> > > > > > device for compatibility with current installations. In case of Intel
> > > > > > graphic passthrough, slot 2 might be needed by the grafic card.
> > > > > > However
> > > > > > now that we can specify the slot explicitly, it is easy to change the
> > > > > > position of the xen-platform device on the PCI bus if graphic
> > > > > > passthrough is enabled.
> > > > > > 
> > > > > > Move the machine options earlier, before any other emulated devices
> > > > > > options. Otherwise the selected PCI slot for the xen-platform device
> > > > > > is
> > > > > > not available in QEMU.
> > > > > > 
> > > > > > Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> > > > > > differently from xenfv, the other QEMU machines do not have that
> > > > > > option
> > > > > > off by default.
> > > > > > 
> > > > > > This patch does not change the emulated environment in the guest.
> > > > > > 
> > > > > > Refer to this thread:
> > > > > > http://marc.info/?l=xen-devel&m=140023775929625&w=2
> > > > > > 
> > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > > > > 
> > > > > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > > > > > index 8abed7b..fef684f 100644
> > > > > > --- a/tools/libxl/libxl_dm.c
> > > > > > +++ b/tools/libxl/libxl_dm.c
> > > > > > @@ -476,6 +476,29 @@ static char **
> > > > > > libxl__build_device_model_args_new(libxl__gc *gc,
> > > > > >            flexarray_vappend(dm_args, "-k", keymap, NULL);
> > > > > >        }
> > > > > >    +    flexarray_append(dm_args, "-machine");
> > > > > > +    switch (b_info->type) {
> > > > > > +    case LIBXL_DOMAIN_TYPE_PV:
> > > > > > +        flexarray_append(dm_args, "xenpv");
> > > > > > +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL;
> > > > > > i++)
> > > > > > +            flexarray_append(dm_args, b_info->extra_pv[i]);
> > > > > > +        break;
> > > > > > +    case LIBXL_DOMAIN_TYPE_HVM:
> > > > > > +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> > > 
> > > I think that a note in README should be added: qemu >=1.6.1 is required for
> > > hvm domUs.
> > > 
> > > About the other xl parameter to be added I think that should be:
> > > qemu_machine_type="i440fx"|"q35" (when also q35 will be supported on xen)
> > > qemu_machine_version="x.y" to specify the machine version, useful for
> > > debug/testing and other some cases.
> > > 
> > > > > > +        flexarray_append(dm_args, "-global");
> > > > > > +        flexarray_append(dm_args,
> > > > > > "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
> > > > > I think is good add a comment for remember to remove this workaround
> > > > > when pc
> > > > > > =2.1 will be the default since qemu 2.1 will fix it.
> > > > > https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04789.html
> > > > The workaround is not actually harmful, it doesn't need to be removed
> > > > when pc >= 2.1 in QEMU.
> > > > 
> > > > 
> > > > > > +        if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
> > > > > > +            flexarray_append(dm_args, "-device");
> > > > > > +            flexarray_append(dm_args, "xen-platform,addr=0x2");
> > > > > The fixed pci address to 0x2 probably is a problem with intel gpu
> > > > > passthrough:
> > > > > http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03726.html
> > > > Right, however we cannot really change the default position of the
> > > > xen-platform device on the PCI bus, otherwise we'll end up changing the
> > > > emulated environment for all the VMs out there.
> > > > 
> > > > I'll leave it to Tiejun to move xen-platform to another slot when
> > > > graphic passthrough is enabled.
> > > 
> > 
> > I found another case of problem with xen-platform's fixed pci slot.
> > I tested this patch and I saw that qemu not start also in other cases, for
> > example the domU of my test:
> > > qemu-system-i386: -device xen-platform,addr=0x2: PCI: slot 2 function 0 not
> > > available for xen-platform, in use by intel-hda
> > > qemu-system-i386: -device xen-platform,addr=0x2: Device initialization
> > > failed.
> > > qemu-system-i386: -device xen-platform,addr=0x2: Device 'xen-platform' could
> > > not be initialized
> > 
> > The domU's xl cfg:
> > > name='W7'
> > > builder="hvm"
> > > #device_model_override="/usr/lib/xen/bin/qemu-gdb"
> > > #device_model_override="/usr/bin/qemu-system-x86_64"
> > > #bios="ovmf"
> > > memory=2048
> > > vcpus=2
> > > #nestedhvm=1
> > > #vif=['model=e1000,bridge=xenbr0']
> > > vif=['bridge=xenbr0,mac=00:16:3e:42:ae:8f']
> > > disk=['/mnt/vm/disks/W7.disk1.xm,raw,hda,rw',',raw,hdb,ro,cdrom']
> > > boot='dc'
> > > device_model_version="qemu-xen"
> > > viridian=1
> > > vnc=0
> > > keymap="it"
> > > on_crash="destroy"
> > > vga="qxl"
> > > #videoram=64
> > > spice=1
> > > spicehost='0.0.0.0'
> > > spiceport=6002
> > > spicedisable_ticketing=1
> > > spicevdagent=1
> > > spice_clipboard_sharing=0
> > > spice_image_compression="off"
> > > spice_streaming_video="filter"
> > > spiceusbredirection=4
> > > soundhw="hda"
> 
> >From the error message, it looks like soundhw is the option that
> conflicts with xen-platform,addr=0x2.
> 
> 
> > > localtime=1
> > > usbversion=2
> > 
> > Probably there are also other cases that can create a problem with
> > xen-platform fixed address, FWIK now new usb controller (with usbversion) is
> > the only other with fixed pci address in libxl, all other emulated qemu
> > components not.
> > And call it before in qemu binary starts notaffect the pci slot order because
> > the xen-platform is already before audio.
> 
> Actually it looks like that by passing -device xen-platform before the
> other options, it already tries to assign slot 2 if possible. I don't
> need to specify addr=0x2. That seems to be the best course of action.
 
However it would still place xen-platform at slot 3 instead of slot 2 if
soundhw is specified. It seems to me that there is not a perfect
solution to this problem.  We can either:

- Switch to -machine pc-i440fx-1.6 by default and use consistently the
same -machine option no matter the value of xen_platform_pci. Nice, but
we break compatibility with existing guests if soundhw is specified.

- Switch to -machine pc-i440fx-1.6 only when xen_platform_pci=0 is
specified and keep xenfv if xen_platform_pci=1. We still break
compatibility when soundhw is specified together with xen_platform_pci=0.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-05-28 16:41           ` Stefano Stabellini
@ 2014-05-28 16:50             ` Paolo Bonzini
  2014-05-28 16:59               ` Fabio Fantoni
  2014-06-03 13:38                 ` Stefano Stabellini
  2014-05-28 16:53             ` [Xen-devel] " Fabio Fantoni
  1 sibling, 2 replies; 31+ messages in thread
From: Paolo Bonzini @ 2014-05-28 16:50 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Ian Campbell, Ian Jackson, qemu-devel, Fabio Fantoni,
	Anthony.Perard, tiejun.chen, Paul Durrant

Il 28/05/2014 18:41, Stefano Stabellini ha scritto:
> However it would still place xen-platform at slot 3 instead of slot 2 if
> soundhw is specified. It seems to me that there is not a perfect
> solution to this problem.  We can either:
>
> - Switch to -machine pc-i440fx-1.6 by default and use consistently the
> same -machine option no matter the value of xen_platform_pci. Nice, but
> we break compatibility with existing guests if soundhw is specified.
>
> - Switch to -machine pc-i440fx-1.6 only when xen_platform_pci=0 is
> specified and keep xenfv if xen_platform_pci=1. We still break
> compatibility when soundhw is specified together with xen_platform_pci=0.

- change the implementation of the soundhw option to use -device instead.

Paolo

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-05-28 16:41           ` Stefano Stabellini
  2014-05-28 16:50             ` Paolo Bonzini
@ 2014-05-28 16:53             ` Fabio Fantoni
  2014-05-29  8:08               ` Fabio Fantoni
  1 sibling, 1 reply; 31+ messages in thread
From: Fabio Fantoni @ 2014-05-28 16:53 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Ian Campbell, Ian Jackson, qemu-devel, Paul Durrant,
	Anthony PERARD, Tiejun Chen

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

2014-05-28 18:41 GMT+02:00 Stefano Stabellini <
stefano.stabellini@eu.citrix.com>:

> On Wed, 28 May 2014, Stefano Stabellini wrote:
> > On Wed, 28 May 2014, Fabio Fantoni wrote:
> > > Il 26/05/2014 10:00, Fabio Fantoni ha scritto:
> > > > Il 25/05/2014 16:14, Stefano Stabellini ha scritto:
> > > > > On Fri, 23 May 2014, Fabio Fantoni wrote:
> > > > > > Il 23/05/2014 18:07, Stefano Stabellini ha scritto:
> > > > > > > Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we
> know for
> > > > > > > sure what is the machine that we are emulating.
> > > > > > >
> > > > > > > Use pc-i440fx-1.6 regardless of the xen_platform_pci option.
> Add the
> > > > > > > xen-platform device if requested. Choose slot 2 for the
> xen-platform
> > > > > > > device for compatibility with current installations. In case
> of Intel
> > > > > > > graphic passthrough, slot 2 might be needed by the grafic card.
> > > > > > > However
> > > > > > > now that we can specify the slot explicitly, it is easy to
> change the
> > > > > > > position of the xen-platform device on the PCI bus if graphic
> > > > > > > passthrough is enabled.
> > > > > > >
> > > > > > > Move the machine options earlier, before any other emulated
> devices
> > > > > > > options. Otherwise the selected PCI slot for the xen-platform
> device
> > > > > > > is
> > > > > > > not available in QEMU.
> > > > > > >
> > > > > > > Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off,
> because
> > > > > > > differently from xenfv, the other QEMU machines do not have
> that
> > > > > > > option
> > > > > > > off by default.
> > > > > > >
> > > > > > > This patch does not change the emulated environment in the
> guest.
> > > > > > >
> > > > > > > Refer to this thread:
> > > > > > > http://marc.info/?l=xen-devel&m=140023775929625&w=2
> > > > > > >
> > > > > > > Signed-off-by: Stefano Stabellini <
> stefano.stabellini@eu.citrix.com>
> > > > > > >
> > > > > > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > > > > > > index 8abed7b..fef684f 100644
> > > > > > > --- a/tools/libxl/libxl_dm.c
> > > > > > > +++ b/tools/libxl/libxl_dm.c
> > > > > > > @@ -476,6 +476,29 @@ static char **
> > > > > > > libxl__build_device_model_args_new(libxl__gc *gc,
> > > > > > >            flexarray_vappend(dm_args, "-k", keymap, NULL);
> > > > > > >        }
> > > > > > >    +    flexarray_append(dm_args, "-machine");
> > > > > > > +    switch (b_info->type) {
> > > > > > > +    case LIBXL_DOMAIN_TYPE_PV:
> > > > > > > +        flexarray_append(dm_args, "xenpv");
> > > > > > > +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i]
> != NULL;
> > > > > > > i++)
> > > > > > > +            flexarray_append(dm_args, b_info->extra_pv[i]);
> > > > > > > +        break;
> > > > > > > +    case LIBXL_DOMAIN_TYPE_HVM:
> > > > > > > +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> > > >
> > > > I think that a note in README should be added: qemu >=1.6.1 is
> required for
> > > > hvm domUs.
> > > >
> > > > About the other xl parameter to be added I think that should be:
> > > > qemu_machine_type="i440fx"|"q35" (when also q35 will be supported on
> xen)
> > > > qemu_machine_version="x.y" to specify the machine version, useful for
> > > > debug/testing and other some cases.
> > > >
> > > > > > > +        flexarray_append(dm_args, "-global");
> > > > > > > +        flexarray_append(dm_args,
> > > > > > > "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
> > > > > > I think is good add a comment for remember to remove this
> workaround
> > > > > > when pc
> > > > > > > =2.1 will be the default since qemu 2.1 will fix it.
> > > > > >
> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04789.html
> > > > > The workaround is not actually harmful, it doesn't need to be
> removed
> > > > > when pc >= 2.1 in QEMU.
> > > > >
> > > > >
> > > > > > > +        if
> (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
> > > > > > > +            flexarray_append(dm_args, "-device");
> > > > > > > +            flexarray_append(dm_args,
> "xen-platform,addr=0x2");
> > > > > > The fixed pci address to 0x2 probably is a problem with intel gpu
> > > > > > passthrough:
> > > > > >
> http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03726.html
> > > > > Right, however we cannot really change the default position of the
> > > > > xen-platform device on the PCI bus, otherwise we'll end up
> changing the
> > > > > emulated environment for all the VMs out there.
> > > > >
> > > > > I'll leave it to Tiejun to move xen-platform to another slot when
> > > > > graphic passthrough is enabled.
> > > >
> > >
> > > I found another case of problem with xen-platform's fixed pci slot.
> > > I tested this patch and I saw that qemu not start also in other cases,
> for
> > > example the domU of my test:
> > > > qemu-system-i386: -device xen-platform,addr=0x2: PCI: slot 2
> function 0 not
> > > > available for xen-platform, in use by intel-hda
> > > > qemu-system-i386: -device xen-platform,addr=0x2: Device
> initialization
> > > > failed.
> > > > qemu-system-i386: -device xen-platform,addr=0x2: Device
> 'xen-platform' could
> > > > not be initialized
> > >
> > > The domU's xl cfg:
> > > > name='W7'
> > > > builder="hvm"
> > > > #device_model_override="/usr/lib/xen/bin/qemu-gdb"
> > > > #device_model_override="/usr/bin/qemu-system-x86_64"
> > > > #bios="ovmf"
> > > > memory=2048
> > > > vcpus=2
> > > > #nestedhvm=1
> > > > #vif=['model=e1000,bridge=xenbr0']
> > > > vif=['bridge=xenbr0,mac=00:16:3e:42:ae:8f']
> > > > disk=['/mnt/vm/disks/W7.disk1.xm,raw,hda,rw',',raw,hdb,ro,cdrom']
> > > > boot='dc'
> > > > device_model_version="qemu-xen"
> > > > viridian=1
> > > > vnc=0
> > > > keymap="it"
> > > > on_crash="destroy"
> > > > vga="qxl"
> > > > #videoram=64
> > > > spice=1
> > > > spicehost='0.0.0.0'
> > > > spiceport=6002
> > > > spicedisable_ticketing=1
> > > > spicevdagent=1
> > > > spice_clipboard_sharing=0
> > > > spice_image_compression="off"
> > > > spice_streaming_video="filter"
> > > > spiceusbredirection=4
> > > > soundhw="hda"
> >
> > >From the error message, it looks like soundhw is the option that
> > conflicts with xen-platform,addr=0x2.
> >
> >
> > > > localtime=1
> > > > usbversion=2
> > >
> > > Probably there are also other cases that can create a problem with
> > > xen-platform fixed address, FWIK now new usb controller (with
> usbversion) is
> > > the only other with fixed pci address in libxl, all other emulated qemu
> > > components not.
> > > And call it before in qemu binary starts notaffect the pci slot order
> because
> > > the xen-platform is already before audio.
> >
> > Actually it looks like that by passing -device xen-platform before the
> > other options, it already tries to assign slot 2 if possible. I don't
> > need to specify addr=0x2. That seems to be the best course of action.
>
> However it would still place xen-platform at slot 3 instead of slot 2 if
> soundhw is specified. It seems to me that there is not a perfect
> solution to this problem.  We can either:
>
> - Switch to -machine pc-i440fx-1.6 by default and use consistently the
> same -machine option no matter the value of xen_platform_pci. Nice, but
> we break compatibility with existing guests if soundhw is specified.
>
> - Switch to -machine pc-i440fx-1.6 only when xen_platform_pci=0 is
> specified and keep xenfv if xen_platform_pci=1. We still break
> compatibility when soundhw is specified together with xen_platform_pci=0.
>

FWIK removing the xen-platform's fixed pci slot will keep compatibility in
any cases, assigning slots as it did before.
I'll try tomorrow.
Otherwise I think there would be problems in other cases, not only audio
case.
Perhaps even with save/restore/migrate.

[-- Attachment #2: Type: text/html, Size: 11541 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-05-28 16:50             ` Paolo Bonzini
@ 2014-05-28 16:59               ` Fabio Fantoni
  2014-06-03 13:38                 ` Stefano Stabellini
  1 sibling, 0 replies; 31+ messages in thread
From: Fabio Fantoni @ 2014-05-28 16:59 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: xen-devel, Ian Campbell, Stefano Stabellini, Ian Jackson,
	qemu-devel, Paul Durrant, Anthony PERARD, Tiejun Chen

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

2014-05-28 18:50 GMT+02:00 Paolo Bonzini <pbonzini@redhat.com>:

> Il 28/05/2014 18:41, Stefano Stabellini ha scritto:
>
>  However it would still place xen-platform at slot 3 instead of slot 2 if
>> soundhw is specified. It seems to me that there is not a perfect
>> solution to this problem.  We can either:
>>
>> - Switch to -machine pc-i440fx-1.6 by default and use consistently the
>> same -machine option no matter the value of xen_platform_pci. Nice, but
>> we break compatibility with existing guests if soundhw is specified.
>>
>> - Switch to -machine pc-i440fx-1.6 only when xen_platform_pci=0 is
>> specified and keep xenfv if xen_platform_pci=1. We still break
>> compatibility when soundhw is specified together with xen_platform_pci=0.
>>
>
> - change the implementation of the soundhw option to use -device instead.
>
> Paolo
>

I already did the new features with -device and switched vga to -device but
when I tried to convert the disks to -device (also required for
compatibility q35) had a problem that I was not able to solve (and also
reported to qemu-devel time ago), and then I had postponed the complete
conversion to -device.

[-- Attachment #2: Type: text/html, Size: 2274 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-05-28 16:53             ` [Xen-devel] " Fabio Fantoni
@ 2014-05-29  8:08               ` Fabio Fantoni
  0 siblings, 0 replies; 31+ messages in thread
From: Fabio Fantoni @ 2014-05-29  8:08 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Ian Campbell, Ian Jackson, qemu-devel, Paul Durrant,
	Anthony PERARD, Tiejun Chen

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

Il 28/05/2014 18:53, Fabio Fantoni ha scritto:
> 2014-05-28 18:41 GMT+02:00 Stefano Stabellini 
> <stefano.stabellini@eu.citrix.com 
> <mailto:stefano.stabellini@eu.citrix.com>>:
>
>     On Wed, 28 May 2014, Stefano Stabellini wrote:
>     > On Wed, 28 May 2014, Fabio Fantoni wrote:
>     > > Il 26/05/2014 10:00, Fabio Fantoni ha scritto:
>     > > > Il 25/05/2014 16:14, Stefano Stabellini ha scritto:
>     > > > > On Fri, 23 May 2014, Fabio Fantoni wrote:
>     > > > > > Il 23/05/2014 18:07, Stefano Stabellini ha scritto:
>     > > > > > > Choose pc-i440fx-1.6 instead of pc for HVM guests, so
>     that we know for
>     > > > > > > sure what is the machine that we are emulating.
>     > > > > > >
>     > > > > > > Use pc-i440fx-1.6 regardless of the xen_platform_pci
>     option. Add the
>     > > > > > > xen-platform device if requested. Choose slot 2 for
>     the xen-platform
>     > > > > > > device for compatibility with current installations.
>     In case of Intel
>     > > > > > > graphic passthrough, slot 2 might be needed by the
>     grafic card.
>     > > > > > > However
>     > > > > > > now that we can specify the slot explicitly, it is
>     easy to change the
>     > > > > > > position of the xen-platform device on the PCI bus if
>     graphic
>     > > > > > > passthrough is enabled.
>     > > > > > >
>     > > > > > > Move the machine options earlier, before any other
>     emulated devices
>     > > > > > > options. Otherwise the selected PCI slot for the
>     xen-platform device
>     > > > > > > is
>     > > > > > > not available in QEMU.
>     > > > > > >
>     > > > > > > Specify
>     PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
>     > > > > > > differently from xenfv, the other QEMU machines do not
>     have that
>     > > > > > > option
>     > > > > > > off by default.
>     > > > > > >
>     > > > > > > This patch does not change the emulated environment in
>     the guest.
>     > > > > > >
>     > > > > > > Refer to this thread:
>     > > > > > > http://marc.info/?l=xen-devel&m=140023775929625&w=2
>     > > > > > >
>     > > > > > > Signed-off-by: Stefano Stabellini
>     <stefano.stabellini@eu.citrix.com
>     <mailto:stefano.stabellini@eu.citrix.com>>
>     > > > > > >
>     > > > > > > diff --git a/tools/libxl/libxl_dm.c
>     b/tools/libxl/libxl_dm.c
>     > > > > > > index 8abed7b..fef684f 100644
>     > > > > > > --- a/tools/libxl/libxl_dm.c
>     > > > > > > +++ b/tools/libxl/libxl_dm.c
>     > > > > > > @@ -476,6 +476,29 @@ static char **
>     > > > > > > libxl__build_device_model_args_new(libxl__gc *gc,
>     > > > > > >  flexarray_vappend(dm_args, "-k", keymap, NULL);
>     > > > > > >        }
>     > > > > > >    +  flexarray_append(dm_args, "-machine");
>     > > > > > > +    switch (b_info->type) {
>     > > > > > > +    case LIBXL_DOMAIN_TYPE_PV:
>     > > > > > > +  flexarray_append(dm_args, "xenpv");
>     > > > > > > +        for (i = 0; b_info->extra_pv &&
>     b_info->extra_pv[i] != NULL;
>     > > > > > > i++)
>     > > > > > > +  flexarray_append(dm_args, b_info->extra_pv[i]);
>     > > > > > > +        break;
>     > > > > > > +    case LIBXL_DOMAIN_TYPE_HVM:
>     > > > > > > +  flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
>     > > >
>     > > > I think that a note in README should be added: qemu >=1.6.1
>     is required for
>     > > > hvm domUs.
>     > > >
>     > > > About the other xl parameter to be added I think that should be:
>     > > > qemu_machine_type="i440fx"|"q35" (when also q35 will be
>     supported on xen)
>     > > > qemu_machine_version="x.y" to specify the machine version,
>     useful for
>     > > > debug/testing and other some cases.
>     > > >
>     > > > > > > +  flexarray_append(dm_args, "-global");
>     > > > > > > +  flexarray_append(dm_args,
>     > > > > > > "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
>     > > > > > I think is good add a comment for remember to remove
>     this workaround
>     > > > > > when pc
>     > > > > > > =2.1 will be the default since qemu 2.1 will fix it.
>     > > > > >
>     https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04789.html
>     > > > > The workaround is not actually harmful, it doesn't need to
>     be removed
>     > > > > when pc >= 2.1 in QEMU.
>     > > > >
>     > > > >
>     > > > > > > +        if
>     (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
>     > > > > > > +  flexarray_append(dm_args, "-device");
>     > > > > > > +  flexarray_append(dm_args, "xen-platform,addr=0x2");
>     > > > > > The fixed pci address to 0x2 probably is a problem with
>     intel gpu
>     > > > > > passthrough:
>     > > > > >
>     http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03726.html
>     > > > > Right, however we cannot really change the default
>     position of the
>     > > > > xen-platform device on the PCI bus, otherwise we'll end up
>     changing the
>     > > > > emulated environment for all the VMs out there.
>     > > > >
>     > > > > I'll leave it to Tiejun to move xen-platform to another
>     slot when
>     > > > > graphic passthrough is enabled.
>     > > >
>     > >
>     > > I found another case of problem with xen-platform's fixed pci
>     slot.
>     > > I tested this patch and I saw that qemu not start also in
>     other cases, for
>     > > example the domU of my test:
>     > > > qemu-system-i386: -device xen-platform,addr=0x2: PCI: slot 2
>     function 0 not
>     > > > available for xen-platform, in use by intel-hda
>     > > > qemu-system-i386: -device xen-platform,addr=0x2: Device
>     initialization
>     > > > failed.
>     > > > qemu-system-i386: -device xen-platform,addr=0x2: Device
>     'xen-platform' could
>     > > > not be initialized
>     > >
>     > > The domU's xl cfg:
>     > > > name='W7'
>     > > > builder="hvm"
>     > > > #device_model_override="/usr/lib/xen/bin/qemu-gdb"
>     > > > #device_model_override="/usr/bin/qemu-system-x86_64"
>     > > > #bios="ovmf"
>     > > > memory=2048
>     > > > vcpus=2
>     > > > #nestedhvm=1
>     > > > #vif=['model=e1000,bridge=xenbr0']
>     > > > vif=['bridge=xenbr0,mac=00:16:3e:42:ae:8f']
>     > > >
>     disk=['/mnt/vm/disks/W7.disk1.xm,raw,hda,rw',',raw,hdb,ro,cdrom']
>     > > > boot='dc'
>     > > > device_model_version="qemu-xen"
>     > > > viridian=1
>     > > > vnc=0
>     > > > keymap="it"
>     > > > on_crash="destroy"
>     > > > vga="qxl"
>     > > > #videoram=64
>     > > > spice=1
>     > > > spicehost='0.0.0.0'
>     > > > spiceport=6002
>     > > > spicedisable_ticketing=1
>     > > > spicevdagent=1
>     > > > spice_clipboard_sharing=0
>     > > > spice_image_compression="off"
>     > > > spice_streaming_video="filter"
>     > > > spiceusbredirection=4
>     > > > soundhw="hda"
>     >
>     > >From the error message, it looks like soundhw is the option that
>     > conflicts with xen-platform,addr=0x2.
>     >
>     >
>     > > > localtime=1
>     > > > usbversion=2
>     > >
>     > > Probably there are also other cases that can create a problem with
>     > > xen-platform fixed address, FWIK now new usb controller (with
>     usbversion) is
>     > > the only other with fixed pci address in libxl, all other
>     emulated qemu
>     > > components not.
>     > > And call it before in qemu binary starts notaffect the pci
>     slot order because
>     > > the xen-platform is already before audio.
>     >
>     > Actually it looks like that by passing -device xen-platform
>     before the
>     > other options, it already tries to assign slot 2 if possible. I
>     don't
>     > need to specify addr=0x2. That seems to be the best course of
>     action.
>
>     However it would still place xen-platform at slot 3 instead of
>     slot 2 if
>     soundhw is specified. It seems to me that there is not a perfect
>     solution to this problem.  We can either:
>
>     - Switch to -machine pc-i440fx-1.6 by default and use consistently the
>     same -machine option no matter the value of xen_platform_pci.
>     Nice, but
>     we break compatibility with existing guests if soundhw is specified.
>
>     - Switch to -machine pc-i440fx-1.6 only when xen_platform_pci=0 is
>     specified and keep xenfv if xen_platform_pci=1. We still break
>     compatibility when soundhw is specified together with
>     xen_platform_pci=0.
>
>
> FWIK removing the xen-platform's fixed pci slot will keep 
> compatibility in any cases, assigning slots as it did before.
> I'll try tomorrow.
> Otherwise I think there would be problems in other cases, not only 
> audio case.
> Perhaps even with save/restore/migrate.
>

Removing xen-platform's fixed pci slot works, I not found any case with 
problem for now.
The only problem probably will be only intel graphic passthrough (not 
tested), same with fixed pci slot of starting patch.


[-- Attachment #2: Type: text/html, Size: 16618 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-05-28 16:50             ` Paolo Bonzini
@ 2014-06-03 13:38                 ` Stefano Stabellini
  2014-06-03 13:38                 ` Stefano Stabellini
  1 sibling, 0 replies; 31+ messages in thread
From: Stefano Stabellini @ 2014-06-03 13:38 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: xen-devel, Ian Campbell, Stefano Stabellini, Ian Jackson,
	qemu-devel, Fabio Fantoni, Anthony.Perard, tiejun.chen,
	Paul Durrant

On Wed, 28 May 2014, Paolo Bonzini wrote:
> Il 28/05/2014 18:41, Stefano Stabellini ha scritto:
> > However it would still place xen-platform at slot 3 instead of slot 2 if
> > soundhw is specified. It seems to me that there is not a perfect
> > solution to this problem.  We can either:
> > 
> > - Switch to -machine pc-i440fx-1.6 by default and use consistently the
> > same -machine option no matter the value of xen_platform_pci. Nice, but
> > we break compatibility with existing guests if soundhw is specified.
> > 
> > - Switch to -machine pc-i440fx-1.6 only when xen_platform_pci=0 is
> > specified and keep xenfv if xen_platform_pci=1. We still break
> > compatibility when soundhw is specified together with xen_platform_pci=0.
> 
> - change the implementation of the soundhw option to use -device instead.

Thanks, this sounds like the best option by far.

I noticed that "-soundhw hda" becomes "-device intel-hda".

Given that the VM config file exports a soundhw paramter, we would need
a programmatic way to get the -device command line option from the
soundhw paramter. Is there a way to do that?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
@ 2014-06-03 13:38                 ` Stefano Stabellini
  0 siblings, 0 replies; 31+ messages in thread
From: Stefano Stabellini @ 2014-06-03 13:38 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: xen-devel, Ian Campbell, Stefano Stabellini, Ian Jackson,
	qemu-devel, Fabio Fantoni, Anthony.Perard, tiejun.chen,
	Paul Durrant

On Wed, 28 May 2014, Paolo Bonzini wrote:
> Il 28/05/2014 18:41, Stefano Stabellini ha scritto:
> > However it would still place xen-platform at slot 3 instead of slot 2 if
> > soundhw is specified. It seems to me that there is not a perfect
> > solution to this problem.  We can either:
> > 
> > - Switch to -machine pc-i440fx-1.6 by default and use consistently the
> > same -machine option no matter the value of xen_platform_pci. Nice, but
> > we break compatibility with existing guests if soundhw is specified.
> > 
> > - Switch to -machine pc-i440fx-1.6 only when xen_platform_pci=0 is
> > specified and keep xenfv if xen_platform_pci=1. We still break
> > compatibility when soundhw is specified together with xen_platform_pci=0.
> 
> - change the implementation of the soundhw option to use -device instead.

Thanks, this sounds like the best option by far.

I noticed that "-soundhw hda" becomes "-device intel-hda".

Given that the VM config file exports a soundhw paramter, we would need
a programmatic way to get the -device command line option from the
soundhw paramter. Is there a way to do that?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-06-03 13:38                 ` Stefano Stabellini
@ 2014-06-03 13:43                   ` Paolo Bonzini
  -1 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2014-06-03 13:43 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Ian Campbell, Ian Jackson, qemu-devel, Fabio Fantoni,
	Anthony.Perard, tiejun.chen, Paul Durrant

Il 03/06/2014 15:38, Stefano Stabellini ha scritto:
> Thanks, this sounds like the best option by far.
>
> I noticed that "-soundhw hda" becomes "-device intel-hda".
>
> Given that the VM config file exports a soundhw paramter, we would need
> a programmatic way to get the -device command line option from the
> soundhw paramter. Is there a way to do that?

A table. :)

-soundhw	corresponding -device syntax
---------------------------------------------------------
ac97		-device AC97
adlib		-device adlib
cs4231a		-device cs4231a
es1370		-device ES1370
gus		-device gus
hda		-device intel-hda,id=libxl-intel-hda \
		-device hda-duplex,bus=libxl-intel-hda.0
pcspk		-device isa-pcspk
sb16		-device sb16

(To review it start with "git grep _register_soundhw").

Paolo

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
@ 2014-06-03 13:43                   ` Paolo Bonzini
  0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2014-06-03 13:43 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Ian Campbell, Ian Jackson, qemu-devel, Fabio Fantoni,
	Anthony.Perard, tiejun.chen, Paul Durrant

Il 03/06/2014 15:38, Stefano Stabellini ha scritto:
> Thanks, this sounds like the best option by far.
>
> I noticed that "-soundhw hda" becomes "-device intel-hda".
>
> Given that the VM config file exports a soundhw paramter, we would need
> a programmatic way to get the -device command line option from the
> soundhw paramter. Is there a way to do that?

A table. :)

-soundhw	corresponding -device syntax
---------------------------------------------------------
ac97		-device AC97
adlib		-device adlib
cs4231a		-device cs4231a
es1370		-device ES1370
gus		-device gus
hda		-device intel-hda,id=libxl-intel-hda \
		-device hda-duplex,bus=libxl-intel-hda.0
pcspk		-device isa-pcspk
sb16		-device sb16

(To review it start with "git grep _register_soundhw").

Paolo

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-06-03 13:38                 ` Stefano Stabellini
@ 2014-06-03 13:47                   ` Ian Campbell
  -1 siblings, 0 replies; 31+ messages in thread
From: Ian Campbell @ 2014-06-03 13:47 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Ian Jackson, qemu-devel, Fabio Fantoni, tiejun.chen,
	Anthony.Perard, Paolo Bonzini, Paul Durrant

On Tue, 2014-06-03 at 14:38 +0100, Stefano Stabellini wrote:
> Given that the VM config file exports a soundhw paramter, 

... as a string.

That sounds like a libxl API mistake to me. libxl should take an enum
for this sort of thing. Then xl should parse whatever strings it likes
(including specific string needed for backward compatibility) into the
appropriate enum values and libxl should generate the correct
corresponding qemu options for whichever version it is using.

libxl will need to keep the existing string field alongside the new enum
field for API compatibility, I suppose it will also need to parse some
legacy values into the enum itself, those probably equate to whatever
qemu -soundhw used to accept.

Icky, but not insurmountable, I think.

Ian.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
@ 2014-06-03 13:47                   ` Ian Campbell
  0 siblings, 0 replies; 31+ messages in thread
From: Ian Campbell @ 2014-06-03 13:47 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Ian Jackson, qemu-devel, Fabio Fantoni, tiejun.chen,
	Anthony.Perard, Paolo Bonzini, Paul Durrant

On Tue, 2014-06-03 at 14:38 +0100, Stefano Stabellini wrote:
> Given that the VM config file exports a soundhw paramter, 

... as a string.

That sounds like a libxl API mistake to me. libxl should take an enum
for this sort of thing. Then xl should parse whatever strings it likes
(including specific string needed for backward compatibility) into the
appropriate enum values and libxl should generate the correct
corresponding qemu options for whichever version it is using.

libxl will need to keep the existing string field alongside the new enum
field for API compatibility, I suppose it will also need to parse some
legacy values into the enum itself, those probably equate to whatever
qemu -soundhw used to accept.

Icky, but not insurmountable, I think.

Ian.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-06-03 13:38                 ` Stefano Stabellini
@ 2014-06-03 14:03                   ` Fabio Fantoni
  -1 siblings, 0 replies; 31+ messages in thread
From: Fabio Fantoni @ 2014-06-03 14:03 UTC (permalink / raw)
  To: Stefano Stabellini, Paolo Bonzini
  Cc: xen-devel, Ian Campbell, Ian Jackson, qemu-devel, Paul Durrant,
	Anthony.Perard, tiejun.chen

Il 03/06/2014 15:38, Stefano Stabellini ha scritto:
> On Wed, 28 May 2014, Paolo Bonzini wrote:
>> Il 28/05/2014 18:41, Stefano Stabellini ha scritto:
>>> However it would still place xen-platform at slot 3 instead of slot 2 if
>>> soundhw is specified. It seems to me that there is not a perfect
>>> solution to this problem.  We can either:
>>>
>>> - Switch to -machine pc-i440fx-1.6 by default and use consistently the
>>> same -machine option no matter the value of xen_platform_pci. Nice, but
>>> we break compatibility with existing guests if soundhw is specified.
>>>
>>> - Switch to -machine pc-i440fx-1.6 only when xen_platform_pci=0 is
>>> specified and keep xenfv if xen_platform_pci=1. We still break
>>> compatibility when soundhw is specified together with xen_platform_pci=0.
>> - change the implementation of the soundhw option to use -device instead.
> Thanks, this sounds like the best option by far.
>
> I noticed that "-soundhw hda" becomes "-device intel-hda".
>
> Given that the VM config file exports a soundhw paramter, we would need
> a programmatic way to get the -device command line option from the
> soundhw paramter. Is there a way to do that?
Not only -device intel-hda, if I remember good there is also -device 
hda-duplex
soundhw now in libxl is only a string passed to qemu without check, 
probably is good add another parameter with a selectable options 
supported, similar to vga, for example audio=ac97|intelhda and libxl 
will check if exist and give to qemu the correct -device parameters.
The only problem is that audio device as many and not all qemu build 
support all (howevercurrently libxl does not care and isthe user having 
to make sure of the features support in qemu and view the log if qemu 
fails).
In addition there are also the disks to convert in -device.
I tried last year but with -device the automatic selection of bus and 
slot (unit now used in xen is not supported in -device if I remember 
good) was not working properly and the manual I did not know if I could 
do it working with cd/disk hot-plug working.

Thanks for any reply and sorry for my bad english.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
@ 2014-06-03 14:03                   ` Fabio Fantoni
  0 siblings, 0 replies; 31+ messages in thread
From: Fabio Fantoni @ 2014-06-03 14:03 UTC (permalink / raw)
  To: Stefano Stabellini, Paolo Bonzini
  Cc: xen-devel, Ian Campbell, Ian Jackson, qemu-devel, Paul Durrant,
	Anthony.Perard, tiejun.chen

Il 03/06/2014 15:38, Stefano Stabellini ha scritto:
> On Wed, 28 May 2014, Paolo Bonzini wrote:
>> Il 28/05/2014 18:41, Stefano Stabellini ha scritto:
>>> However it would still place xen-platform at slot 3 instead of slot 2 if
>>> soundhw is specified. It seems to me that there is not a perfect
>>> solution to this problem.  We can either:
>>>
>>> - Switch to -machine pc-i440fx-1.6 by default and use consistently the
>>> same -machine option no matter the value of xen_platform_pci. Nice, but
>>> we break compatibility with existing guests if soundhw is specified.
>>>
>>> - Switch to -machine pc-i440fx-1.6 only when xen_platform_pci=0 is
>>> specified and keep xenfv if xen_platform_pci=1. We still break
>>> compatibility when soundhw is specified together with xen_platform_pci=0.
>> - change the implementation of the soundhw option to use -device instead.
> Thanks, this sounds like the best option by far.
>
> I noticed that "-soundhw hda" becomes "-device intel-hda".
>
> Given that the VM config file exports a soundhw paramter, we would need
> a programmatic way to get the -device command line option from the
> soundhw paramter. Is there a way to do that?
Not only -device intel-hda, if I remember good there is also -device 
hda-duplex
soundhw now in libxl is only a string passed to qemu without check, 
probably is good add another parameter with a selectable options 
supported, similar to vga, for example audio=ac97|intelhda and libxl 
will check if exist and give to qemu the correct -device parameters.
The only problem is that audio device as many and not all qemu build 
support all (howevercurrently libxl does not care and isthe user having 
to make sure of the features support in qemu and view the log if qemu 
fails).
In addition there are also the disks to convert in -device.
I tried last year but with -device the automatic selection of bus and 
slot (unit now used in xen is not supported in -device if I remember 
good) was not working properly and the manual I did not know if I could 
do it working with cd/disk hot-plug working.

Thanks for any reply and sorry for my bad english.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-05-23 16:07 ` Stefano Stabellini
@ 2014-06-10 11:14   ` Ian Campbell
  -1 siblings, 0 replies; 31+ messages in thread
From: Ian Campbell @ 2014-06-10 11:14 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Ian Jackson, qemu-devel, Paul Durrant, Anthony.Perard,
	tiejun.chen

On Fri, 2014-05-23 at 17:07 +0100, Stefano Stabellini wrote:
> Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
> sure what is the machine that we are emulating.

Does 1.6 here refer to the qemu version? If so then I think we need to
consciously decide that we are happy for Xen 4.5 to depend on qemu >=
1.6 and document it in README.

qemu 1.6 was release in August 2013, which is ~10 months ago, it'll be
approximately a year old when we release. Are we happy with this from a
distro PoV?

I think it is likely that distros which have an older qemu (and users
of such distros) will be using the qemu which comes with Xen (and is
naturally new enough) rather than expecting to use the system qemu.

At some point we really ought to grow an option to let the user choose
the machine...

> Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> xen-platform device if requested. Choose slot 2 for the xen-platform
> device for compatibility with current installations. In case of Intel
> graphic passthrough, slot 2 might be needed by the grafic card. However

"graphics"

> now that we can specify the slot explicitly, it is easy to change the
> position of the xen-platform device on the PCI bus if graphic
> passthrough is enabled.
> 
> Move the machine options earlier, before any other emulated devices
> options. Otherwise the selected PCI slot for the xen-platform device is
> not available in QEMU.
> 
> Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> differently from xenfv, the other QEMU machines do not have that option
> off by default.
> 
> This patch does not change the emulated environment in the guest.
> 
> Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2

This takes me to "[Xen-devel] [v2][PATCH 4/8] xen, gfx passthrough:
reserve 00:02.0 for INTEL IGD" which I can't see the link to (including
via the 2 replies). Wrong link perhaps?

A Message-Id is generally a better identifier since I can feed it to the
archive of my choice and they aren't subject to e.g. future accidentally
renumberings etc.

> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 8abed7b..fef684f 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -476,6 +476,29 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>          flexarray_vappend(dm_args, "-k", keymap, NULL);
>      }
>  
> +    flexarray_append(dm_args, "-machine");
> +    switch (b_info->type) {
> +    case LIBXL_DOMAIN_TYPE_PV:
> +        flexarray_append(dm_args, "xenpv");
> +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
> +            flexarray_append(dm_args, b_info->extra_pv[i]);
> +        break;
> +    case LIBXL_DOMAIN_TYPE_HVM:
> +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> +        flexarray_append(dm_args, "-global");
> +        flexarray_append(dm_args, "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");

FWIW you can use flexarray_append_pair which more naturally gathers an
option and its argument together (improving readability).

(personally I think the -machine should be pulled into both cases using
this method, but that's not a prereq for accepting the patch IMHO)

Other than that the patch itself seems sound (although I dislike code
motion mixed with real changes, I suppose this one is small enough that
I can cope).

Ian.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
@ 2014-06-10 11:14   ` Ian Campbell
  0 siblings, 0 replies; 31+ messages in thread
From: Ian Campbell @ 2014-06-10 11:14 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: xen-devel, Ian Jackson, qemu-devel, Paul Durrant, Anthony.Perard,
	tiejun.chen

On Fri, 2014-05-23 at 17:07 +0100, Stefano Stabellini wrote:
> Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
> sure what is the machine that we are emulating.

Does 1.6 here refer to the qemu version? If so then I think we need to
consciously decide that we are happy for Xen 4.5 to depend on qemu >=
1.6 and document it in README.

qemu 1.6 was release in August 2013, which is ~10 months ago, it'll be
approximately a year old when we release. Are we happy with this from a
distro PoV?

I think it is likely that distros which have an older qemu (and users
of such distros) will be using the qemu which comes with Xen (and is
naturally new enough) rather than expecting to use the system qemu.

At some point we really ought to grow an option to let the user choose
the machine...

> Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> xen-platform device if requested. Choose slot 2 for the xen-platform
> device for compatibility with current installations. In case of Intel
> graphic passthrough, slot 2 might be needed by the grafic card. However

"graphics"

> now that we can specify the slot explicitly, it is easy to change the
> position of the xen-platform device on the PCI bus if graphic
> passthrough is enabled.
> 
> Move the machine options earlier, before any other emulated devices
> options. Otherwise the selected PCI slot for the xen-platform device is
> not available in QEMU.
> 
> Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> differently from xenfv, the other QEMU machines do not have that option
> off by default.
> 
> This patch does not change the emulated environment in the guest.
> 
> Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2

This takes me to "[Xen-devel] [v2][PATCH 4/8] xen, gfx passthrough:
reserve 00:02.0 for INTEL IGD" which I can't see the link to (including
via the 2 replies). Wrong link perhaps?

A Message-Id is generally a better identifier since I can feed it to the
archive of my choice and they aren't subject to e.g. future accidentally
renumberings etc.

> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 8abed7b..fef684f 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -476,6 +476,29 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>          flexarray_vappend(dm_args, "-k", keymap, NULL);
>      }
>  
> +    flexarray_append(dm_args, "-machine");
> +    switch (b_info->type) {
> +    case LIBXL_DOMAIN_TYPE_PV:
> +        flexarray_append(dm_args, "xenpv");
> +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
> +            flexarray_append(dm_args, b_info->extra_pv[i]);
> +        break;
> +    case LIBXL_DOMAIN_TYPE_HVM:
> +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> +        flexarray_append(dm_args, "-global");
> +        flexarray_append(dm_args, "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");

FWIW you can use flexarray_append_pair which more naturally gathers an
option and its argument together (improving readability).

(personally I think the -machine should be pulled into both cases using
this method, but that's not a prereq for accepting the patch IMHO)

Other than that the patch itself seems sound (although I dislike code
motion mixed with real changes, I suppose this one is small enough that
I can cope).

Ian.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-06-10 11:14   ` Ian Campbell
@ 2014-06-11 10:35     ` Stefano Stabellini
  -1 siblings, 0 replies; 31+ messages in thread
From: Stefano Stabellini @ 2014-06-11 10:35 UTC (permalink / raw)
  To: Ian Campbell
  Cc: xen-devel, Stefano Stabellini, Ian Jackson, qemu-devel,
	Paul Durrant, Anthony.Perard, tiejun.chen

On Tue, 10 Jun 2014, Ian Campbell wrote:
> On Fri, 2014-05-23 at 17:07 +0100, Stefano Stabellini wrote:
> > Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
> > sure what is the machine that we are emulating.
> 
> Does 1.6 here refer to the qemu version?

Yes, every new QEMU version introduces a new PC machine version string.


> If so then I think we need to
> consciously decide that we are happy for Xen 4.5 to depend on qemu >=
> 1.6 and document it in README.
> 
> qemu 1.6 was release in August 2013, which is ~10 months ago, it'll be
> approximately a year old when we release. Are we happy with this from a
> distro PoV?
> 
> I think it is likely that distros which have an older qemu (and users
> of such distros) will be using the qemu which comes with Xen (and is
> naturally new enough) rather than expecting to use the system qemu.
> 
> At some point we really ought to grow an option to let the user choose
> the machine...

I think that the dependency on QEMU >= 1.6 is OK. The Xen PV Device went
in v1.6.0 and we certainy cannot go back to versions older than v1.5 due
to missing mapcache fixes. Fabio reported multiple times that QEMU 1.6
is the first stable version he tested.


> > Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> > xen-platform device if requested. Choose slot 2 for the xen-platform
> > device for compatibility with current installations. In case of Intel
> > graphic passthrough, slot 2 might be needed by the grafic card. However
> 
> "graphics"
> 
> > now that we can specify the slot explicitly, it is easy to change the
> > position of the xen-platform device on the PCI bus if graphic
> > passthrough is enabled.
> > 
> > Move the machine options earlier, before any other emulated devices
> > options. Otherwise the selected PCI slot for the xen-platform device is
> > not available in QEMU.
> > 
> > Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> > differently from xenfv, the other QEMU machines do not have that option
> > off by default.
> > 
> > This patch does not change the emulated environment in the guest.
> > 
> > Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2
> 
> This takes me to "[Xen-devel] [v2][PATCH 4/8] xen, gfx passthrough:
> reserve 00:02.0 for INTEL IGD" which I can't see the link to (including
> via the 2 replies). Wrong link perhaps?
> 
> A Message-Id is generally a better identifier since I can feed it to the
> archive of my choice and they aren't subject to e.g. future accidentally
> renumberings etc.

1400237624-8505-5-git-send-email-tiejun.chen@intel.com


> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 8abed7b..fef684f 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -476,6 +476,29 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> >          flexarray_vappend(dm_args, "-k", keymap, NULL);
> >      }
> >  
> > +    flexarray_append(dm_args, "-machine");
> > +    switch (b_info->type) {
> > +    case LIBXL_DOMAIN_TYPE_PV:
> > +        flexarray_append(dm_args, "xenpv");
> > +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
> > +            flexarray_append(dm_args, b_info->extra_pv[i]);
> > +        break;
> > +    case LIBXL_DOMAIN_TYPE_HVM:
> > +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> > +        flexarray_append(dm_args, "-global");
> > +        flexarray_append(dm_args, "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
> 
> FWIW you can use flexarray_append_pair which more naturally gathers an
> option and its argument together (improving readability).
> 
> (personally I think the -machine should be pulled into both cases using
> this method, but that's not a prereq for accepting the patch IMHO)
> 
> Other than that the patch itself seems sound (although I dislike code
> motion mixed with real changes, I suppose this one is small enough that
> I can cope).
 
Should I go ahead with these minor changes and leave the soundhw problem
unsolved? I am not keen on fixing that.

1401803223.1582.5.camel@kazak.uk.xensource.com

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
@ 2014-06-11 10:35     ` Stefano Stabellini
  0 siblings, 0 replies; 31+ messages in thread
From: Stefano Stabellini @ 2014-06-11 10:35 UTC (permalink / raw)
  To: Ian Campbell
  Cc: xen-devel, Stefano Stabellini, Ian Jackson, qemu-devel,
	Paul Durrant, Anthony.Perard, tiejun.chen

On Tue, 10 Jun 2014, Ian Campbell wrote:
> On Fri, 2014-05-23 at 17:07 +0100, Stefano Stabellini wrote:
> > Choose pc-i440fx-1.6 instead of pc for HVM guests, so that we know for
> > sure what is the machine that we are emulating.
> 
> Does 1.6 here refer to the qemu version?

Yes, every new QEMU version introduces a new PC machine version string.


> If so then I think we need to
> consciously decide that we are happy for Xen 4.5 to depend on qemu >=
> 1.6 and document it in README.
> 
> qemu 1.6 was release in August 2013, which is ~10 months ago, it'll be
> approximately a year old when we release. Are we happy with this from a
> distro PoV?
> 
> I think it is likely that distros which have an older qemu (and users
> of such distros) will be using the qemu which comes with Xen (and is
> naturally new enough) rather than expecting to use the system qemu.
> 
> At some point we really ought to grow an option to let the user choose
> the machine...

I think that the dependency on QEMU >= 1.6 is OK. The Xen PV Device went
in v1.6.0 and we certainy cannot go back to versions older than v1.5 due
to missing mapcache fixes. Fabio reported multiple times that QEMU 1.6
is the first stable version he tested.


> > Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> > xen-platform device if requested. Choose slot 2 for the xen-platform
> > device for compatibility with current installations. In case of Intel
> > graphic passthrough, slot 2 might be needed by the grafic card. However
> 
> "graphics"
> 
> > now that we can specify the slot explicitly, it is easy to change the
> > position of the xen-platform device on the PCI bus if graphic
> > passthrough is enabled.
> > 
> > Move the machine options earlier, before any other emulated devices
> > options. Otherwise the selected PCI slot for the xen-platform device is
> > not available in QEMU.
> > 
> > Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> > differently from xenfv, the other QEMU machines do not have that option
> > off by default.
> > 
> > This patch does not change the emulated environment in the guest.
> > 
> > Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2
> 
> This takes me to "[Xen-devel] [v2][PATCH 4/8] xen, gfx passthrough:
> reserve 00:02.0 for INTEL IGD" which I can't see the link to (including
> via the 2 replies). Wrong link perhaps?
> 
> A Message-Id is generally a better identifier since I can feed it to the
> archive of my choice and they aren't subject to e.g. future accidentally
> renumberings etc.

1400237624-8505-5-git-send-email-tiejun.chen@intel.com


> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > 
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 8abed7b..fef684f 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -476,6 +476,29 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> >          flexarray_vappend(dm_args, "-k", keymap, NULL);
> >      }
> >  
> > +    flexarray_append(dm_args, "-machine");
> > +    switch (b_info->type) {
> > +    case LIBXL_DOMAIN_TYPE_PV:
> > +        flexarray_append(dm_args, "xenpv");
> > +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
> > +            flexarray_append(dm_args, b_info->extra_pv[i]);
> > +        break;
> > +    case LIBXL_DOMAIN_TYPE_HVM:
> > +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> > +        flexarray_append(dm_args, "-global");
> > +        flexarray_append(dm_args, "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
> 
> FWIW you can use flexarray_append_pair which more naturally gathers an
> option and its argument together (improving readability).
> 
> (personally I think the -machine should be pulled into both cases using
> this method, but that's not a prereq for accepting the patch IMHO)
> 
> Other than that the patch itself seems sound (although I dislike code
> motion mixed with real changes, I suppose this one is small enough that
> I can cope).
 
Should I go ahead with these minor changes and leave the soundhw problem
unsolved? I am not keen on fixing that.

1401803223.1582.5.camel@kazak.uk.xensource.com

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-06-11 10:35     ` Stefano Stabellini
@ 2014-06-11 10:44       ` Ian Campbell
  -1 siblings, 0 replies; 31+ messages in thread
From: Ian Campbell @ 2014-06-11 10:44 UTC (permalink / raw)
  To: Stefano Stabellini, Konrad Rzeszutek Wilk
  Cc: xen-devel, Ian Jackson, qemu-devel, Paul Durrant, Anthony.Perard,
	tiejun.chen

On Wed, 2014-06-11 at 11:35 +0100, Stefano Stabellini wrote:
> On Tue, 10 Jun 2014, Ian Campbell wrote:
> > On Fri, 2014-05-23 at 17:07 +0100, Stefano Stabellini wrote:
> I think that the dependency on QEMU >= 1.6 is OK. The Xen PV Device went
> in v1.6.0 and we certainy cannot go back to versions older than v1.5 due
> to missing mapcache fixes. Fabio reported multiple times that QEMU 1.6
> is the first stable version he tested.

OK, then please can we update the README (and any other relevant docs)
as part of this change.

> > > Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> > > xen-platform device if requested. Choose slot 2 for the xen-platform
> > > device for compatibility with current installations. In case of Intel
> > > graphic passthrough, slot 2 might be needed by the grafic card. However
> > 
> > "graphics"
> > 
> > > now that we can specify the slot explicitly, it is easy to change the
> > > position of the xen-platform device on the PCI bus if graphic
> > > passthrough is enabled.
> > > 
> > > Move the machine options earlier, before any other emulated devices
> > > options. Otherwise the selected PCI slot for the xen-platform device is
> > > not available in QEMU.
> > > 
> > > Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> > > differently from xenfv, the other QEMU machines do not have that option
> > > off by default.
> > > 
> > > This patch does not change the emulated environment in the guest.
> > > 
> > > Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2
> > 
> > This takes me to "[Xen-devel] [v2][PATCH 4/8] xen, gfx passthrough:
> > reserve 00:02.0 for INTEL IGD" which I can't see the link to (including
> > via the 2 replies). Wrong link perhaps?
> > 
> > A Message-Id is generally a better identifier since I can feed it to the
> > archive of my choice and they aren't subject to e.g. future accidentally
> > renumberings etc.
> 
> 1400237624-8505-5-git-send-email-tiejun.chen@intel.com

(pedantic: the <>s are formally part of the message ID...)

OK, so I'm still not sure why this message is relevant to the sentence
which precedes the link.


> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > 
> > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > > index 8abed7b..fef684f 100644
> > > --- a/tools/libxl/libxl_dm.c
> > > +++ b/tools/libxl/libxl_dm.c
> > > @@ -476,6 +476,29 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> > >          flexarray_vappend(dm_args, "-k", keymap, NULL);
> > >      }
> > >  
> > > +    flexarray_append(dm_args, "-machine");
> > > +    switch (b_info->type) {
> > > +    case LIBXL_DOMAIN_TYPE_PV:
> > > +        flexarray_append(dm_args, "xenpv");
> > > +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
> > > +            flexarray_append(dm_args, b_info->extra_pv[i]);
> > > +        break;
> > > +    case LIBXL_DOMAIN_TYPE_HVM:
> > > +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> > > +        flexarray_append(dm_args, "-global");
> > > +        flexarray_append(dm_args, "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
> > 
> > FWIW you can use flexarray_append_pair which more naturally gathers an
> > option and its argument together (improving readability).
> > 
> > (personally I think the -machine should be pulled into both cases using
> > this method, but that's not a prereq for accepting the patch IMHO)
> > 
> > Other than that the patch itself seems sound (although I dislike code
> > motion mixed with real changes, I suppose this one is small enough that
> > I can cope).
>  
> Should I go ahead with these minor changes

Please.

>  and leave the soundhw problem
> unsolved? I am not keen on fixing that.
> 
> 1401803223.1582.5.camel@kazak.uk.xensource.com

I came into this at the point where we were discussing how to support
using -device for soundhw, but I missed the bit where it was explained
why it was necessary to switch to that scheme.

If this change leads to regressions where existing cfg files no longer
work then we need to decide if that would be blocker for 4.5. I can't
see why it wouldn't be, but it's up to you and Konrad to agree I
suppose.

Ian.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
@ 2014-06-11 10:44       ` Ian Campbell
  0 siblings, 0 replies; 31+ messages in thread
From: Ian Campbell @ 2014-06-11 10:44 UTC (permalink / raw)
  To: Stefano Stabellini, Konrad Rzeszutek Wilk
  Cc: xen-devel, Ian Jackson, qemu-devel, Paul Durrant, Anthony.Perard,
	tiejun.chen

On Wed, 2014-06-11 at 11:35 +0100, Stefano Stabellini wrote:
> On Tue, 10 Jun 2014, Ian Campbell wrote:
> > On Fri, 2014-05-23 at 17:07 +0100, Stefano Stabellini wrote:
> I think that the dependency on QEMU >= 1.6 is OK. The Xen PV Device went
> in v1.6.0 and we certainy cannot go back to versions older than v1.5 due
> to missing mapcache fixes. Fabio reported multiple times that QEMU 1.6
> is the first stable version he tested.

OK, then please can we update the README (and any other relevant docs)
as part of this change.

> > > Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> > > xen-platform device if requested. Choose slot 2 for the xen-platform
> > > device for compatibility with current installations. In case of Intel
> > > graphic passthrough, slot 2 might be needed by the grafic card. However
> > 
> > "graphics"
> > 
> > > now that we can specify the slot explicitly, it is easy to change the
> > > position of the xen-platform device on the PCI bus if graphic
> > > passthrough is enabled.
> > > 
> > > Move the machine options earlier, before any other emulated devices
> > > options. Otherwise the selected PCI slot for the xen-platform device is
> > > not available in QEMU.
> > > 
> > > Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> > > differently from xenfv, the other QEMU machines do not have that option
> > > off by default.
> > > 
> > > This patch does not change the emulated environment in the guest.
> > > 
> > > Refer to this thread: http://marc.info/?l=xen-devel&m=140023775929625&w=2
> > 
> > This takes me to "[Xen-devel] [v2][PATCH 4/8] xen, gfx passthrough:
> > reserve 00:02.0 for INTEL IGD" which I can't see the link to (including
> > via the 2 replies). Wrong link perhaps?
> > 
> > A Message-Id is generally a better identifier since I can feed it to the
> > archive of my choice and they aren't subject to e.g. future accidentally
> > renumberings etc.
> 
> 1400237624-8505-5-git-send-email-tiejun.chen@intel.com

(pedantic: the <>s are formally part of the message ID...)

OK, so I'm still not sure why this message is relevant to the sentence
which precedes the link.


> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > 
> > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > > index 8abed7b..fef684f 100644
> > > --- a/tools/libxl/libxl_dm.c
> > > +++ b/tools/libxl/libxl_dm.c
> > > @@ -476,6 +476,29 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> > >          flexarray_vappend(dm_args, "-k", keymap, NULL);
> > >      }
> > >  
> > > +    flexarray_append(dm_args, "-machine");
> > > +    switch (b_info->type) {
> > > +    case LIBXL_DOMAIN_TYPE_PV:
> > > +        flexarray_append(dm_args, "xenpv");
> > > +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] != NULL; i++)
> > > +            flexarray_append(dm_args, b_info->extra_pv[i]);
> > > +        break;
> > > +    case LIBXL_DOMAIN_TYPE_HVM:
> > > +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> > > +        flexarray_append(dm_args, "-global");
> > > +        flexarray_append(dm_args, "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
> > 
> > FWIW you can use flexarray_append_pair which more naturally gathers an
> > option and its argument together (improving readability).
> > 
> > (personally I think the -machine should be pulled into both cases using
> > this method, but that's not a prereq for accepting the patch IMHO)
> > 
> > Other than that the patch itself seems sound (although I dislike code
> > motion mixed with real changes, I suppose this one is small enough that
> > I can cope).
>  
> Should I go ahead with these minor changes

Please.

>  and leave the soundhw problem
> unsolved? I am not keen on fixing that.
> 
> 1401803223.1582.5.camel@kazak.uk.xensource.com

I came into this at the point where we were discussing how to support
using -device for soundhw, but I missed the bit where it was explained
why it was necessary to switch to that scheme.

If this change leads to regressions where existing cfg files no longer
work then we need to decide if that would be blocker for 4.5. I can't
see why it wouldn't be, but it's up to you and Konrad to agree I
suppose.

Ian.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
  2014-06-11 10:44       ` Ian Campbell
@ 2014-06-11 18:05         ` Fabio Fantoni
  -1 siblings, 0 replies; 31+ messages in thread
From: Fabio Fantoni @ 2014-06-11 18:05 UTC (permalink / raw)
  To: Ian Campbell
  Cc: xen-devel, Konrad Rzeszutek Wilk, Stefano Stabellini,
	Ian Jackson, qemu-devel, Paul Durrant, Anthony PERARD,
	Tiejun Chen

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

2014-06-11 12:44 GMT+02:00 Ian Campbell <Ian.Campbell@citrix.com>:

> On Wed, 2014-06-11 at 11:35 +0100, Stefano Stabellini wrote:
> > On Tue, 10 Jun 2014, Ian Campbell wrote:
> > > On Fri, 2014-05-23 at 17:07 +0100, Stefano Stabellini wrote:
> > I think that the dependency on QEMU >= 1.6 is OK. The Xen PV Device went
> > in v1.6.0 and we certainy cannot go back to versions older than v1.5 due
> > to missing mapcache fixes. Fabio reported multiple times that QEMU 1.6
> > is the first stable version he tested.
>
> OK, then please can we update the README (and any other relevant docs)
> as part of this change.
>

I tested upstream qemu from 1.2 and based on my test at least qemu 1.6.1 is
needed to be fairly complete and stable on xen (no major or critical bugs), I
started using it in production for a few month, in the next few days I will
start to put also qemu 2.0 on a production system.
I want to be remembered to specify >=1.6.1 because 1.6.0 have critical bug
that that prevents all hvm domUs start.


>
> > > > Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> > > > xen-platform device if requested. Choose slot 2 for the xen-platform
> > > > device for compatibility with current installations. In case of Intel
> > > > graphic passthrough, slot 2 might be needed by the grafic card.
> However
> > >
> > > "graphics"
> > >
> > > > now that we can specify the slot explicitly, it is easy to change the
> > > > position of the xen-platform device on the PCI bus if graphic
> > > > passthrough is enabled.
> > > >
> > > > Move the machine options earlier, before any other emulated devices
> > > > options. Otherwise the selected PCI slot for the xen-platform device
> is
> > > > not available in QEMU.
> > > >
> > > > Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> > > > differently from xenfv, the other QEMU machines do not have that
> option
> > > > off by default.
> > > >
> > > > This patch does not change the emulated environment in the guest.
> > > >
> > > > Refer to this thread:
> http://marc.info/?l=xen-devel&m=140023775929625&w=2
> > >
> > > This takes me to "[Xen-devel] [v2][PATCH 4/8] xen, gfx passthrough:
> > > reserve 00:02.0 for INTEL IGD" which I can't see the link to (including
> > > via the 2 replies). Wrong link perhaps?
> > >
> > > A Message-Id is generally a better identifier since I can feed it to
> the
> > > archive of my choice and they aren't subject to e.g. future
> accidentally
> > > renumberings etc.
> >
> > 1400237624-8505-5-git-send-email-tiejun.chen@intel.com
>
> (pedantic: the <>s are formally part of the message ID...)
>
> OK, so I'm still not sure why this message is relevant to the sentence
> which precedes the link.
>
>
> > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > >
> > > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > > > index 8abed7b..fef684f 100644
> > > > --- a/tools/libxl/libxl_dm.c
> > > > +++ b/tools/libxl/libxl_dm.c
> > > > @@ -476,6 +476,29 @@ static char **
> libxl__build_device_model_args_new(libxl__gc *gc,
> > > >          flexarray_vappend(dm_args, "-k", keymap, NULL);
> > > >      }
> > > >
> > > > +    flexarray_append(dm_args, "-machine");
> > > > +    switch (b_info->type) {
> > > > +    case LIBXL_DOMAIN_TYPE_PV:
> > > > +        flexarray_append(dm_args, "xenpv");
> > > > +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] !=
> NULL; i++)
> > > > +            flexarray_append(dm_args, b_info->extra_pv[i]);
> > > > +        break;
> > > > +    case LIBXL_DOMAIN_TYPE_HVM:
> > > > +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> > > > +        flexarray_append(dm_args, "-global");
> > > > +        flexarray_append(dm_args,
> "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
> > >
> > > FWIW you can use flexarray_append_pair which more naturally gathers an
> > > option and its argument together (improving readability).
> > >
> > > (personally I think the -machine should be pulled into both cases using
> > > this method, but that's not a prereq for accepting the patch IMHO)
> > >
> > > Other than that the patch itself seems sound (although I dislike code
> > > motion mixed with real changes, I suppose this one is small enough that
> > > I can cope).
> >
> > Should I go ahead with these minor changes
>
> Please.
>
> >  and leave the soundhw problem
> > unsolved? I am not keen on fixing that.
> >
> > 1401803223.1582.5.camel@kazak.uk.xensource.com
>
> I came into this at the point where we were discussing how to support
> using -device for soundhw, but I missed the bit where it was explained
> why it was necessary to switch to that scheme.
>
> If this change leads to regressions where existing cfg files no longer
> work then we need to decide if that would be blocker for 4.5. I can't
> see why it wouldn't be, but it's up to you and Konrad to agree I
> suppose.
>

I think add new option with audio device select and check similar to vga
would be good but I'm afraid I do not have time to do it recently.
May still be other cases in addition to the audio to cause problems if you
put xen-platform's pci slot fixed, I have not tested further.
Complete the conversion to -device does not seem blocker, FWIK would be
blocking the disks part conversion with q35 cipset but is not
currently supported
by xen.

Sorry for my bad english.


>
> Ian.
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>

[-- Attachment #2: Type: text/html, Size: 9795 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Xen-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6
@ 2014-06-11 18:05         ` Fabio Fantoni
  0 siblings, 0 replies; 31+ messages in thread
From: Fabio Fantoni @ 2014-06-11 18:05 UTC (permalink / raw)
  To: Ian Campbell
  Cc: xen-devel, Konrad Rzeszutek Wilk, Stefano Stabellini,
	Ian Jackson, qemu-devel, Paul Durrant, Anthony PERARD,
	Tiejun Chen

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

2014-06-11 12:44 GMT+02:00 Ian Campbell <Ian.Campbell@citrix.com>:

> On Wed, 2014-06-11 at 11:35 +0100, Stefano Stabellini wrote:
> > On Tue, 10 Jun 2014, Ian Campbell wrote:
> > > On Fri, 2014-05-23 at 17:07 +0100, Stefano Stabellini wrote:
> > I think that the dependency on QEMU >= 1.6 is OK. The Xen PV Device went
> > in v1.6.0 and we certainy cannot go back to versions older than v1.5 due
> > to missing mapcache fixes. Fabio reported multiple times that QEMU 1.6
> > is the first stable version he tested.
>
> OK, then please can we update the README (and any other relevant docs)
> as part of this change.
>

I tested upstream qemu from 1.2 and based on my test at least qemu 1.6.1 is
needed to be fairly complete and stable on xen (no major or critical bugs), I
started using it in production for a few month, in the next few days I will
start to put also qemu 2.0 on a production system.
I want to be remembered to specify >=1.6.1 because 1.6.0 have critical bug
that that prevents all hvm domUs start.


>
> > > > Use pc-i440fx-1.6 regardless of the xen_platform_pci option. Add the
> > > > xen-platform device if requested. Choose slot 2 for the xen-platform
> > > > device for compatibility with current installations. In case of Intel
> > > > graphic passthrough, slot 2 might be needed by the grafic card.
> However
> > >
> > > "graphics"
> > >
> > > > now that we can specify the slot explicitly, it is easy to change the
> > > > position of the xen-platform device on the PCI bus if graphic
> > > > passthrough is enabled.
> > > >
> > > > Move the machine options earlier, before any other emulated devices
> > > > options. Otherwise the selected PCI slot for the xen-platform device
> is
> > > > not available in QEMU.
> > > >
> > > > Specify PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off, because
> > > > differently from xenfv, the other QEMU machines do not have that
> option
> > > > off by default.
> > > >
> > > > This patch does not change the emulated environment in the guest.
> > > >
> > > > Refer to this thread:
> http://marc.info/?l=xen-devel&m=140023775929625&w=2
> > >
> > > This takes me to "[Xen-devel] [v2][PATCH 4/8] xen, gfx passthrough:
> > > reserve 00:02.0 for INTEL IGD" which I can't see the link to (including
> > > via the 2 replies). Wrong link perhaps?
> > >
> > > A Message-Id is generally a better identifier since I can feed it to
> the
> > > archive of my choice and they aren't subject to e.g. future
> accidentally
> > > renumberings etc.
> >
> > 1400237624-8505-5-git-send-email-tiejun.chen@intel.com
>
> (pedantic: the <>s are formally part of the message ID...)
>
> OK, so I'm still not sure why this message is relevant to the sentence
> which precedes the link.
>
>
> > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > >
> > > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > > > index 8abed7b..fef684f 100644
> > > > --- a/tools/libxl/libxl_dm.c
> > > > +++ b/tools/libxl/libxl_dm.c
> > > > @@ -476,6 +476,29 @@ static char **
> libxl__build_device_model_args_new(libxl__gc *gc,
> > > >          flexarray_vappend(dm_args, "-k", keymap, NULL);
> > > >      }
> > > >
> > > > +    flexarray_append(dm_args, "-machine");
> > > > +    switch (b_info->type) {
> > > > +    case LIBXL_DOMAIN_TYPE_PV:
> > > > +        flexarray_append(dm_args, "xenpv");
> > > > +        for (i = 0; b_info->extra_pv && b_info->extra_pv[i] !=
> NULL; i++)
> > > > +            flexarray_append(dm_args, b_info->extra_pv[i]);
> > > > +        break;
> > > > +    case LIBXL_DOMAIN_TYPE_HVM:
> > > > +        flexarray_append(dm_args, "pc-i440fx-1.6,accel=xen");
> > > > +        flexarray_append(dm_args, "-global");
> > > > +        flexarray_append(dm_args,
> "PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off");
> > >
> > > FWIW you can use flexarray_append_pair which more naturally gathers an
> > > option and its argument together (improving readability).
> > >
> > > (personally I think the -machine should be pulled into both cases using
> > > this method, but that's not a prereq for accepting the patch IMHO)
> > >
> > > Other than that the patch itself seems sound (although I dislike code
> > > motion mixed with real changes, I suppose this one is small enough that
> > > I can cope).
> >
> > Should I go ahead with these minor changes
>
> Please.
>
> >  and leave the soundhw problem
> > unsolved? I am not keen on fixing that.
> >
> > 1401803223.1582.5.camel@kazak.uk.xensource.com
>
> I came into this at the point where we were discussing how to support
> using -device for soundhw, but I missed the bit where it was explained
> why it was necessary to switch to that scheme.
>
> If this change leads to regressions where existing cfg files no longer
> work then we need to decide if that would be blocker for 4.5. I can't
> see why it wouldn't be, but it's up to you and Konrad to agree I
> suppose.
>

I think add new option with audio device select and check similar to vga
would be good but I'm afraid I do not have time to do it recently.
May still be other cases in addition to the audio to cause problems if you
put xen-platform's pci slot fixed, I have not tested further.
Complete the conversion to -device does not seem blocker, FWIK would be
blocking the disks part conversion with q35 cipset but is not
currently supported
by xen.

Sorry for my bad english.


>
> Ian.
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>

[-- Attachment #2: Type: text/html, Size: 9795 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2014-06-11 18:05 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-23 16:07 [Qemu-devel] [PATCH] libxl: change default QEMU machine to pc-i440fx-1.6 Stefano Stabellini
2014-05-23 16:07 ` Stefano Stabellini
2014-05-23 17:08 ` [Qemu-devel] [Xen-devel] " Fabio Fantoni
2014-05-23 17:08   ` Fabio Fantoni
2014-05-25 14:14   ` [Qemu-devel] " Stefano Stabellini
2014-05-25 14:14     ` Stefano Stabellini
2014-05-26  8:00     ` [Qemu-devel] " Fabio Fantoni
2014-05-26  8:00       ` Fabio Fantoni
2014-05-28 14:45       ` Fabio Fantoni
2014-05-28 16:18         ` Stefano Stabellini
2014-05-28 16:41           ` Stefano Stabellini
2014-05-28 16:50             ` Paolo Bonzini
2014-05-28 16:59               ` Fabio Fantoni
2014-06-03 13:38               ` [Qemu-devel] " Stefano Stabellini
2014-06-03 13:38                 ` Stefano Stabellini
2014-06-03 13:43                 ` [Qemu-devel] " Paolo Bonzini
2014-06-03 13:43                   ` Paolo Bonzini
2014-06-03 13:47                 ` [Qemu-devel] " Ian Campbell
2014-06-03 13:47                   ` Ian Campbell
2014-06-03 14:03                 ` [Qemu-devel] " Fabio Fantoni
2014-06-03 14:03                   ` Fabio Fantoni
2014-05-28 16:53             ` [Xen-devel] " Fabio Fantoni
2014-05-29  8:08               ` Fabio Fantoni
2014-06-10 11:14 ` [Qemu-devel] " Ian Campbell
2014-06-10 11:14   ` Ian Campbell
2014-06-11 10:35   ` [Qemu-devel] " Stefano Stabellini
2014-06-11 10:35     ` Stefano Stabellini
2014-06-11 10:44     ` [Qemu-devel] " Ian Campbell
2014-06-11 10:44       ` Ian Campbell
2014-06-11 18:05       ` [Qemu-devel] [Xen-devel] " Fabio Fantoni
2014-06-11 18:05         ` Fabio Fantoni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.