Move HostdevHostSupportsPassthroughVFIO method to hypervisor to be shared between qemu and ch drivers. Signed-off-by: Praveen K Paladugu <prapal(a)linux.microsoft.com&gt; --- src/hypervisor/virhostdev.c | 16 ++++++++++++++++ src/hypervisor/virhostdev.h | 2 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hostdev.c | 15 --------------- src/qemu/qemu_hostdev.h | 2 -- tests/domaincapstest.c | 2 +- 9 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c index 4b06e74894..220578f0c0 100644 --- a/src/hypervisor/virhostdev.c +++ b/src/hypervisor/virhostdev.c @@ -29,12 +29,14 @@ #include "virhostdev.h" #include "viralloc.h" #include "virerror.h" +#include "virfile.h" #include "virlog.h" #include "virutil.h" #include "virnetdev.h" #include "configmake.h" #define VIR_FROM_THIS VIR_FROM_NONE +#define VIR_DEV_VFIO "/dev/vfio/vfio" VIR_LOG_INIT("util.hostdev"); @@ -2519,3 +2521,17 @@ virHostdevNeedsVFIO(const virDomainHostdevDef *hostdev) return virHostdevIsPCIDevice(hostdev) || virHostdevIsMdevDevice(hostdev); } + +bool +virHostdevHostSupportsPassthroughVFIO(void) +{ + /* condition 1 - host has IOMMU */ + if (!virHostHasIOMMU()) + return false; + + /* condition 2 - /dev/vfio/vfio exists */ + if (!virFileExists(VIR_DEV_VFIO)) + return false; + + return true; +} diff --git a/src/hypervisor/virhostdev.h b/src/hypervisor/virhostdev.h index b9e6108816..b7f8473560 100644 --- a/src/hypervisor/virhostdev.h +++ b/src/hypervisor/virhostdev.h @@ -235,3 +235,5 @@ virHostdevUpdateActiveNVMeDevices(virHostdevManager *hostdev_mgr, bool virHostdevNeedsVFIO(const virDomainHostdevDef *hostdev); + +bool virHostdevHostSupportsPassthroughVFIO(void); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cca071f866..39988cf0f4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1669,6 +1669,7 @@ virCloseCallbacksDomainRunForConn; # hypervisor/virhostdev.h virHostdevFindUSBDevice; +virHostdevHostSupportsPassthroughVFIO; virHostdevManagerGetDefault; virHostdevNeedsVFIO; virHostdevPCINodeDeviceDetach; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ac135aa301..e2f97c3d9a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -6378,7 +6378,7 @@ static void virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCaps *qemuCaps, virDomainCapsDeviceHostdev *hostdev) { - bool supportsPassthroughVFIO = qemuHostdevHostSupportsPassthroughVFIO(); + bool supportsPassthroughVFIO = virHostdevHostSupportsPassthroughVFIO(); hostdev->supported = VIR_TRISTATE_BOOL_YES; hostdev->mode.report = true; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fda4439b0b..065a37445e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11698,7 +11698,7 @@ static int qemuDomainPrepareHostdevPCI(virDomainHostdevDef *hostdev, virQEMUCaps *qemuCaps) { - bool supportsPassthroughVFIO = qemuHostdevHostSupportsPassthroughVFIO(); + bool supportsPassthroughVFIO = virHostdevHostSupportsPassthroughVFIO(); virDeviceHostdevPCIDriverName *driverName = &hostdev->source.subsys.u.pci.driver.name; /* assign defaults for hostdev passthrough */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f27c21ca8c..e0adebf7ea 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11482,7 +11482,7 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, * further validation until then. */ - if (!qemuHostdevHostSupportsPassthroughVFIO()) { + if (!virHostdevHostSupportsPassthroughVFIO()) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("VFIO device assignment is currently not supported on this system")); return -1; diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index f25ccaf1a4..ab2769d482 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -130,21 +130,6 @@ qemuHostdevUpdateActiveDomainDevices(virQEMUDriver *driver, } -bool -qemuHostdevHostSupportsPassthroughVFIO(void) -{ - /* condition 1 - host has IOMMU */ - if (!virHostHasIOMMU()) - return false; - - /* condition 2 - /dev/vfio/vfio exists */ - if (!virFileExists(QEMU_DEV_VFIO)) - return false; - - return true; -} - - int qemuHostdevPrepareOneNVMeDisk(virQEMUDriver *driver, const char *name, diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h index bbf7bb11e7..b6dd2e0207 100644 --- a/src/qemu/qemu_hostdev.h +++ b/src/qemu/qemu_hostdev.h @@ -23,8 +23,6 @@ #include "qemu_conf.h" -bool qemuHostdevHostSupportsPassthroughVFIO(void); - int qemuHostdevUpdateActiveNVMeDisks(virQEMUDriver *driver, virDomainDef *def); int qemuHostdevUpdateActiveMediatedDevices(virQEMUDriver *driver, diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index e557337617..e520c7d7bc 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -105,7 +105,7 @@ fillQemuCaps(virDomainCaps *domCaps, return -1; /* The function above tries to query host's VFIO capabilities by calling - * qemuHostdevHostSupportsPassthroughVFIO() which, however, can't be + * virHostdevHostSupportsPassthroughVFIO() which, however, can't be * successfully mocked as they are not exposed as internal APIs. Therefore, * instead of mocking set the expected values here by hand. */ VIR_DOMAIN_CAPS_ENUM_SET(domCaps->hostdev.pciBackend, -- 2.44.0

