diff mbox

libxl: change default QEMU machine to pc-i440fx-1.6

Message ID alpine.DEB.2.02.1405231555560.14596@kaball.uk.xensource.com
State New
Headers show

Commit Message

Stefano Stabellini May 23, 2014, 4:07 p.m. UTC
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>

Comments

Fabio Fantoni May 23, 2014, 5:08 p.m. UTC | #1
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
Stefano Stabellini May 25, 2014, 2:14 p.m. UTC | #2
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.
Fabio Fantoni May 26, 2014, 8 a.m. UTC | #3
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.
Fabio Fantoni May 28, 2014, 2:45 p.m. UTC | #4
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.
Stefano Stabellini May 28, 2014, 4:18 p.m. UTC | #5
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!
Stefano Stabellini May 28, 2014, 4:41 p.m. UTC | #6
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.
Paolo Bonzini May 28, 2014, 4:50 p.m. UTC | #7
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
Fabio Fantoni May 28, 2014, 4:53 p.m. UTC | #8
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.
Fabio Fantoni May 28, 2014, 4:59 p.m. UTC | #9
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.
Fabio Fantoni May 29, 2014, 8:08 a.m. UTC | #10
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.
Stefano Stabellini June 3, 2014, 1:38 p.m. UTC | #11
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?
Paolo Bonzini June 3, 2014, 1:43 p.m. UTC | #12
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
Ian Campbell June 3, 2014, 1:47 p.m. UTC | #13
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.
Fabio Fantoni June 3, 2014, 2:03 p.m. UTC | #14
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.
Ian Campbell June 10, 2014, 11:14 a.m. UTC | #15
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.
Stefano Stabellini June 11, 2014, 10:35 a.m. UTC | #16
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
Ian Campbell June 11, 2014, 10:44 a.m. UTC | #17
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.
Fabio Fantoni June 11, 2014, 6:05 p.m. UTC | #18
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
>
diff mbox

Patch

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