[ PATCH vf-token 0/8] Introduce vf-token when using userspace PF
The VFIO PCI ABI has been extended to require userspace PF driver to set a VF token to a known value. The VF drivers are then required to provide this token to access the VF device. The vf-token is set by the PF driver before VF drivers can access the device. The kernel provides no means to retrieve the token in use; but there is no specification describing the distribution or level of confidentiality of the token. Qemu has been extended to require the vf-token when vf device is used. An important point to note is that the vf-token is required only when both the PF and VF are used in userspace. This patch series adds support to provide the vf-token (uuid format) in the domain XML and to generate the qemu commandline including the vf-token. To support vf-token the new element will be used as follows: <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> <address domain='0x0000' bus='0x0' slot='0x00' function='0x1'> <vf-token uuid='00112233-4455-6677-8899-aabbccddeeff'/> </address> </source> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </hostdev> The generated commandline will include the following: -device {"driver":"vfio-pci","host":"0000:00:0.1", "vf-token":"00112233-4455-6677-8899-aabbccddeeff", "id":"hostdev0","bus":"pci.0","addr":"0x1"} Changes since initial RFC based on review comments received: 1. Added documentation 2. Added test cases and ran successful test suite after each patch commit 3. fixed spaces, coding sytle, and uuid string format 4. Used S:vftoken in virJSONValueObjectAdd instead of a conditional Vivek Kashyap (8): Define the vf-token extension for PCI device Introduce the vf-token qemu capability This patch introduces the PCI address extension flag for vf-token This patch introduces new XML parser/formatter functions for parsing the vf-token Introduce a validation function for vf-token support in qemu and generate vf-token device attribute in qemu command line Provide information about the vf-token flag Add tests for the vf-token flag to the qemuxml2argv and qemuxml2xml test suites Update news about vf-token NEWS.rst | 8 +++ docs/formatdomain.rst | 3 ++ src/conf/device_conf.c | 49 ++++++++++++++++--- src/conf/domain_addr.h | 1 + src/conf/domain_conf.c | 8 +++ src/conf/schemas/basictypes.rng | 7 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 3 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 8 +++ src/qemu/qemu_domain_address.c | 3 ++ src/qemu/qemu_validate.c | 20 ++++++++ src/util/virpci.c | 7 +++ src/util/virpci.h | 10 ++++ .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + .../caps_8.1.0_x86_64.xml | 1 + .../caps_8.2.0_x86_64.xml | 1 + .../hostdev-vfio-vf-token.x86_64-latest.args | 34 +++++++++++++ .../hostdev-vfio-vf-token.xml | 22 +++++++++ tests/qemuxml2argvtest.c | 1 + .../hostdev-vfio-vf-token.x86_64-latest.xml | 40 +++++++++++++++ tests/qemuxml2xmltest.c | 1 + 22 files changed, 223 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-vf-token.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-vf-token.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-vf-token.x86_64-latest.xml -- 2.33.8
Define the vf-token extension for PCI device Signed-off-by: Vivek Kashyap <vivek.kashyap@linux.intel.com> --- src/util/virpci.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/util/virpci.h b/src/util/virpci.h index bc7cb2329f..da32c2f4d2 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -50,6 +50,13 @@ struct _virZPCIDeviceAddress { /* Don't forget to update virPCIDeviceAddressCopy if needed. */ }; +typedef struct _virPCIDeviceToken virPCIDeviceToken; + +struct _virPCIDeviceToken { + unsigned char uuid[VIR_UUID_BUFLEN]; + bool isSet; +}; + struct _virPCIDeviceAddress { unsigned int domain; unsigned int bus; @@ -58,6 +65,7 @@ struct _virPCIDeviceAddress { virTristateSwitch multi; int extFlags; /* enum virPCIDeviceAddressExtensionFlags */ virZPCIDeviceAddress zpci; + virPCIDeviceToken token; /* Don't forget to update virPCIDeviceAddressCopy if needed. */ }; -- 2.33.8
Introduce the vf-token qemu capability Signed-off-by: Vivek Kashyap <vivek.kashyap@linux.intel.com> --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml | 1 + 5 files changed, 7 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 83119e871a..f4cacd48d0 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -698,6 +698,7 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 450 */ "run-with.async-teardown", /* QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN */ "virtio-blk-vhost-vdpa", /* QEMU_CAPS_DEVICE_VIRTIO_BLK_VHOST_VDPA */ + "vf-token", /* QEMU_CAPS_VFIO_VFTOKEN */ ); @@ -1385,6 +1386,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "virtio-crypto-device", QEMU_CAPS_DEVICE_VIRTIO_CRYPTO }, { "cryptodev-backend-lkcf", QEMU_CAPS_OBJECT_CRYPTO_LKCF }, { "pvpanic-pci", QEMU_CAPS_DEVICE_PANIC_PCI }, + { "vf-token", QEMU_CAPS_VFIO_VFTOKEN }, }; @@ -1447,6 +1449,7 @@ static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVirtioSCSI[] = { }; static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVfioPCI[] = { + { "vf-token", QEMU_CAPS_VFIO_VFTOKEN, NULL }, }; static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsSCSIDisk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 3c4f7f625b..f97b1c9fd5 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -677,6 +677,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 450 */ QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN, /* asynchronous teardown -run-with async-teardown=on|off */ QEMU_CAPS_DEVICE_VIRTIO_BLK_VHOST_VDPA, /* virtio-blk-vhost-vdpa block driver */ + QEMU_CAPS_VFIO_VFTOKEN, /* vf-token support */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml index 427ee9d5c7..f4a65a133f 100644 --- a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml +++ b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml @@ -112,6 +112,7 @@ <flag name='rbd-encryption-layering'/> <flag name='rbd-encryption-luks-any'/> <flag name='run-with.async-teardown'/> + <flag name='vf-token'/> <version>8000050</version> <microcodeVersion>39100245</microcodeVersion> <package>v8.0.0-1270-g1c12355b</package> diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml index d266dd0f31..202a2c7f8d 100644 --- a/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml @@ -198,6 +198,7 @@ <flag name='qcow2-discard-no-unref'/> <flag name='run-with.async-teardown'/> <flag name='virtio-blk-vhost-vdpa'/> + <flag name='vf-token'/> <version>8001000</version> <microcodeVersion>43100245</microcodeVersion> <package>v8.1.0</package> diff --git a/tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml index ef3bd14597..24809ab70f 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_x86_64.xml @@ -199,6 +199,7 @@ <flag name='qcow2-discard-no-unref'/> <flag name='run-with.async-teardown'/> <flag name='virtio-blk-vhost-vdpa'/> + <flag name='vf-token'/> <version>8001050</version> <microcodeVersion>43100246</microcodeVersion> <package>v8.1.0-3111-gad6ef0a42e</package> -- 2.33.8
This patch introduces the PCI address extension flag for vf-token Signed-off-by: Vivek Kashyap <vivek.kashyap@linux.intel.com> --- src/conf/domain_addr.h | 1 + src/qemu/qemu_domain_address.c | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index e72fb48847..29e7257177 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -29,6 +29,7 @@ typedef enum { VIR_PCI_ADDRESS_EXTENSION_NONE = 0, /* no extension */ VIR_PCI_ADDRESS_EXTENSION_ZPCI = 1 << 0, /* zPCI support */ + VIR_PCI_ADDRESS_EXTENSION_VFTOKEN = 1 << 1, /* VF token support */ } virPCIDeviceAddressExtensionFlags; typedef enum { diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 099778b2a8..3be5acbc9e 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -575,6 +575,9 @@ qemuDomainDeviceCalculatePCIAddressExtensionFlags(virQEMUCaps *qemuCaps, extFlags |= VIR_PCI_ADDRESS_EXTENSION_ZPCI; } + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VFIO_VFTOKEN)) + extFlags |= VIR_PCI_ADDRESS_EXTENSION_VFTOKEN; + return extFlags; } -- 2.33.8
This patch introduces new XML parser/formatter functions for parsing the vf-token Signed-off-by: Vivek Kashyap <vivek.kashyap@linux.intel.com> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> --- src/conf/device_conf.c | 49 ++++++++++++++++++++++++++++++++++------ src/conf/domain_conf.c | 8 +++++++ src/libvirt_private.syms | 1 + src/util/virpci.c | 7 ++++++ src/util/virpci.h | 2 ++ 5 files changed, 60 insertions(+), 7 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index f3d977f2b7..f490aeef9a 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -70,6 +70,21 @@ virZPCIDeviceAddressParseXML(xmlNodePtr node, return 0; } + +static int +virPCIDeviceTokenParseXML(xmlNodePtr node, + virPCIDeviceAddress *addr) +{ + if (virXMLPropUUID(node, "uuid", VIR_XML_PROP_NONE, + addr->token.uuid) < 0) + return -1; + + addr->token.isSet = 1; + + return 0; +} + + void virDomainDeviceInfoClear(virDomainDeviceInfo *info) { @@ -200,6 +215,7 @@ virPCIDeviceAddressParseXML(xmlNodePtr node, virPCIDeviceAddress *addr) { xmlNodePtr zpci; + xmlNodePtr token; memset(addr, 0, sizeof(*addr)); @@ -231,6 +247,11 @@ virPCIDeviceAddressParseXML(xmlNodePtr node, return -1; } + if ((token = virXMLNodeGetSubelement(node, "vf-token"))) { + if (virPCIDeviceTokenParseXML(token, addr) < 0) + return -1; + } + return 0; } @@ -239,13 +260,27 @@ virPCIDeviceAddressFormat(virBuffer *buf, virPCIDeviceAddress addr, bool includeTypeInAddr) { - virBufferAsprintf(buf, "<address %sdomain='0x%04x' bus='0x%02x' " - "slot='0x%02x' function='0x%d'/>\n", - includeTypeInAddr ? "type='pci' " : "", - addr.domain, - addr.bus, - addr.slot, - addr.function); + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) tokenBuf = VIR_BUFFER_INIT_CHILD(buf); + virBuffer *tb = NULL; + + virBufferAsprintf(&attrBuf, " %sdomain='0x%04x' bus='0x%02x' " + "slot='0x%02x' function='0x%d'", + includeTypeInAddr ? "type='pci' " : "", + addr.domain, + addr.bus, + addr.slot, + addr.function); + + if (virPCIVFIOTokenIDIsPresent(&addr.token)) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virBufferAsprintf(&tokenBuf, "<vf-token uuid='%s'/>\n", + virUUIDFormat(addr.token.uuid, uuidstr)); + tb = &tokenBuf; + } + + virXMLFormatElement(buf, "address", &attrBuf, tb); } int diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 22ad43e1d7..8bda81815a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5403,6 +5403,14 @@ virDomainDeviceInfoFormat(virBuffer *buf, info->addr.pci.zpci.uid.value, info->addr.pci.zpci.fid.value); } + + if (virPCIVFIOTokenIDIsPresent(&info->addr.pci.token)) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virBufferAsprintf(&childBuf, "<vf-token uuid='%s'/>\n", + virUUIDFormat(info->addr.pci.token.uuid, + uuidstr)); + } break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE: diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 31c0f169c3..b2bc26c323 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3138,6 +3138,7 @@ virPCIHeaderTypeToString; virPCIIsVirtualFunction; virPCIStubDriverTypeFromString; virPCIStubDriverTypeToString; +virPCIVFIOTokenIDIsPresent; virPCIVirtualFunctionListFree; virZPCIDeviceAddressIsIncomplete; virZPCIDeviceAddressIsPresent; diff --git a/src/util/virpci.c b/src/util/virpci.c index afce7b52b7..0a9ae7a881 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2314,6 +2314,13 @@ virZPCIDeviceAddressIsPresent(const virZPCIDeviceAddress *addr) } +bool +virPCIVFIOTokenIDIsPresent(const virPCIDeviceToken *token) +{ + return token->isSet; +} + + void virPCIVirtualFunctionListFree(virPCIVirtualFunctionList *list) { diff --git a/src/util/virpci.h b/src/util/virpci.h index da32c2f4d2..8510752e84 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -271,6 +271,8 @@ int virPCIDeviceAddressParse(char *address, virPCIDeviceAddress *bdf); bool virZPCIDeviceAddressIsIncomplete(const virZPCIDeviceAddress *addr); bool virZPCIDeviceAddressIsPresent(const virZPCIDeviceAddress *addr); +bool virPCIVFIOTokenIDIsPresent(const virPCIDeviceToken *token); + int virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path, int pfNetDevIdx, char **pfname, -- 2.33.8
Introduce a validation function for vf-token support in qemu and generate vf-token device attribute in qemu command line Signed-off-by: Vivek Kashyap <vivek.kashyap@linux.intel.com> --- src/qemu/qemu_command.c | 8 ++++++++ src/qemu/qemu_validate.c | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 54fb8220e8..0e81a3ed73 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4706,6 +4706,7 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def, virDomainNetTeamingInfo *teaming; g_autofree char *host = virPCIDeviceAddressAsString(&pcisrc->addr); const char *failover_pair_id = NULL; + g_autofree char *token = NULL; /* caller has to assign proper passthrough backend type */ switch (pcisrc->backend) { @@ -4732,9 +4733,16 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def, teaming->persistent) failover_pair_id = teaming->persistent; + if ((dev->info->pciAddrExtFlags & VIR_PCI_ADDRESS_EXTENSION_VFTOKEN) && + pcisrc->addr.token.isSet) { + token = g_new0(char, VIR_UUID_STRING_BUFLEN); + virUUIDFormat(pcisrc->addr.token.uuid, token); + } + if (virJSONValueObjectAdd(&props, "s:driver", "vfio-pci", "s:host", host, + "S:vf-token", token, "s:id", dev->info->alias, "p:bootindex", dev->info->effectiveBootIndex, "S:failover_pair_id", failover_pair_id, diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index e475ad035e..13114ca3d1 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -1364,6 +1364,24 @@ qemuValidateDomainDeviceDefZPCIAddress(virDomainDeviceInfo *info, } +static int +qemuValidateDomainDeviceDefVFTokenId(virDomainDeviceInfo *info, + virQEMUCaps *qemuCaps) +{ + virPCIDeviceToken *vftoken = &info->addr.pci.token; + + if (virPCIVFIOTokenIDIsPresent(vftoken) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VFIO_VFTOKEN)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", + _("This QEMU binary doesn't support vf token ids")); + return -1; + } + + return 0; +} + + static int qemuValidateDomainDeviceDefAddressDrive(virDomainDeviceInfo *info, const virDomainDef *def, @@ -1483,6 +1501,8 @@ qemuValidateDomainDeviceDefAddress(const virDomainDeviceDef *dev, case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: if (qemuValidateDomainDeviceDefZPCIAddress(info, qemuCaps) < 0) return -1; + if (qemuValidateDomainDeviceDefVFTokenId(info, qemuCaps) < 0) + return -1; break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE: -- 2.33.8
Provide information about the vf-token flag Signed-off-by: Vivek Kashyap <vivek.kashyap@linux.intel.com> --- docs/formatdomain.rst | 3 +++ src/conf/schemas/basictypes.rng | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 310d2bc427..29a7b3145e 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -3744,6 +3744,9 @@ control where on the bus the device will be placed: between 0x0001 and 0xffff, inclusive), and ``fid`` (a hex value between 0x00000000 and 0xffffffff, inclusive) used by PCI devices on S390 for User-defined Identifiers and Function Identifiers. + The ``vf-token`` element is supported in uuid format. The vf-token is a + shared secret between userspace vfio-pci PF driver and VF driver. The + token is set by the PF driver, and must be provided for VF access. :since:`Since 1.3.5` , some hypervisor drivers may accept an ``<address type='pci'/>`` element with no other attributes as an explicit request to assign a PCI address for the device rather than some other type of diff --git a/src/conf/schemas/basictypes.rng b/src/conf/schemas/basictypes.rng index 26eb538077..bbb7484430 100644 --- a/src/conf/schemas/basictypes.rng +++ b/src/conf/schemas/basictypes.rng @@ -121,6 +121,13 @@ <ref name="virOnOff"/> </attribute> </optional> + <optional> + <element name="vf-token"> + <attribute name="uuid"> + <ref name="UUID"/> + </attribute> + </element> + </optional> </define> <define name="zpciaddress"> <optional> -- 2.33.8
Add tests for the vf-token flag to the qemuxml2argv and qemuxml2xml test suites Signed-off-by: Vivek Kashyap <vivek.kashyap@linux.intel.com> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com> --- .../hostdev-vfio-vf-token.x86_64-latest.args | 34 ++++++++++++++++ .../hostdev-vfio-vf-token.xml | 22 ++++++++++ tests/qemuxml2argvtest.c | 1 + .../hostdev-vfio-vf-token.x86_64-latest.xml | 40 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 5 files changed, 98 insertions(+) create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-vf-token.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-vf-token.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-vf-token.x86_64-latest.xml diff --git a/tests/qemuxml2argvdata/hostdev-vfio-vf-token.x86_64-latest.args b/tests/qemuxml2argvdata/hostdev-vfio-vf-token.x86_64-latest.args new file mode 100644 index 0000000000..e449c84ea9 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-vfio-vf-token.x86_64-latest.args @@ -0,0 +1,34 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \ +-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \ +-accel tcg \ +-cpu qemu64 \ +-m size=219136k \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-device '{"driver":"vfio-pci","host":"0000:00:00.0","vf-token":"00112233-4455-6677-8899-aabbccddeeff","id":"hostdev0","bus":"pci.0","addr":"0x8"}' \ +-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x2"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/hostdev-vfio-vf-token.xml b/tests/qemuxml2argvdata/hostdev-vfio-vf-token.xml new file mode 100644 index 0000000000..87dddd762a --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-vfio-vf-token.xml @@ -0,0 +1,22 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pci-root'/> + <hostdev mode='subsystem' type='pci' managed='no'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x00' slot='0x00' function='0x0'> + <vf-token uuid='00112233-4455-6677-8899-aabbccddeeff'/> + </address> + </source> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + </hostdev> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b2ea2191dc..20bc914748 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1547,6 +1547,7 @@ mymain(void) DO_TEST_CAPS_LATEST_PARSE_ERROR("hostdev-pci-duplicate"); DO_TEST_CAPS_LATEST("hostdev-vfio"); DO_TEST_CAPS_LATEST("hostdev-vfio-multidomain"); + DO_TEST_CAPS_LATEST("hostdev-vfio-vf-token"); DO_TEST_CAPS_LATEST("hostdev-mdev-precreated"); DO_TEST_CAPS_LATEST_PARSE_ERROR("hostdev-mdev-src-address-invalid"); DO_TEST_CAPS_LATEST_PARSE_ERROR("hostdev-mdev-invalid-target-address"); diff --git a/tests/qemuxml2xmloutdata/hostdev-vfio-vf-token.x86_64-latest.xml b/tests/qemuxml2xmloutdata/hostdev-vfio-vf-token.x86_64-latest.xml new file mode 100644 index 0000000000..65c4fc6a4a --- /dev/null +++ b/tests/qemuxml2xmloutdata/hostdev-vfio-vf-token.x86_64-latest.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>qemu64</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='pci' index='0' model='pci-root'/> + <controller type='usb' index='0' model='piix3-uhci'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <audio id='1' type='none'/> + <hostdev mode='subsystem' type='pci' managed='no'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x00' slot='0x00' function='0x0'> + <vf-token uuid='00112233-4455-6677-8899-aabbccddeeff'/> + </address> + </source> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + </hostdev> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 1010b68ebc..4b2c0e980a 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -432,6 +432,7 @@ mymain(void) DO_TEST_CAPS_LATEST("hostdev-pci-address-unassigned"); DO_TEST_CAPS_LATEST("hostdev-pci-multifunction"); DO_TEST_CAPS_LATEST("hostdev-vfio"); + DO_TEST_CAPS_LATEST("hostdev-vfio-vf-token"); DO_TEST_CAPS_ARCH_LATEST("hostdev-vfio-zpci", "s390x"); DO_TEST_CAPS_ARCH_LATEST("hostdev-vfio-zpci-multidomain-many", "s390x"); DO_TEST_CAPS_ARCH_LATEST("hostdev-vfio-zpci-autogenerate", "s390x"); -- 2.33.8
Update news about vf-token Signed-off-by: Vivek Kashyap <vivek.kashyap@linux.intel.com> --- NEWS.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index dc40602c72..5e6a7c3147 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,6 +17,14 @@ v10.0.0 (unreleased) * **New features** + * qemu: support VF tokens for vfio-pci + + "vf-token",implemented as a UUID is part of VFIO PCI ABI, and acts as + a shared key between vfio PF and VF drivers. The token is set by the + PF driver and the VF driver provides it to access the VF. The + vfio vf-token uuid is included in the VM XML specification for the pci + device, and the token is passed in qemu commandline on VM launch. + * **Improvements** * **Bug fixes** -- 2.33.8
On Tue, 2 Jan 2024 18:55:10 +0530 Vivek Kashyap <vivek.kashyap@linux.intel.com> wrote:
The VFIO PCI ABI has been extended to require userspace PF driver to set a VF token to a known value. The VF drivers are then required to provide this token to access the VF device. The vf-token is set by the PF driver before VF drivers can access the device. The kernel provides no means to retrieve the token in use; but there is no specification describing the distribution or level of confidentiality of the token.
At the same time, both the kernel and this series indicate the token is a shared secret, which is the reason the kernel provides no means to access the token. Is it reasonable to have a secret shared token in xml, logs, and QEMU command line? The token is shared between all VFs associated to a PF, so as this support more formally moves from a QEMU one-off hack to libvirt support, are we revealing a secret by promoting this model? Furthermore, libvirt has always been able to consider the vfio-pci device trusted, at least so far as it's provided by a kernel driver. With VF token support, the VF driver itself may still be a kernel driver, but the PF is managed via a userspace driver with unknown capabilities relative to the integrity of the VF device. I don't know if we need to or how we take into account that lack of device authentication. Certainly without some degree of attestation of the PF driver and VF token, or potentially a mechanism for a more cryptographic statement of trust, such a device ought not to be involved with a confidential VM. I'm not sure what needs to be done here, maybe the device level trust is a problem for a higher level management tool, but I'd like to take a more thoughtful look at the implications of VF token support as we move up the stack rather than position this as simply filling a gap in QEMU vfio-pci support. Thanks, Alex
Qemu has been extended to require the vf-token when vf device is used. An important point to note is that the vf-token is required only when both the PF and VF are used in userspace.
This patch series adds support to provide the vf-token (uuid format) in the domain XML and to generate the qemu commandline including the vf-token.
To support vf-token the new element will be used as follows: <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> <address domain='0x0000' bus='0x0' slot='0x00' function='0x1'> <vf-token uuid='00112233-4455-6677-8899-aabbccddeeff'/> </address> </source> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </hostdev>
The generated commandline will include the following:
-device {"driver":"vfio-pci","host":"0000:00:0.1", "vf-token":"00112233-4455-6677-8899-aabbccddeeff", "id":"hostdev0","bus":"pci.0","addr":"0x1"}
Changes since initial RFC based on review comments received: 1. Added documentation 2. Added test cases and ran successful test suite after each patch commit 3. fixed spaces, coding sytle, and uuid string format 4. Used S:vftoken in virJSONValueObjectAdd instead of a conditional
Vivek Kashyap (8): Define the vf-token extension for PCI device Introduce the vf-token qemu capability This patch introduces the PCI address extension flag for vf-token This patch introduces new XML parser/formatter functions for parsing the vf-token Introduce a validation function for vf-token support in qemu and generate vf-token device attribute in qemu command line Provide information about the vf-token flag Add tests for the vf-token flag to the qemuxml2argv and qemuxml2xml test suites Update news about vf-token
NEWS.rst | 8 +++ docs/formatdomain.rst | 3 ++ src/conf/device_conf.c | 49 ++++++++++++++++--- src/conf/domain_addr.h | 1 + src/conf/domain_conf.c | 8 +++ src/conf/schemas/basictypes.rng | 7 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 3 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 8 +++ src/qemu/qemu_domain_address.c | 3 ++ src/qemu/qemu_validate.c | 20 ++++++++ src/util/virpci.c | 7 +++ src/util/virpci.h | 10 ++++ .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + .../caps_8.1.0_x86_64.xml | 1 + .../caps_8.2.0_x86_64.xml | 1 + .../hostdev-vfio-vf-token.x86_64-latest.args | 34 +++++++++++++ .../hostdev-vfio-vf-token.xml | 22 +++++++++ tests/qemuxml2argvtest.c | 1 + .../hostdev-vfio-vf-token.x86_64-latest.xml | 40 +++++++++++++++ tests/qemuxml2xmltest.c | 1 + 22 files changed, 223 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-vf-token.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-vf-token.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-vf-token.x86_64-latest.xml
On Tue, 2 Jan 2024, Alex Williamson wrote:
On Tue, 2 Jan 2024 18:55:10 +0530 Vivek Kashyap <vivek.kashyap@linux.intel.com> wrote:
The VFIO PCI ABI has been extended to require userspace PF driver to set a VF token to a known value. The VF drivers are then required to provide this token to access the VF device. The vf-token is set by the PF driver before VF drivers can access the device. The kernel provides no means to retrieve the token in use; but there is no specification describing the distribution or level of confidentiality of the token.
At the same time, both the kernel and this series indicate the token is a shared secret, which is the reason the kernel provides no means to access the token. Is it reasonable to have a secret shared token in xml, logs, and QEMU command line? The token is shared between all VFs associated to a PF, so as this support more formally moves from a QEMU one-off hack to libvirt support, are we revealing a secret by promoting this model?
The level of confidentiality has been left open in the vf-token implementation across kernel and qemu. Perhaps we need to find a way to allow a higher level solution to restrict/tighten it further based on a policy. I don't immediately have a suggestion on how. Until then the problem is that we are unable to launch the VMs when the PF is in the uesrspace. For now this patch is only bridging the gap to qemu commandline. -vk
Furthermore, libvirt has always been able to consider the vfio-pci device trusted, at least so far as it's provided by a kernel driver. With VF token support, the VF driver itself may still be a kernel driver, but the PF is managed via a userspace driver with unknown capabilities relative to the integrity of the VF device. I don't know if we need to or how we take into account that lack of device authentication. Certainly without some degree of attestation of the PF driver and VF token, or potentially a mechanism for a more cryptographic statement of trust, such a device ought not to be involved with a confidential VM.
I'm not sure what needs to be done here, maybe the device level trust is a problem for a higher level management tool, but I'd like to take a more thoughtful look at the implications of VF token support as we move up the stack rather than position this as simply filling a gap in QEMU vfio-pci support. Thanks,
Alex
Qemu has been extended to require the vf-token when vf device is used. An important point to note is that the vf-token is required only when both the PF and VF are used in userspace.
This patch series adds support to provide the vf-token (uuid format) in the domain XML and to generate the qemu commandline including the vf-token.
To support vf-token the new element will be used as follows: <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> <address domain='0x0000' bus='0x0' slot='0x00' function='0x1'> <vf-token uuid='00112233-4455-6677-8899-aabbccddeeff'/> </address> </source> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </hostdev>
The generated commandline will include the following:
-device {"driver":"vfio-pci","host":"0000:00:0.1", "vf-token":"00112233-4455-6677-8899-aabbccddeeff", "id":"hostdev0","bus":"pci.0","addr":"0x1"}
Changes since initial RFC based on review comments received: 1. Added documentation 2. Added test cases and ran successful test suite after each patch commit 3. fixed spaces, coding sytle, and uuid string format 4. Used S:vftoken in virJSONValueObjectAdd instead of a conditional
Vivek Kashyap (8): Define the vf-token extension for PCI device Introduce the vf-token qemu capability This patch introduces the PCI address extension flag for vf-token This patch introduces new XML parser/formatter functions for parsing the vf-token Introduce a validation function for vf-token support in qemu and generate vf-token device attribute in qemu command line Provide information about the vf-token flag Add tests for the vf-token flag to the qemuxml2argv and qemuxml2xml test suites Update news about vf-token
NEWS.rst | 8 +++ docs/formatdomain.rst | 3 ++ src/conf/device_conf.c | 49 ++++++++++++++++--- src/conf/domain_addr.h | 1 + src/conf/domain_conf.c | 8 +++ src/conf/schemas/basictypes.rng | 7 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 3 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 8 +++ src/qemu/qemu_domain_address.c | 3 ++ src/qemu/qemu_validate.c | 20 ++++++++ src/util/virpci.c | 7 +++ src/util/virpci.h | 10 ++++ .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + .../caps_8.1.0_x86_64.xml | 1 + .../caps_8.2.0_x86_64.xml | 1 + .../hostdev-vfio-vf-token.x86_64-latest.args | 34 +++++++++++++ .../hostdev-vfio-vf-token.xml | 22 +++++++++ tests/qemuxml2argvtest.c | 1 + .../hostdev-vfio-vf-token.x86_64-latest.xml | 40 +++++++++++++++ tests/qemuxml2xmltest.c | 1 + 22 files changed, 223 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-vf-token.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-vf-token.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-vf-token.x86_64-latest.xml
On Wed, Jan 03, 2024 at 22:42:41 +0530, Vivek Kashyap wrote:
On Tue, 2 Jan 2024, Alex Williamson wrote:
On Tue, 2 Jan 2024 18:55:10 +0530 Vivek Kashyap <vivek.kashyap@linux.intel.com> wrote:
The VFIO PCI ABI has been extended to require userspace PF driver to set a VF token to a known value. The VF drivers are then required to provide this token to access the VF device. The vf-token is set by the PF driver before VF drivers can access the device. The kernel provides no means to retrieve the token in use; but there is no specification describing the distribution or level of confidentiality of the token.
At the same time, both the kernel and this series indicate the token is a shared secret, which is the reason the kernel provides no means to access the token. Is it reasonable to have a secret shared token in xml, logs, and QEMU command line? The token is shared between all VFs associated to a PF, so as this support more formally moves from a QEMU one-off hack to libvirt support, are we revealing a secret by promoting this model?
The level of confidentiality has been left open in the vf-token implementation across kernel and qemu. Perhaps we need to find a way to allow a higher level solution to restrict/tighten it further based on a policy. I don't immediately have a suggestion on how.
If there is even a slight expectation of confidentiality (IMO just calling it a 'secret' in documentation is enough to justify that expectation) it should be treated as such. Thus qemu needs to add the possibility to pass it via the 'secret' object, so that libvirt can pass it encrypted. On the device commandline we'll just pass the alias to the secret. There's a well documented and maintained way to do that so it should be a very straightforward and quick modification.
Until then the problem is that we are unable to launch the VMs when the PF is in the uesrspace. For now this patch is only bridging the gap to qemu commandline.
The above should be done prior doing this in libvirt so that we can use the new approach without having any duplicate code.
If there is even a slight expectation of confidentiality (IMO just calling it a 'secret' in documentation is enough to justify that expectation) it should be treated as such.
Thus qemu needs to add the possibility to pass it via the 'secret' object, so that libvirt can pass it encrypted. On the device commandline we'll just pass the alias to the secret.
There's a well documented and maintained way to do that so it should be a very straightforward and quick modification.
Until then the problem is that we are unable to launch the VMs when the PF is in the uesrspace. For now this patch is only bridging the gap to qemu commandline.
The above should be done prior doing this in libvirt so that we can use the new approach without having any duplicate code.
Yes, that sounds good. We need to add the mechanism to pass the vf-token via a secret object. However, until it is done we are unable to proceed with our VMs with PF in the userspace unless we provide the vf-token as in this series. So for now either a) the qemu commit needs to be revert (so that we can continue without providing vf-token uuid), OR b) We add libvirt support for clear-text vf-token, then add the choice to qemu to additionally provide the vf-token via a secret object and then update libvirt to pass the encrypted secret. -vk
On Mon, Jan 08, 2024 at 17:05:45 +0530, Vivek Kashyap wrote:
If there is even a slight expectation of confidentiality (IMO just calling it a 'secret' in documentation is enough to justify that expectation) it should be treated as such.
Thus qemu needs to add the possibility to pass it via the 'secret' object, so that libvirt can pass it encrypted. On the device commandline we'll just pass the alias to the secret.
There's a well documented and maintained way to do that so it should be a very straightforward and quick modification.
Until then the problem is that we are unable to launch the VMs when the PF is in the uesrspace. For now this patch is only bridging the gap to qemu commandline.
The above should be done prior doing this in libvirt so that we can use the new approach without having any duplicate code.
Yes, that sounds good. We need to add the mechanism to pass the vf-token via a secret object. However, until it is done we are unable to proceed with our VMs with PF in the userspace unless we provide the vf-token as in this series.
So for now either a) the qemu commit needs to be revert (so that we can continue without providing vf-token uuid), OR b) We add libvirt support for clear-text vf-token, then add the choice to qemu to additionally provide the vf-token via a secret object and then update libvirt to pass the encrypted secret.
As I've stated above, for libvirt we should consider only passing it via the 'secret' object.
On Mon, Jan 08, 2024 at 12:44:01 +0100, Peter Krempa wrote:
On Mon, Jan 08, 2024 at 17:05:45 +0530, Vivek Kashyap wrote:
If there is even a slight expectation of confidentiality (IMO just calling it a 'secret' in documentation is enough to justify that expectation) it should be treated as such.
Thus qemu needs to add the possibility to pass it via the 'secret' object, so that libvirt can pass it encrypted. On the device commandline we'll just pass the alias to the secret.
There's a well documented and maintained way to do that so it should be a very straightforward and quick modification.
Until then the problem is that we are unable to launch the VMs when the PF is in the uesrspace. For now this patch is only bridging the gap to qemu commandline.
The above should be done prior doing this in libvirt so that we can use the new approach without having any duplicate code.
Yes, that sounds good. We need to add the mechanism to pass the vf-token via a secret object. However, until it is done we are unable to proceed with our VMs with PF in the userspace unless we provide the vf-token as in this series.
So for now either a) the qemu commit needs to be revert (so that we can continue without providing vf-token uuid), OR b) We add libvirt support for clear-text vf-token, then add the choice to qemu to additionally provide the vf-token via a secret object and then update libvirt to pass the encrypted secret.
As I've stated above, for libvirt we should consider only passing it via the 'secret' object.
Forgot to add: If you need a way to test it with a libvirt-started VM in the interim until the qemu commandline configuration accepts secrets (which should be fairly trivial, and we will accept patches based on qemu code which was pushed but not released yet) you can use device-property overrides: https://libvirt.org/drvqemu.html#overriding-properties-of-qemu-devices note that it has the same implications about supportability as using commandline overrides: https://libvirt.org/drvqemu.html#pass-through-of-arbitrary-qemu-commands
...
As I've stated above, for libvirt we should consider only passing it via the 'secret' object.
Sounds good. Will follow this up. Alex - will you be working on the qemu update?
Forgot to add:
If you need a way to test it with a libvirt-started VM in the interim until the qemu commandline configuration accepts secrets (which should be fairly trivial, and we will accept patches based on qemu code which was pushed but not released yet) you can use device-property overrides:
https://libvirt.org/drvqemu.html#overriding-properties-of-qemu-devices
note that it has the same implications about supportability as using commandline overrides:
https://libvirt.org/drvqemu.html#pass-through-of-arbitrary-qemu-commands
Thanks for the pointer. Will try it out. -vk
On Mon, 8 Jan 2024 18:42:19 +0530 (IST) Vivek Kashyap <vivek.kashyap@linux.intel.com> wrote:
...
As I've stated above, for libvirt we should consider only passing it via the 'secret' object.
Sounds good. Will follow this up.
Alex - will you be working on the qemu update?
I'm not the one driving a use case that requires libvirt support for vf-token. Thanks, Alex
On Mon, 8 Jan 2024, Alex Williamson wrote:
On Mon, 8 Jan 2024 18:42:19 +0530 (IST) Vivek Kashyap <vivek.kashyap@linux.intel.com> wrote:
...
As I've stated above, for libvirt we should consider only passing it via the 'secret' object.
Sounds good. Will follow this up.
Alex - will you be working on the qemu update?
I'm not the one driving a use case that requires libvirt support for vf-token. Thanks,
Not asking wrt libvirt but queried wrt qemu due to your original implementation in qemu: https://lore.kernel.org/lkml/20200204161737.34696b91@w520.home https://github.com/qemu/qemu/commit/2dca1b37a7605abb135559ef1b0d63929e7ae60d thanks vk
Alex
On Mon, 8 Jan 2024 23:32:04 +0530 (IST) Vivek Kashyap <vivek.kashyap@linux.intel.com> wrote:
On Mon, 8 Jan 2024, Alex Williamson wrote:
On Mon, 8 Jan 2024 18:42:19 +0530 (IST) Vivek Kashyap <vivek.kashyap@linux.intel.com> wrote:
...
As I've stated above, for libvirt we should consider only passing it via the 'secret' object.
Sounds good. Will follow this up.
Alex - will you be working on the qemu update?
I'm not the one driving a use case that requires libvirt support for vf-token. Thanks,
Not asking wrt libvirt but queried wrt qemu due to your original implementation in qemu:
https://lore.kernel.org/lkml/20200204161737.34696b91@w520.home https://github.com/qemu/qemu/commit/2dca1b37a7605abb135559ef1b0d63929e7ae60d
Per the first link: NB. It's unclear whether there's value to this QEMU support without further exposure of SR-IOV within a VM. This is meant mostly as a test case where the real initial users will likely be DPDK drivers. I didn't author the commit in the second link. Based on the above comment it's clear that QEMU, or any VMM use case, was not the initial target for vf-token support. It was intended for DPDK, which already has a pretty low security model. QEMU was only a proof of concept with a code base more familiar to me. I also noted in the above my expectation that SR-IOV would be the legitimate use case in QEMU. My thought there was that QEMU would set a private vf-token and emulate an SR-IOV capability to the guest. Enabling SR-IOV by the guest would trigger a call-out to libvirt to effect the change via host pci-sysfs and attach the resulting VFs back to the VM. The vf-token would remain private and act as a measure of protection against other use cases for the VFs. I've never been strongly in favor of general vf-token support in QEMU. If a VF requires a vf-token, then by definition the PF is being managed by another vfio userspace driver. Somebody needs to decide whether that userspace driver is trustworthy since it may have access to all the data accessible to the VF. An in-kernel PF driver would be trusted by default, so what's the underlying motivation to make vf-token support more ubiquitous through the stack? In my view, this support attempts to de-emphasize the security risk of a 3rd party userspace PF driver while also promoting their very existence. I won't deny that use cases for this exist, but I have yet to see evidence that those are use cases I care to promote. So no, I won't be implementing a secret object implementation of this in QEMU. Thanks, Alex
On Mon, 8 Jan 2024, Alex Williamson wrote:
On Mon, 8 Jan 2024 23:32:04 +0530 (IST) Vivek Kashyap <vivek.kashyap@linux.intel.com> wrote:
On Mon, 8 Jan 2024, Alex Williamson wrote:
On Mon, 8 Jan 2024 18:42:19 +0530 (IST) Vivek Kashyap <vivek.kashyap@linux.intel.com> wrote:
...
As I've stated above, for libvirt we should consider only passing it via the 'secret' object.
Sounds good. Will follow this up.
Alex - will you be working on the qemu update?
I'm not the one driving a use case that requires libvirt support for vf-token. Thanks,
Not asking wrt libvirt but queried wrt qemu due to your original implementation in qemu:
https://lore.kernel.org/lkml/20200204161737.34696b91@w520.home https://github.com/qemu/qemu/commit/2dca1b37a7605abb135559ef1b0d63929e7ae60d
Per the first link:
NB. It's unclear whether there's value to this QEMU support without further exposure of SR-IOV within a VM. This is meant mostly as a test case where the real initial users will likely be DPDK drivers.
I didn't author the commit in the second link.
Based on the above comment it's clear that QEMU, or any VMM use case, was not the initial target for vf-token support. It was intended for DPDK, which already has a pretty low security model. QEMU was only a proof of concept with a code base more familiar to me.
I also noted in the above my expectation that SR-IOV would be the legitimate use case in QEMU. My thought there was that QEMU would set a private vf-token and emulate an SR-IOV capability to the guest. Enabling SR-IOV by the guest would trigger a call-out to libvirt to effect the change via host pci-sysfs and attach the resulting VFs back to the VM. The vf-token would remain private and act as a measure of protection against other use cases for the VFs.
I've never been strongly in favor of general vf-token support in QEMU. If a VF requires a vf-token, then by definition the PF is being managed by another vfio userspace driver. Somebody needs to decide whether that userspace driver is trustworthy since it may have access to all the data accessible to the VF. An in-kernel PF driver would be trusted by default, so what's the underlying motivation to make vf-token support more ubiquitous through the stack?
In my view, this support attempts to de-emphasize the security risk of a 3rd party userspace PF driver while also promoting their very existence. I won't deny that use cases for this exist, but I have yet to see evidence that those are use cases I care to promote. So no, I won't be implementing a secret object implementation of this in QEMU. Thanks,
Thanks for the explanation. I'll follow-up on the secret object implementation. -vk
Alex
participants (3)
-
Alex Williamson -
Peter Krempa -
Vivek Kashyap