qemuHostdevPreparePCIDevices, virHostdevPreparePCIDevices are identical. Replacing qemu method with the generic one to reduce code duplication. This generic method will be used in ch driver to passthrough PCI devices. Signed-off-by: Praveen K Paladugu <prapal(a)linux.microsoft.com&gt; --- src/qemu/qemu_hostdev.c | 17 ++--------------- src/qemu/qemu_hostdev.h | 6 ------ src/qemu/qemu_hotplug.c | 5 +++-- 3 files changed, 5 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index ab2769d482..894f12e04f 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -152,20 +152,6 @@ qemuHostdevPrepareNVMeDisks(virQEMUDriver *driver, name, disks, ndisks); } -int -qemuHostdevPreparePCIDevices(virQEMUDriver *driver, - const char *name, - const unsigned char *uuid, - virDomainHostdevDef **hostdevs, - int nhostdevs, - unsigned int flags) -{ - return virHostdevPreparePCIDevices(driver->hostdevMgr, - QEMU_DRIVER_NAME, - name, uuid, hostdevs, - nhostdevs, flags); -} - int qemuHostdevPrepareUSBDevices(virQEMUDriver *driver, const char *name, @@ -244,7 +230,8 @@ qemuHostdevPrepareDomainDevices(virQEMUDriver *driver, if (qemuHostdevPrepareNVMeDisks(driver, def->name, def->disks, def->ndisks) < 0) return -1; - if (qemuHostdevPreparePCIDevices(driver, def->name, def->uuid, + if (virHostdevPreparePCIDevices(driver->hostdevMgr, QEMU_DRIVER_NAME, + def->name, def->uuid, def->hostdevs, def->nhostdevs, flags) < 0) return -1; diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h index b6dd2e0207..6f6c4f82bc 100644 --- a/src/qemu/qemu_hostdev.h +++ b/src/qemu/qemu_hostdev.h @@ -43,12 +43,6 @@ int qemuHostdevPrepareNVMeDisks(virQEMUDriver *driver, const char *name, virDomainDiskDef **disks, size_t ndisks); -int qemuHostdevPreparePCIDevices(virQEMUDriver *driver, - const char *name, - const unsigned char *uuid, - virDomainHostdevDef **hostdevs, - int nhostdevs, - unsigned int flags); int qemuHostdevPrepareUSBDevices(virQEMUDriver *driver, const char *name, virDomainHostdevDef **hostdevs, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 4d4bcde1bc..d1c042c6a0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1544,8 +1544,9 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver, if (!cfg->relaxedACS) flags |= VIR_HOSTDEV_STRICT_ACS_CHECK; - if (qemuHostdevPreparePCIDevices(driver, vm->def->name, - vm->def->uuid, &hostdev, 1, flags) < 0) + if (virHostdevPreparePCIDevices(driver->hostdevMgr, QEMU_DRIVER_NAME, + vm->def->name, vm->def->uuid, + &hostdev, 1, flags) < 0) return -1; if (qemuDomainAdjustMaxMemLockHostdev(vm, hostdev) < 0) -- 2.44.0

On 11/15/2024 6:22 AM, Michal Prívozník wrote: Understood, thanks for the feedback Michal. -- Regards, Praveen K Paladugu

From: Wei Liu <liuwe(a)microsoft.com&gt; They are left empty for now. Make sure CH driver still builds. Content will be filled in later. Signed-off-by: Wei Liu <liuwe(a)microsoft.com&gt; Signed-off-by: Praveen K Paladugu <prapal(a)linux.microsoft.com&gt; --- src/ch/ch_hostdev.c | 28 ++++++++++++++++++++++++++++ src/ch/ch_hostdev.h | 24 ++++++++++++++++++++++++ src/ch/meson.build | 2 ++ 3 files changed, 54 insertions(+) create mode 100644 src/ch/ch_hostdev.c create mode 100644 src/ch/ch_hostdev.h diff --git a/src/ch/ch_hostdev.c b/src/ch/ch_hostdev.c new file mode 100644 index 0000000000..20ce6efa10 --- /dev/null +++ b/src/ch/ch_hostdev.c @@ -0,0 +1,28 @@ +/* + * ch_hostdev.c: Cloud Hypervisor hostdev management + * + * Copyright (C) 2021 Wei Liu <liuwe(a)microsoft.com&gt; + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#include <config.h> + +#include "ch_hostdev.h" +#include "virlog.h" + +#define VIR_FROM_THIS VIR_FROM_CH + +VIR_LOG_INIT("ch.ch_hostdev"); diff --git a/src/ch/ch_hostdev.h b/src/ch/ch_hostdev.h new file mode 100644 index 0000000000..02b7f9c2d8 --- /dev/null +++ b/src/ch/ch_hostdev.h @@ -0,0 +1,24 @@ +/* + * ch_hostdev.h: Cloud Hypervisor hostdev management + * + * Copyright (C) 2021 Wei Liu <liuwe(a)microsoft.com&gt; + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#pragma once + +#include "ch_conf.h" +#include "domain_conf.h" diff --git a/src/ch/meson.build b/src/ch/meson.build index 633966aac7..ca1291c158 100644 --- a/src/ch/meson.build +++ b/src/ch/meson.build @@ -13,6 +13,8 @@ ch_driver_sources = [ 'ch_monitor.h', 'ch_process.c', 'ch_process.h', + 'ch_hostdev.c', + 'ch_hostdev.h', ] driver_source_files += files(ch_driver_sources) -- 2.44.0

Check if the domain definition is valid for PCI passthrough and update it if necessary. Signed-off-by: Praveen K Paladugu <prapal(a)linux.microsoft.com&gt; --- po/POTFILES | 1 + src/ch/ch_hostdev.c | 67 +++++++++++++++++++++++++++++++++++++++++++++ src/ch/ch_hostdev.h | 3 ++ src/ch/ch_process.c | 39 ++++++++++++++++++++++++++ 4 files changed, 110 insertions(+) diff --git a/po/POTFILES b/po/POTFILES index 1ed4086d2c..66465b798f 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -21,6 +21,7 @@ src/bhyve/bhyve_process.c src/ch/ch_conf.c src/ch/ch_domain.c src/ch/ch_driver.c +src/ch/ch_hostdev.c src/ch/ch_interface.c src/ch/ch_monitor.c src/ch/ch_process.c diff --git a/src/ch/ch_hostdev.c b/src/ch/ch_hostdev.c index 20ce6efa10..1e6210e162 100644 --- a/src/ch/ch_hostdev.c +++ b/src/ch/ch_hostdev.c @@ -26,3 +26,70 @@ #define VIR_FROM_THIS VIR_FROM_CH VIR_LOG_INIT("ch.ch_hostdev"); + +static int +virCHDomainPrepareHostdevPCI(virDomainHostdevDef *hostdev) +{ + bool supportsPassthroughVFIO = virHostdevHostSupportsPassthroughVFIO(); + virDeviceHostdevPCIDriverName *driverName = + &hostdev->source.subsys.u.pci.driver.name; + + /* assign defaults for hostdev passthrough */ + switch (*driverName) { + case VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_DEFAULT: + if (supportsPassthroughVFIO) { + *driverName = VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_VFIO; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("host doesn't support passthrough of host PCI devices")); + return -1; + } + break; + + case VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_VFIO: + if (!supportsPassthroughVFIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("host doesn't support VFIO PCI passthrough")); + return false; + } + break; + + case VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_KVM: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("host doesn't support legacy PCI passthrough")); + return false; + + case VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_XEN: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("CH does not support device assignment mode '%1$s'"), + virDeviceHostdevPCIDriverNameTypeToString(*driverName)); + return false; + + default: + case VIR_DEVICE_HOSTDEV_PCI_DRIVER_NAME_LAST: + virReportEnumRangeError(virDeviceHostdevPCIDriverName, *driverName); + break; + } + + return true; +} + +int +virCHDomainPrepareHostdev(virDomainHostdevDef *hostdev) +{ + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + return 0; + + switch (hostdev->source.subsys.type) { + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + return virCHDomainPrepareHostdevPCI(hostdev); + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST: + break; + } + + return 0; +} diff --git a/src/ch/ch_hostdev.h b/src/ch/ch_hostdev.h index 02b7f9c2d8..f9ba40ab71 100644 --- a/src/ch/ch_hostdev.h +++ b/src/ch/ch_hostdev.h @@ -22,3 +22,6 @@ #include "ch_conf.h" #include "domain_conf.h" + +int +virCHDomainPrepareHostdev(virDomainHostdevDef *hostdev); diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index 9816509e49..c5b5b6ebb2 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -37,6 +37,7 @@ #include "virnuma.h" #include "virstring.h" #include "ch_interface.h" +#include "ch_hostdev.h" #define VIR_FROM_THIS VIR_FROM_CH @@ -808,6 +809,40 @@ virCHProcessStartValidate(virCHDriver *driver, return 0; } +static int +virCHProcessPrepareDomainHostdevs(virDomainObj *vm) +{ + size_t i; + + for (i = 0; i < vm->def->nhostdevs; i++) { + virDomainHostdevDef *hostdev = vm->def->hostdevs[i]; + + if (virCHDomainPrepareHostdev(hostdev) < 0) + return -1; + } + + return 0; +} + +/** + * virCHProcessPrepareDomain: + * @vm: domain object + * + * This function groups all code that modifies only live XML of a domain which + * is about to start and it's the only place to do those modifications. + * + * This function MUST be called before virCHProcessPrepareHost(). + * + */ +static int +virCHProcessPrepareDomain(virDomainObj *vm) +{ + if (virCHProcessPrepareDomainHostdevs(vm) < 0) + return -1; + + return 0; +} + /** * virCHProcessStart: * @driver: pointer to driver structure @@ -839,6 +874,10 @@ virCHProcessStart(virCHDriver *driver, return -1; } + if (virCHProcessPrepareDomain(vm) < 0) { + return -1; + } + if (!priv->monitor) { /* And we can get the first monitor connection now too */ if (!(priv->monitor = virCHProcessConnectMonitor(driver, vm))) { -- 2.44.0

