RFC: qemu and Xen ABI-unstable libs

RFC: qemu and Xen ABI-unstable libs

[Ian Jackson]

Hi all.  Michael Tokarev has been looking into the problem that qemu
is using Xen libraries with usntable ABIs.  We did an experiment to
see which abi-unstable symbols qemu links to, by suppressing libxc
from the link line.  The results are below.[1]

Things are not looking too bad.  After some discussion on #xendevel I
have tried to summarise the situation for each of the troublesome
symbols.

Also, we discovered that upstream qemu does not link against any
abi-unstable Xen libraries if PCI passthrough is disabled.

Please would my Xen colleages correct me if I have made any mistakes.
Michael, I hope this is helpful and clear.


In order from easy to hard:


xc_domain_shutdown

This call in qemu needs to be replaced with a call to the existing
function xendevicemodel_shutdown in libxendevicemodel.  I think it is
likely that this call is fixed in qemu upstream.


xc_get_hvm_param

There are three references in qemu's
xen_get_default_ioreq_server_info, relating to ioreq servers.  These
uses (and perhaps surrounding code at this function's call site)
should be replaced by use of xendevicemodel_create_ioreq_server
etc. from libxendevicemodel.  I think it is likely that this call is
fixed in qemu upstream.


xc_physdev_map_pirq
xc_physdev_map_pirq_msi
xc_physdev_unmap_pirq

These are all small wrappers for the PHYSDEVOP_map_pirq hypercall.
PHYSDEVOP is already reasonably abi-stable at the hypervisor level (in
theory it's versioned, but changing it would break all dom0's).
These calls could just be provided as-is by a new stable abi
entrypoint.  We think this should probably go in libxendevicemodel.

So, what's needed is to make Xen upstream change to add versions of
these three functions to tools/libs/devicemodel.  Change qemu to use
them.


xc_domain_iomem_permission
xc_domain_populate_physmap_exact
xc_domain_ioport_mapping
xc_domain_memory_mapping

The things done by these calls in qemu should be done by the Xen
toolstack (libxl), during domain creation etc., instead.

For at least some of them, there are patches on xen-devel, see
  From: Grzegorz Uriasz <gorbak25@gmail.com>
  Subject: [PATCH 1/3] tools/libxl: Grant VGA IO port permission for
   stubdom/target domain
  Date: Sun, 14 Jun 2020 23:12:01 +0100
et seq.  These patches have been reviewed and as far as I can tell
from the thread we are awaiting a resend.


xc_set_hvm_param

Two calls both relating to HVM_PARAM_ACPI_S_STATE.

These need to be turned into DMOP hypercalls (ie, new hypercalls added
to the hypervisor) and entrypoints provided in libxendevicemodel.


xc_domain_bind_pt_pci_irq
xc_domain_unbind_msi_irq
xc_domain_unbind_pt_irq
xc_domain_update_msi_irq

These are currently XEN_DOMCTL_* hypercalls.  These do not have a
stable ABI at the hypervisor interface.  AIUI Xen hypervisor folks
think they should be changed to use the DMOP or PHYSDEVOP hypercalls.

Additionally, we need calls for these in a userspace library with a
stable ABI.  We think that should be libxendevicemodel.

I think the userspace library part can go ahead right away: we can
change the implementation to use DMOP when the hypervisor work is
done.  In the meantime, the library would have a stable ABI for
callers, but the implementation would be tied to the hypervisor ABI.


xc_interface_close
xc_interface_open

When everything else is done, these calls will no longer be needed
because nothing will use the xc handle.

Ian.


[1]

/usr/bin/ld: accel/xen/xen-all.o: in function `xen_init':
/build/qemu/debian-qemu/accel/xen/xen-all.c:160: undefined reference to `xc_interface_open'
/usr/bin/ld: /build/qemu/debian-qemu/accel/xen/xen-all.c:175: undefined reference to `xc_interface_close'
/usr/bin/ld: /build/qemu/debian-qemu/accel/xen/xen-all.c:168: undefined reference to `xc_interface_close'
/usr/bin/ld: hw/xen/xen_pt.o: in function `xen_pt_destroy':
/build/qemu/debian-qemu/hw/xen/xen_pt.c:725: undefined reference to `xc_domain_unbind_pt_irq'
/usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt.c:751: undefined reference to `xc_physdev_unmap_pirq'
/usr/bin/ld: hw/xen/xen_pt.o: in function `xen_pt_realize':
/build/qemu/debian-qemu/hw/xen/xen_pt.c:866: undefined reference to `xc_physdev_map_pirq'
/usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt.c:885: undefined reference to `xc_domain_bind_pt_pci_irq'
/usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt.c:898: undefined reference to `xc_physdev_unmap_pirq'
/usr/bin/ld: hw/xen/xen_pt.o: in function `xen_pt_region_update':
/build/qemu/debian-qemu/hw/xen/xen_pt.c:631: undefined reference to `xc_domain_ioport_mapping'
/usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt.c:643: undefined reference to `xc_domain_memory_mapping'
/usr/bin/ld: hw/xen/xen_pt_graphics.o: in function `xen_pt_register_vga_regions':
/build/qemu/debian-qemu/hw/xen/xen_pt_graphics.c:68: undefined reference to `xc_domain_memory_mapping'
/usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt_graphics.c:63: undefined reference to `xc_domain_ioport_mapping'
/usr/bin/ld: hw/xen/xen_pt_graphics.o: in function `xen_pt_unregister_vga_regions':
/build/qemu/debian-qemu/hw/xen/xen_pt_graphics.c:104: undefined reference to `xc_domain_memory_mapping'
/usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt_graphics.c:99: undefined reference to `xc_domain_ioport_mapping'
/usr/bin/ld: hw/xen/xen_pt_graphics.o: in function `igd_write_opregion':
/build/qemu/debian-qemu/hw/xen/xen_pt_graphics.c:260: undefined reference to `xc_domain_iomem_permission'
/usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt_graphics.c:273: undefined reference to `xc_domain_memory_mapping'
/usr/bin/ld: hw/xen/xen_pt_graphics.o: in function `xen_pt_unregister_vga_regions':
/build/qemu/debian-qemu/hw/xen/xen_pt_graphics.c:119: undefined reference to `xc_domain_memory_mapping'
/usr/bin/ld: hw/xen/xen_pt_msi.o: in function `msi_msix_disable':
/build/qemu/debian-qemu/hw/xen/xen_pt_msi.c:213: undefined reference to `xc_domain_unbind_msi_irq'
/usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt_msi.c:222: undefined reference to `xc_physdev_unmap_pirq'
/usr/bin/ld: hw/xen/xen_pt_msi.o: in function `msi_msix_setup':
/build/qemu/debian-qemu/hw/xen/xen_pt_msi.c:138: undefined reference to `xc_physdev_map_pirq_msi'
/usr/bin/ld: hw/xen/xen_pt_msi.o: in function `msi_msix_update':
/build/qemu/debian-qemu/hw/xen/xen_pt_msi.c:178: undefined reference to `xc_domain_update_msi_irq'
/usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt_msi.c:185: undefined reference to `xc_physdev_unmap_pirq'
/usr/bin/ld: hw/xen/xen_pt_msi.o: in function `xen_pt_msix_update_remap':
/build/qemu/debian-qemu/hw/xen/xen_pt_msi.c:415: undefined reference to `xc_domain_unbind_pt_irq'
/usr/bin/ld: hw/i386/xen/xen-hvm.o: in function `xen_ram_alloc':
/build/qemu/debian-qemu/hw/i386/xen/xen-hvm.c:290: undefined reference to `xc_domain_populate_physmap_exact'
/usr/bin/ld: hw/i386/xen/xen-hvm.o: in function `xen_get_default_ioreq_server_info':
/build/qemu/debian-qemu/include/hw/xen/xen_common.h:395: undefined reference to `xc_get_hvm_param'
/usr/bin/ld: /build/qemu/debian-qemu/include/hw/xen/xen_common.h:403: undefined reference to `xc_get_hvm_param'
/usr/bin/ld: /build/qemu/debian-qemu/include/hw/xen/xen_common.h:411: undefined reference to `xc_get_hvm_param'
/usr/bin/ld: hw/i386/xen/xen-hvm.o: in function `destroy_hvm_domain':
/build/qemu/debian-qemu/hw/i386/xen/xen-hvm.c:1551: undefined reference to `xc_interface_open'
/usr/bin/ld: /build/qemu/debian-qemu/hw/i386/xen/xen-hvm.c:1555: undefined reference to `xc_domain_shutdown'
/usr/bin/ld: hw/i386/xen/xen-hvm.o: in function `xen_wakeup_notifier':
/build/qemu/debian-qemu/hw/i386/xen/xen-hvm.c:1317: undefined reference to `xc_set_hvm_param'
/usr/bin/ld: hw/i386/xen/xen-hvm.o: in function `xen_suspend_notifier':
/build/qemu/debian-qemu/hw/i386/xen/xen-hvm.c:183: undefined reference to `xc_set_hvm_param'
/usr/bin/ld: hw/i386/xen/xen-hvm.o: in function `destroy_hvm_domain':
/build/qemu/debian-qemu/hw/i386/xen/xen-hvm.c:1564: undefined reference to `xc_interface_close'
/usr/bin/ld: /build/qemu/debian-qemu/hw/i386/xen/xen-hvm.c:1564: undefined reference to `xc_interface_close'
collect2: error: ld returned 1 exit status


List:
xc_domain_bind_pt_pci_irq
xc_domain_iomem_permission
xc_domain_ioport_mapping
xc_domain_memory_mapping
xc_domain_populate_physmap_exact
xc_domain_shutdown
xc_domain_unbind_msi_irq
xc_domain_unbind_pt_irq
xc_domain_update_msi_irq
xc_get_hvm_param
xc_interface_close
xc_interface_open
xc_physdev_map_pirq
xc_physdev_map_pirq_msi
xc_physdev_unmap_pirq
xc_set_hvm_param

RE: qemu and Xen ABI-unstable libs

[Paul Durrant]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Ian Jackson
> Sent: 18 September 2020 17:39
> To: Debian folks: Michael Tokarev <mjt@tls.msk.ru>; Hans van Kranenburg <hans@knorrie.org>; Xen
> upstream folks with an interest: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monné
> <roger.pau@citrix.com>
> Cc: pkg-xen-devel@lists.alioth.debian.org; xen-devel@lists.xenproject.org; My Xen upstream tools co-
> maintainer: Wei Liu <wl@xen.org>
> Subject: RFC: qemu and Xen ABI-unstable libs
> 
> Hi all.  Michael Tokarev has been looking into the problem that qemu
> is using Xen libraries with usntable ABIs.  We did an experiment to
> see which abi-unstable symbols qemu links to, by suppressing libxc
> from the link line.  The results are below.[1]
> 
> Things are not looking too bad.  After some discussion on #xendevel I
> have tried to summarise the situation for each of the troublesome
> symbols.
> 
> Also, we discovered that upstream qemu does not link against any
> abi-unstable Xen libraries if PCI passthrough is disabled.
> 
> Please would my Xen colleages correct me if I have made any mistakes.
> Michael, I hope this is helpful and clear.
> 
> 
> In order from easy to hard:
> 
> 
> xc_domain_shutdown
> 
> This call in qemu needs to be replaced with a call to the existing
> function xendevicemodel_shutdown in libxendevicemodel.  I think it is
> likely that this call is fixed in qemu upstream.
> 

I just pulled QEMU master and it appears that destroy_hvm_domain() is still calling xc_domain_shutdown().

> 
> xc_get_hvm_param
> 
> There are three references in qemu's
> xen_get_default_ioreq_server_info, relating to ioreq servers.  These
> uses (and perhaps surrounding code at this function's call site)
> should be replaced by use of xendevicemodel_create_ioreq_server
> etc. from libxendevicemodel.  I think it is likely that this call is
> fixed in qemu upstream.
> 

These references are in compat code for Xen < 4.6. Use of (non-default) ioreq server has been present in the code for a long time.
We can remove them by retiring the compat code.

> 
> xc_physdev_map_pirq
> xc_physdev_map_pirq_msi
> xc_physdev_unmap_pirq
> 
> These are all small wrappers for the PHYSDEVOP_map_pirq hypercall.
> PHYSDEVOP is already reasonably abi-stable at the hypervisor level (in
> theory it's versioned, but changing it would break all dom0's).

The hypercalls are non-tools and directly called from the Linux kernel code so they are ABI.

> These calls could just be provided as-is by a new stable abi
> entrypoint.  We think this should probably go in libxendevicemodel.
> 

Rather than simply moving this calls into libxendevicemodel, we should think about their interactions with calls such as
xc_domain_bind_pt_pci_irq() below and maybe have a stable library that actually provides a better API/ABI for interrupt
mapping/triggering although... I've long felt PCI pass-through should not be done by QEMU anyway (not least because we currently
have no mechanism for PCI pass-through to PVH domains).

> So, what's needed is to make Xen upstream change to add versions of
> these three functions to tools/libs/devicemodel.  Change qemu to use
> them.
> 
> 
> xc_domain_iomem_permission
> xc_domain_populate_physmap_exact
> xc_domain_ioport_mapping
> xc_domain_memory_mapping
> 
> The things done by these calls in qemu should be done by the Xen
> toolstack (libxl), during domain creation etc., instead.

I don't think that is practical. E.g. if a guest re-programs a PCI I/O BAR then it may necessitate re-calling
xc_domain_ioport_mapping(); the tool-stack cannot know a priori where PCI BARs will end up in guest port/memory space.

> 
> For at least some of them, there are patches on xen-devel, see
>   From: Grzegorz Uriasz <gorbak25@gmail.com>
>   Subject: [PATCH 1/3] tools/libxl: Grant VGA IO port permission for
>    stubdom/target domain
>   Date: Sun, 14 Jun 2020 23:12:01 +0100
> et seq.  These patches have been reviewed and as far as I can tell
> from the thread we are awaiting a resend.
> 

For legacy ranges, such as VGA, it is practical.

> 
> xc_set_hvm_param
> 
> Two calls both relating to HVM_PARAM_ACPI_S_STATE.
> 
> These need to be turned into DMOP hypercalls (ie, new hypercalls added
> to the hypervisor) and entrypoints provided in libxendevicemodel.
> 

Yes, this is certainly a candidate for a DM op.

> 
> xc_domain_bind_pt_pci_irq
> xc_domain_unbind_msi_irq
> xc_domain_unbind_pt_irq
> xc_domain_update_msi_irq
> 
> These are currently XEN_DOMCTL_* hypercalls.  These do not have a
> stable ABI at the hypervisor interface.  AIUI Xen hypervisor folks
> think they should be changed to use the DMOP or PHYSDEVOP hypercalls.
> 
> Additionally, we need calls for these in a userspace library with a
> stable ABI.  We think that should be libxendevicemodel.
> 

What I said above: This needs more consideration.

A while ago I hacked together xenpt (https://xenbits.xen.org/gitweb/?p=people/pauldu/xenpt.git), a stand-alone PCI pass-through
emulator. One option would be to get this into shape and pull it into the Xen tool-stack. This would facilitate removal of the PCI
pass-through code from QEMU and hence removal of use of unstable interfaces.

  Paul

> I think the userspace library part can go ahead right away: we can
> change the implementation to use DMOP when the hypervisor work is
> done.  In the meantime, the library would have a stable ABI for
> callers, but the implementation would be tied to the hypervisor ABI.
> 
> 
> xc_interface_close
> xc_interface_open
> 
> When everything else is done, these calls will no longer be needed
> because nothing will use the xc handle.
> 
> Ian.
> 
> 
> [1]
> 
> /usr/bin/ld: accel/xen/xen-all.o: in function `xen_init':
> /build/qemu/debian-qemu/accel/xen/xen-all.c:160: undefined reference to `xc_interface_open'
> /usr/bin/ld: /build/qemu/debian-qemu/accel/xen/xen-all.c:175: undefined reference to
> `xc_interface_close'
> /usr/bin/ld: /build/qemu/debian-qemu/accel/xen/xen-all.c:168: undefined reference to
> `xc_interface_close'
> /usr/bin/ld: hw/xen/xen_pt.o: in function `xen_pt_destroy':
> /build/qemu/debian-qemu/hw/xen/xen_pt.c:725: undefined reference to `xc_domain_unbind_pt_irq'
> /usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt.c:751: undefined reference to
> `xc_physdev_unmap_pirq'
> /usr/bin/ld: hw/xen/xen_pt.o: in function `xen_pt_realize':
> /build/qemu/debian-qemu/hw/xen/xen_pt.c:866: undefined reference to `xc_physdev_map_pirq'
> /usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt.c:885: undefined reference to
> `xc_domain_bind_pt_pci_irq'
> /usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt.c:898: undefined reference to
> `xc_physdev_unmap_pirq'
> /usr/bin/ld: hw/xen/xen_pt.o: in function `xen_pt_region_update':
> /build/qemu/debian-qemu/hw/xen/xen_pt.c:631: undefined reference to `xc_domain_ioport_mapping'
> /usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt.c:643: undefined reference to
> `xc_domain_memory_mapping'
> /usr/bin/ld: hw/xen/xen_pt_graphics.o: in function `xen_pt_register_vga_regions':
> /build/qemu/debian-qemu/hw/xen/xen_pt_graphics.c:68: undefined reference to `xc_domain_memory_mapping'
> /usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt_graphics.c:63: undefined reference to
> `xc_domain_ioport_mapping'
> /usr/bin/ld: hw/xen/xen_pt_graphics.o: in function `xen_pt_unregister_vga_regions':
> /build/qemu/debian-qemu/hw/xen/xen_pt_graphics.c:104: undefined reference to
> `xc_domain_memory_mapping'
> /usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt_graphics.c:99: undefined reference to
> `xc_domain_ioport_mapping'
> /usr/bin/ld: hw/xen/xen_pt_graphics.o: in function `igd_write_opregion':
> /build/qemu/debian-qemu/hw/xen/xen_pt_graphics.c:260: undefined reference to
> `xc_domain_iomem_permission'
> /usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt_graphics.c:273: undefined reference to
> `xc_domain_memory_mapping'
> /usr/bin/ld: hw/xen/xen_pt_graphics.o: in function `xen_pt_unregister_vga_regions':
> /build/qemu/debian-qemu/hw/xen/xen_pt_graphics.c:119: undefined reference to
> `xc_domain_memory_mapping'
> /usr/bin/ld: hw/xen/xen_pt_msi.o: in function `msi_msix_disable':
> /build/qemu/debian-qemu/hw/xen/xen_pt_msi.c:213: undefined reference to `xc_domain_unbind_msi_irq'
> /usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt_msi.c:222: undefined reference to
> `xc_physdev_unmap_pirq'
> /usr/bin/ld: hw/xen/xen_pt_msi.o: in function `msi_msix_setup':
> /build/qemu/debian-qemu/hw/xen/xen_pt_msi.c:138: undefined reference to `xc_physdev_map_pirq_msi'
> /usr/bin/ld: hw/xen/xen_pt_msi.o: in function `msi_msix_update':
> /build/qemu/debian-qemu/hw/xen/xen_pt_msi.c:178: undefined reference to `xc_domain_update_msi_irq'
> /usr/bin/ld: /build/qemu/debian-qemu/hw/xen/xen_pt_msi.c:185: undefined reference to
> `xc_physdev_unmap_pirq'
> /usr/bin/ld: hw/xen/xen_pt_msi.o: in function `xen_pt_msix_update_remap':
> /build/qemu/debian-qemu/hw/xen/xen_pt_msi.c:415: undefined reference to `xc_domain_unbind_pt_irq'
> /usr/bin/ld: hw/i386/xen/xen-hvm.o: in function `xen_ram_alloc':
> /build/qemu/debian-qemu/hw/i386/xen/xen-hvm.c:290: undefined reference to
> `xc_domain_populate_physmap_exact'
> /usr/bin/ld: hw/i386/xen/xen-hvm.o: in function `xen_get_default_ioreq_server_info':
> /build/qemu/debian-qemu/include/hw/xen/xen_common.h:395: undefined reference to `xc_get_hvm_param'
> /usr/bin/ld: /build/qemu/debian-qemu/include/hw/xen/xen_common.h:403: undefined reference to
> `xc_get_hvm_param'
> /usr/bin/ld: /build/qemu/debian-qemu/include/hw/xen/xen_common.h:411: undefined reference to
> `xc_get_hvm_param'
> /usr/bin/ld: hw/i386/xen/xen-hvm.o: in function `destroy_hvm_domain':
> /build/qemu/debian-qemu/hw/i386/xen/xen-hvm.c:1551: undefined reference to `xc_interface_open'
> /usr/bin/ld: /build/qemu/debian-qemu/hw/i386/xen/xen-hvm.c:1555: undefined reference to
> `xc_domain_shutdown'
> /usr/bin/ld: hw/i386/xen/xen-hvm.o: in function `xen_wakeup_notifier':
> /build/qemu/debian-qemu/hw/i386/xen/xen-hvm.c:1317: undefined reference to `xc_set_hvm_param'
> /usr/bin/ld: hw/i386/xen/xen-hvm.o: in function `xen_suspend_notifier':
> /build/qemu/debian-qemu/hw/i386/xen/xen-hvm.c:183: undefined reference to `xc_set_hvm_param'
> /usr/bin/ld: hw/i386/xen/xen-hvm.o: in function `destroy_hvm_domain':
> /build/qemu/debian-qemu/hw/i386/xen/xen-hvm.c:1564: undefined reference to `xc_interface_close'
> /usr/bin/ld: /build/qemu/debian-qemu/hw/i386/xen/xen-hvm.c:1564: undefined reference to
> `xc_interface_close'
> collect2: error: ld returned 1 exit status
> 
> 
> List:
> xc_domain_bind_pt_pci_irq
> xc_domain_iomem_permission
> xc_domain_ioport_mapping
> xc_domain_memory_mapping
> xc_domain_populate_physmap_exact
> xc_domain_shutdown
> xc_domain_unbind_msi_irq
> xc_domain_unbind_pt_irq
> xc_domain_update_msi_irq
> xc_get_hvm_param
> xc_interface_close
> xc_interface_open
> xc_physdev_map_pirq
> xc_physdev_map_pirq_msi
> xc_physdev_unmap_pirq
> xc_set_hvm_param
> 
> 

Re: qemu and Xen ABI-unstable libs

[Jan Beulich]

On 21.09.2020 09:36, Paul Durrant wrote:
>> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Ian Jackson
>> Sent: 18 September 2020 17:39
>>
>> xc_domain_iomem_permission
>> xc_domain_populate_physmap_exact
>> xc_domain_ioport_mapping
>> xc_domain_memory_mapping
>>
>> The things done by these calls in qemu should be done by the Xen
>> toolstack (libxl), during domain creation etc., instead.
> 
> I don't think that is practical. E.g. if a guest re-programs a PCI I/O BAR then it may necessitate re-calling
> xc_domain_ioport_mapping(); the tool-stack cannot know a priori where PCI BARs will end up in guest port/memory space.

In your reply I assume you meant just the latter two of the four?
For these I agree, and as a result they shouldn't be domctl in
the new model.

Jan

RE: qemu and Xen ABI-unstable libs

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 21 September 2020 10:41
> To: paul@xen.org
> Cc: 'Ian Jackson' <iwj@xenproject.org>; 'Debian folks: Michael Tokarev' <mjt@tls.msk.ru>; 'Hans van
> Kranenburg' <hans@knorrie.org>; 'Xen upstream folks with an interest: Andrew Cooper'
> <Andrew.Cooper3@citrix.com>; 'Roger Pau Monné' <roger.pau@citrix.com>; pkg-xen-
> devel@lists.alioth.debian.org; xen-devel@lists.xenproject.org; 'My Xen upstream tools co-maintainer:
> Wei Liu' <wl@xen.org>
> Subject: Re: qemu and Xen ABI-unstable libs
> 
> On 21.09.2020 09:36, Paul Durrant wrote:
> >> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Ian Jackson
> >> Sent: 18 September 2020 17:39
> >>
> >> xc_domain_iomem_permission
> >> xc_domain_populate_physmap_exact
> >> xc_domain_ioport_mapping
> >> xc_domain_memory_mapping
> >>
> >> The things done by these calls in qemu should be done by the Xen
> >> toolstack (libxl), during domain creation etc., instead.
> >
> > I don't think that is practical. E.g. if a guest re-programs a PCI I/O BAR then it may necessitate
> re-calling
> > xc_domain_ioport_mapping(); the tool-stack cannot know a priori where PCI BARs will end up in guest
> port/memory space.
> 
> In your reply I assume you meant just the latter two of the four?
> For these I agree, and as a result they shouldn't be domctl in
> the new model.
> 

Sorry if I wasn't clear. Yes, the latter two are what I was referring to.

  Paul

> Jan

Re: qemu and Xen ABI-unstable libs

[Roger Pau Monné]

On Mon, Sep 21, 2020 at 08:36:55AM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Ian Jackson
> > Sent: 18 September 2020 17:39
> > To: Debian folks: Michael Tokarev <mjt@tls.msk.ru>; Hans van Kranenburg <hans@knorrie.org>; Xen
> > upstream folks with an interest: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monné
> > <roger.pau@citrix.com>
> > Cc: pkg-xen-devel@lists.alioth.debian.org; xen-devel@lists.xenproject.org; My Xen upstream tools co-
> > maintainer: Wei Liu <wl@xen.org>
> > Subject: RFC: qemu and Xen ABI-unstable libs
> > 
> > Hi all.  Michael Tokarev has been looking into the problem that qemu
> > is using Xen libraries with usntable ABIs.  We did an experiment to
> > see which abi-unstable symbols qemu links to, by suppressing libxc
> > from the link line.  The results are below.[1]
> > 
> > Things are not looking too bad.  After some discussion on #xendevel I
> > have tried to summarise the situation for each of the troublesome
> > symbols.
> > 
> > Also, we discovered that upstream qemu does not link against any
> > abi-unstable Xen libraries if PCI passthrough is disabled.
> > 
> > Please would my Xen colleages correct me if I have made any mistakes.
> > Michael, I hope this is helpful and clear.
> > 
> > 
> > In order from easy to hard:
> > 
> > 
> > xc_domain_shutdown
> > 
> > This call in qemu needs to be replaced with a call to the existing
> > function xendevicemodel_shutdown in libxendevicemodel.  I think it is
> > likely that this call is fixed in qemu upstream.
> > 
> 
> I just pulled QEMU master and it appears that destroy_hvm_domain() is still calling xc_domain_shutdown().
> 
> > 
> > xc_get_hvm_param
> > 
> > There are three references in qemu's
> > xen_get_default_ioreq_server_info, relating to ioreq servers.  These
> > uses (and perhaps surrounding code at this function's call site)
> > should be replaced by use of xendevicemodel_create_ioreq_server
> > etc. from libxendevicemodel.  I think it is likely that this call is
> > fixed in qemu upstream.
> > 
> 
> These references are in compat code for Xen < 4.6. Use of (non-default) ioreq server has been present in the code for a long time.
> We can remove them by retiring the compat code.
> 
> > 
> > xc_physdev_map_pirq
> > xc_physdev_map_pirq_msi
> > xc_physdev_unmap_pirq
> > 
> > These are all small wrappers for the PHYSDEVOP_map_pirq hypercall.
> > PHYSDEVOP is already reasonably abi-stable at the hypervisor level (in
> > theory it's versioned, but changing it would break all dom0's).
> 
> The hypercalls are non-tools and directly called from the Linux kernel code so they are ABI.
> 
> > These calls could just be provided as-is by a new stable abi
> > entrypoint.  We think this should probably go in libxendevicemodel.
> > 
> 
> Rather than simply moving this calls into libxendevicemodel, we should think about their interactions with calls such as
> xc_domain_bind_pt_pci_irq() below and maybe have a stable library that actually provides a better API/ABI for interrupt
> mapping/triggering although...

I've thought the same when speaking with Ian about this, as (for HVM
passthrough) we use the physdev op to obtain a pirq from a physical
device interrupt source (a MSI entry in the QEMU case, because the
legacy interrupt is bound by the toolstack IIRC) and then use that
pirq to bind it to a guest lapic vector.

I think in a sense such physical interrupt abstraction (the pirq) is
helpful in order to simplify the binding, as you don't end up with a
hypercall with a massive number of parameters to identify both the
source and destination interrupt data. It's also helpful when the
guest changes the interrupt binding, as you then only update the guest
side and keep using the same pirq.

We might want however to have an interface more specific to
passthrough, such that the pirqs (or maybe we could just call them
handles) returned by such interface can only be used with guest
specific bind hypercalls?

> I've long felt PCI pass-through should not be done by QEMU anyway (not least because we currently
> have no mechanism for PCI pass-through to PVH domains).

