RE: qemu and Xen ABI-unstable libs

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Ian 
> Jackson
> Sent: 18 September 2020 17:39
> To: Debian folks: Michael Tokarev <mjt@xxxxxxxxxx>; Hans van Kranenburg 
> <hans@xxxxxxxxxxx>; Xen
> upstream folks with an interest: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; 
> Roger Pau Monné
> <roger.pau@xxxxxxxxxx>
> Cc: pkg-xen-devel@xxxxxxxxxxxxxxxxxxxxxxx; xen-devel@xxxxxxxxxxxxxxxxxxxx; My 
> Xen upstream tools co-
> maintainer: Wei Liu <wl@xxxxxxx>
> 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@xxxxxxxxx>
>   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
> 
>