Allow hostdev configurations in ch guest definitions. Signed-off-by: Praveen K Paladugu <prapal(a)linux.microsoft.com&gt; --- src/ch/ch_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c index e1e14554a8..bfccabed49 100644 --- a/src/ch/ch_domain.c +++ b/src/ch/ch_domain.c @@ -158,6 +158,7 @@ chValidateDomainDeviceDef(const virDomainDeviceDef *dev, case VIR_DOMAIN_DEVICE_VSOCK: case VIR_DOMAIN_DEVICE_CONTROLLER: case VIR_DOMAIN_DEVICE_CHR: + case VIR_DOMAIN_DEVICE_HOSTDEV: break; case VIR_DOMAIN_DEVICE_LEASE: @@ -165,7 +166,6 @@ chValidateDomainDeviceDef(const virDomainDeviceDef *dev, case VIR_DOMAIN_DEVICE_INPUT: case VIR_DOMAIN_DEVICE_SOUND: case VIR_DOMAIN_DEVICE_VIDEO: - case VIR_DOMAIN_DEVICE_HOSTDEV: case VIR_DOMAIN_DEVICE_WATCHDOG: case VIR_DOMAIN_DEVICE_GRAPHICS: case VIR_DOMAIN_DEVICE_HUB: -- 2.44.0

While sending API requests that don't need any body, explicitly set CURLOPT_INFILESIZE to 0. Without this option, curl sends a chunked request with `Expect: 100-continue` header. The client, in this case curl, expects a response from the server, ch in this case, to respond within a timeout period. If guest definition has a PCI passthrough device configuration, cloud-hypervisor process cannot respond within above mentioned timeout. Even if cloud-hypervisor responds after the timeout, curl cannot read the response. Because of this, virsh request to create a guest, hangs. This only happens while using "mshv" hypervisor. By setting CURLOPT_INFILESIZE to O, curl drops the Expect header and sychronously waits for server to respond. Signed-off-by: Praveen K Paladugu <prapal(a)linux.microsoft.com&gt; --- src/ch/ch_monitor.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c index 3e49902791..e9b70abb69 100644 --- a/src/ch/ch_monitor.c +++ b/src/ch/ch_monitor.c @@ -684,6 +684,7 @@ virCHMonitorPutNoContent(virCHMonitor *mon, const char *endpoint) curl_easy_setopt(mon->handle, CURLOPT_URL, url); curl_easy_setopt(mon->handle, CURLOPT_UPLOAD, 1L); curl_easy_setopt(mon->handle, CURLOPT_HTTPHEADER, NULL); + curl_easy_setopt(mon->handle, CURLOPT_INFILESIZE, 0L); responseCode = virCHMonitorCurlPerform(mon->handle); -- 2.44.0