Having xenpt in tree would be fine IMO. Now we have all the proper
infrastructure in place to allow different pci devices to be handled
by different emulators IIRC, which is all that's required for this to
work correctly.

Thanks, Roger.

RE: qemu and Xen ABI-unstable libs

[Paul Durrant]

> -----Original Message-----
> From: Roger Pau Monné <roger.pau@citrix.com>
> Sent: 21 September 2020 11:16
> To: paul@xen.org
> Cc: 'Ian Jackson' <iwj@xenproject.org>; 'Debian folks: Michael Tokarev' <mjt@tls.msk.ru>; 'Hans van
> Kranenburg' <hans@knorrie.org>; 'Xen upstream folks with an interest: Andrew Cooper'
> <Andrew.Cooper3@citrix.com>; pkg-xen-devel@lists.alioth.debian.org; xen-devel@lists.xenproject.org;
> 'My Xen upstream tools co-maintainer: Wei Liu' <wl@xen.org>
> Subject: Re: qemu and Xen ABI-unstable libs
> 
> On Mon, Sep 21, 2020 at 08:36:55AM +0100, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Ian Jackson
> > > Sent: 18 September 2020 17:39
> > > To: Debian folks: Michael Tokarev <mjt@tls.msk.ru>; Hans van Kranenburg <hans@knorrie.org>; Xen
> > > upstream folks with an interest: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monné
> > > <roger.pau@citrix.com>
> > > Cc: pkg-xen-devel@lists.alioth.debian.org; xen-devel@lists.xenproject.org; My Xen upstream tools
> co-
> > > maintainer: Wei Liu <wl@xen.org>
> > > Subject: RFC: qemu and Xen ABI-unstable libs
> > >
> > > Hi all.  Michael Tokarev has been looking into the problem that qemu
> > > is using Xen libraries with usntable ABIs.  We did an experiment to
> > > see which abi-unstable symbols qemu links to, by suppressing libxc
> > > from the link line.  The results are below.[1]
> > >
> > > Things are not looking too bad.  After some discussion on #xendevel I
> > > have tried to summarise the situation for each of the troublesome
> > > symbols.
> > >
> > > Also, we discovered that upstream qemu does not link against any
> > > abi-unstable Xen libraries if PCI passthrough is disabled.
> > >
> > > Please would my Xen colleages correct me if I have made any mistakes.
> > > Michael, I hope this is helpful and clear.
> > >
> > >
> > > In order from easy to hard:
> > >
> > >
> > > xc_domain_shutdown
> > >
> > > This call in qemu needs to be replaced with a call to the existing
> > > function xendevicemodel_shutdown in libxendevicemodel.  I think it is
> > > likely that this call is fixed in qemu upstream.
> > >
> >
> > I just pulled QEMU master and it appears that destroy_hvm_domain() is still calling
> xc_domain_shutdown().
> >
> > >
> > > xc_get_hvm_param
> > >
> > > There are three references in qemu's
> > > xen_get_default_ioreq_server_info, relating to ioreq servers.  These
> > > uses (and perhaps surrounding code at this function's call site)
> > > should be replaced by use of xendevicemodel_create_ioreq_server
> > > etc. from libxendevicemodel.  I think it is likely that this call is
> > > fixed in qemu upstream.
> > >
> >
> > These references are in compat code for Xen < 4.6. Use of (non-default) ioreq server has been
> present in the code for a long time.
> > We can remove them by retiring the compat code.
> >
> > >
> > > xc_physdev_map_pirq
> > > xc_physdev_map_pirq_msi
> > > xc_physdev_unmap_pirq
> > >
> > > These are all small wrappers for the PHYSDEVOP_map_pirq hypercall.
> > > PHYSDEVOP is already reasonably abi-stable at the hypervisor level (in
> > > theory it's versioned, but changing it would break all dom0's).
> >
> > The hypercalls are non-tools and directly called from the Linux kernel code so they are ABI.
> >
> > > These calls could just be provided as-is by a new stable abi
> > > entrypoint.  We think this should probably go in libxendevicemodel.
> > >
> >
> > Rather than simply moving this calls into libxendevicemodel, we should think about their
> interactions with calls such as
> > xc_domain_bind_pt_pci_irq() below and maybe have a stable library that actually provides a better
> API/ABI for interrupt
> > mapping/triggering although...
> 
> I've thought the same when speaking with Ian about this, as (for HVM
> passthrough) we use the physdev op to obtain a pirq from a physical
> device interrupt source (a MSI entry in the QEMU case, because the
> legacy interrupt is bound by the toolstack IIRC) and then use that
> pirq to bind it to a guest lapic vector.
> 
> I think in a sense such physical interrupt abstraction (the pirq) is
> helpful in order to simplify the binding, as you don't end up with a
> hypercall with a massive number of parameters to identify both the
> source and destination interrupt data. It's also helpful when the
> guest changes the interrupt binding, as you then only update the guest
> side and keep using the same pirq.
> 
> We might want however to have an interface more specific to
> passthrough, such that the pirqs (or maybe we could just call them
> handles) returned by such interface can only be used with guest
> specific bind hypercalls?

Yes, that sounds sensible: we have functions to allocare/free a pirq handle and then functions to bind/unbind such a handle to guest vector.

> 
> > I've long felt PCI pass-through should not be done by QEMU anyway (not least because we currently
> > have no mechanism for PCI pass-through to PVH domains).
> 
> Having xenpt in tree would be fine IMO. Now we have all the proper
> infrastructure in place to allow different pci devices to be handled
> by different emulators IIRC, which is all that's required for this to
> work correctly.

To be useful we need a way of selecting pass-through emulator in the toolstack and we also need to re-visit moving the PCI hotplug controller implementation into Xen (and augmenting the ioreq server API to add some sort of unplug request callback). I'll give it some thought.

  Paul

> 
> Thanks, Roger.