6:04 p.m.
vf token is set by a vfio-pci based PF driver and it must be known to the
vfio-pci based VF driver. This vf-token is set by the PF driver before VF
drivers can access the device. vfio-pci driver and qemu support vf-token.
This RFC 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"}
This patch is get feedback on the approach. Will post with add
documentation and testcases in follow-up.
Vivek Kashyap (5):
virpci: Define vf-token
qemu: vf-token capability
conf: vf-token flag
conf: vf-token parsing and formatting
qemu: validate and generate vf-token on command line
src/conf/device_conf.c | 31 +++++++++++++++++++++++++++++--
src/conf/device_conf.h | 3 +++
src/conf/domain_addr.h | 1 +
src/conf/domain_conf.c | 5 +++++
src/conf/schemas/basictypes.rng | 11 +++++++++++
src/conf/schemas/domaincommon.rng | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 27 ++++++++++++++++++++++++---
src/qemu/qemu_domain_address.c | 3 +++
src/qemu/qemu_validate.c | 19 +++++++++++++++++++
src/util/virpci.c | 5 +++++
src/util/virpci.h | 11 +++++++++++
14 files changed, 117 insertions(+), 5 deletions(-)
--
2.25.1
6:05 p.m.
New subject: [RFC: vf-token 1/5] virpci: Define vf-token
Define the vf-token extension for PCI device
Signed-off-by: Vivek Kashyap <vivek.kashyap(a)linux.intel.com>
---
src/util/virpci.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/src/util/virpci.h b/src/util/virpci.h
index faca6cf6f9..b4e9e95d88 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -50,7 +50,15 @@ struct _virZPCIDeviceAddress {
/* Don't forget to update virPCIDeviceAddressCopy if needed. */
};
+typedef struct _virPCIDeviceToken virPCIDeviceToken;
+
+struct _virPCIDeviceToken {
+ unsigned char uuid[VIR_UUID_BUFLEN];
+ bool isSet;
+};
+
#define VIR_PCI_DEVICE_ADDRESS_FMT "%04x:%02x:%02x.%d"
+#define VIR_PCI_DEVICE_TOKEN_FMT
"%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x"
/* Represents format of PF's phys_port_name in switchdev mode:
* 'p%u' or 'p%us%u'. New line checked since value is read from sysfs
file.
@@ -65,6 +73,7 @@ struct _virPCIDeviceAddress {
virTristateSwitch multi;
int extFlags; /* enum virPCIDeviceAddressExtensionFlags */
virZPCIDeviceAddress zpci;
+ virPCIDeviceToken token;
/* Don't forget to update virPCIDeviceAddressCopy if needed. */
};
@@ -269,6 +278,8 @@ int virPCIDeviceAddressParse(char *address, virPCIDeviceAddress
*bdf);
bool virZPCIDeviceAddressIsIncomplete(const virZPCIDeviceAddress *addr);
bool virZPCIDeviceAddressIsPresent(const virZPCIDeviceAddress *addr);
+bool virPCIVFIOTokenIDIsPresent(const virPCIDeviceToken *token);
+bool virDeviceExtensionIsPresent(const virPCIDeviceAddress *pci);
int virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path,
int pfNetDevIdx,
--
2.25.1
2:23 a.m.
New subject: [RFC: vf-token 1/5] virpci: Define vf-token
On Thu, Oct 05, 2023 at 16:05:00 -0700, Vivek Kashyap wrote:
Define the vf-token extension for PCI device
Signed-off-by: Vivek Kashyap <vivek.kashyap(a)linux.intel.com>
---
src/util/virpci.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/src/util/virpci.h b/src/util/virpci.h
index faca6cf6f9..b4e9e95d88 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -50,7 +50,15 @@ struct _virZPCIDeviceAddress {
/* Don't forget to update virPCIDeviceAddressCopy if needed. */
};
+typedef struct _virPCIDeviceToken virPCIDeviceToken;
+
+struct _virPCIDeviceToken {
+ unsigned char uuid[VIR_UUID_BUFLEN];
+ bool isSet;
+};
+
#define VIR_PCI_DEVICE_ADDRESS_FMT "%04x:%02x:%02x.%d"
+#define VIR_PCI_DEVICE_TOKEN_FMT
"%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x"
/* Represents format of PF's phys_port_name in switchdev mode:
* 'p%u' or 'p%us%u'. New line checked since value is read from sysfs
file.
@@ -65,6 +73,7 @@ struct _virPCIDeviceAddress {
virTristateSwitch multi;
int extFlags; /* enum virPCIDeviceAddressExtensionFlags */
virZPCIDeviceAddress zpci;
+ virPCIDeviceToken token;
/* Don't forget to update virPCIDeviceAddressCopy if needed. */
};
@@ -269,6 +278,8 @@ int virPCIDeviceAddressParse(char *address, virPCIDeviceAddress
*bdf);
bool virZPCIDeviceAddressIsIncomplete(const virZPCIDeviceAddress *addr);
bool virZPCIDeviceAddressIsPresent(const virZPCIDeviceAddress *addr);
+bool virPCIVFIOTokenIDIsPresent(const virPCIDeviceToken *token);
+bool virDeviceExtensionIsPresent(const virPCIDeviceAddress *pci);
These function definitions should go into the patch that actually
implements the functions themselves.
6:05 p.m.
New subject: [RFC: vf-token 2/5] qemu: vf-token capability
Introduce qemu capability for vf-token
Signed-off-by: Vivek Kashyap <vivek.kashyap(a)linux.intel.com>
---
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 3a1bfbf74d..f84c4df8db 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 */
);
@@ -1384,6 +1385,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 },
};
@@ -1446,6 +1448,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;
--
2.25.1
2:31 a.m.
New subject: [RFC: vf-token 2/5] qemu: vf-token capability
On Thu, Oct 05, 2023 at 16:05:01 -0700, Vivek Kashyap wrote:
Introduce qemu capability for vf-token
Signed-off-by: Vivek Kashyap <vivek.kashyap(a)linux.intel.com>
---
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 3a1bfbf74d..f84c4df8db 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 */
);
@@ -1384,6 +1385,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 },
};
@@ -1446,6 +1448,7 @@ static struct virQEMUCapsDevicePropsFlags
virQEMUCapsDevicePropsVirtioSCSI[] = {
};
static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsVfioPCI[] = {
+ { "vf-token", QEMU_CAPS_VFIO_VFTOKEN, NULL },
Extra trailing whitespace.
};
Also the 'vf-token' field was added in qemu-8.1 thus you are missing the
appropriate test file updates.
Also the whitespace error would be caught by our test suite.
Please always make sure to post patches where our test suite passes
after every single commit:
https://www.libvirt.org/hacking.html#preparing-patches
Notably you'll need to re-generate the 'qemucapabilitiestest' output
files to include the new flag. To do so automatically run:
VIR_TEST_REGENERATE_OUTPUT=1 ./tests/qemucapabilitiestest
from your build directory after you build everything.
6:05 p.m.
New subject: [RFC: vf-token 3/5] conf: vf-token flag
Define PCI address extension flag for vf-token
Signed-off-by: Vivek Kashyap <vivek.kashyap(a)linux.intel.com>
---
src/conf/device_conf.h | 3 +++
src/conf/domain_addr.h | 1 +
src/qemu/qemu_domain_address.c | 3 +++
3 files changed, 7 insertions(+)
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index a83377417a..29e03baa99 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -188,6 +188,9 @@ bool virDeviceInfoPCIAddressExtensionIsPresent(const
virDomainDeviceInfo *info);
int virPCIDeviceAddressParseXML(xmlNodePtr node,
virPCIDeviceAddress *addr);
+int virPCIDeviceTokenParseXML(xmlNodePtr node,
+ virPCIDeviceAddress *addr);
+
void virPCIDeviceAddressFormat(virBuffer *buf,
virPCIDeviceAddress addr,
bool includeTypeInAddr);
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index e72fb48847..bca96ee797 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, /* 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.25.1
2:32 a.m.
New subject: [RFC: vf-token 3/5] conf: vf-token flag
On Thu, Oct 05, 2023 at 16:05:02 -0700, Vivek Kashyap wrote:
Define PCI address extension flag for vf-token
Signed-off-by: Vivek Kashyap <vivek.kashyap(a)linux.intel.com>
---
src/conf/device_conf.h | 3 +++
src/conf/domain_addr.h | 1 +
src/qemu/qemu_domain_address.c | 3 +++
3 files changed, 7 insertions(+)
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index a83377417a..29e03baa99 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -188,6 +188,9 @@ bool virDeviceInfoPCIAddressExtensionIsPresent(const
virDomainDeviceInfo *info);
int virPCIDeviceAddressParseXML(xmlNodePtr node,
virPCIDeviceAddress *addr);
+int virPCIDeviceTokenParseXML(xmlNodePtr node,
+ virPCIDeviceAddress *addr);
Broken alignment of the second line. And same as in 1/5 the function is
not used here so don't define it in this patch.
6:05 p.m.
New subject: [RFC: vf-token 4/5] conf: vf-token parsing and formatting
Introduce XML parsing and formatting of vf-token attribute
Signed-off-by: Vivek Kashyap <vivek.kashyap(a)linux.intel.com>
---
src/conf/device_conf.c | 31 +++++++++++++++++++++++++++++--
src/conf/domain_conf.c | 5 +++++
src/conf/schemas/basictypes.rng | 11 +++++++++++
src/conf/schemas/domaincommon.rng | 1 +
src/libvirt_private.syms | 1 +
src/util/virpci.c | 5 +++++
6 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index f3d977f2b7..d5ed4ef452 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -188,11 +188,20 @@ virDeviceInfoPCIAddressExtensionIsWanted(const virDomainDeviceInfo
*info)
virZPCIDeviceAddressIsIncomplete(&info->addr.pci.zpci);
}
+bool
+virDeviceExtensionIsPresent(const virPCIDeviceAddress *pci)
+{
+ return ((pci->extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) &&
+ virZPCIDeviceAddressIsPresent(&pci->zpci)) ||
+ ((pci->extFlags & VIR_PCI_ADDRESS_EXTENSION_VFTOKEN) &&
+ pci->token.isSet);
+}
+
bool
virDeviceInfoPCIAddressExtensionIsPresent(const virDomainDeviceInfo *info)
{
- return (info->addr.pci.extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) &&
- virZPCIDeviceAddressIsPresent(&info->addr.pci.zpci);
+ return (info->addr.pci.extFlags != VIR_PCI_ADDRESS_EXTENSION_NONE) &&
+ virDeviceExtensionIsPresent(&info->addr.pci);
}
int
@@ -200,6 +209,7 @@ virPCIDeviceAddressParseXML(xmlNodePtr node,
virPCIDeviceAddress *addr)
{
xmlNodePtr zpci;
+ xmlNodePtr token;
memset(addr, 0, sizeof(*addr));
@@ -231,6 +241,10 @@ virPCIDeviceAddressParseXML(xmlNodePtr node,
return -1;
}
+ if ((token = virXMLNodeGetSubelement(node, "vf-token")))
+ if (virPCIDeviceTokenParseXML(token, addr) < 0)
+ return -1;
+
return 0;
}
@@ -248,6 +262,19 @@ virPCIDeviceAddressFormat(virBuffer *buf,
addr.function);
}
+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;
+}
+
int
virCCWDeviceAddressParseXML(xmlNodePtr node,
virCCWDeviceAddress *addr)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4435ee2ad4..cceb1d3b83 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5389,6 +5389,11 @@ virDomainDeviceInfoFormat(virBuffer *buf,
info->addr.pci.zpci.uid.value,
info->addr.pci.zpci.fid.value);
}
+ if (virPCIVFIOTokenIDIsPresent(&info->addr.pci.token)) {
+ virBufferAsprintf(&childBuf,
+ "<vf-token uuid='%s'/>\n",
+ info->addr.pci.token.uuid);
+ }
break;
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
diff --git a/src/conf/schemas/basictypes.rng b/src/conf/schemas/basictypes.rng
index 26eb538077..bf2c30dd18 100644
--- a/src/conf/schemas/basictypes.rng
+++ b/src/conf/schemas/basictypes.rng
@@ -138,6 +138,17 @@
</element>
</optional>
</define>
+ <define name="vf-tokenid">
+ <optional>
+ <element name="vf-token">
+ <optional>
+ <attribute name="uuid">
+ <ref name="UUID"/>
+ </attribute>
+ </optional>
+ </element>
+ </optional>
+ </define>
<!-- a 6 byte MAC address in ASCII-hex format, eg "12:34:56:78:9A:BC"
-->
<!-- The lowest bit of the 1st byte is the "multicast" bit. a
-->
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index a26986b5ce..b228a3ca73 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -7034,6 +7034,7 @@
</attribute>
<ref name="pciaddress"/>
<ref name="zpciaddress"/>
+ <ref name="vf-tokenid"/>
</group>
<group>
<attribute name="type">
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4e475d5b1a..0d4866cd56 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 08b82708b1..dfb0f29182 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2312,6 +2312,11 @@ virZPCIDeviceAddressIsPresent(const virZPCIDeviceAddress *addr)
return addr->uid.isSet || addr->fid.isSet;
}
+bool
+virPCIVFIOTokenIDIsPresent(const virPCIDeviceToken *token)
+{
+ return token->isSet;
+}
void
virPCIVirtualFunctionListFree(virPCIVirtualFunctionList *list)
--
2.25.1
2:40 a.m.
New subject: [RFC: vf-token 4/5] conf: vf-token parsing and formatting
On Thu, Oct 05, 2023 at 16:05:03 -0700, Vivek Kashyap wrote:
Introduce XML parsing and formatting of vf-token attribute
Signed-off-by: Vivek Kashyap <vivek.kashyap(a)linux.intel.com>
---
src/conf/device_conf.c | 31 +++++++++++++++++++++++++++++--
src/conf/domain_conf.c | 5 +++++
src/conf/schemas/basictypes.rng | 11 +++++++++++
src/conf/schemas/domaincommon.rng | 1 +
src/libvirt_private.syms | 1 +
src/util/virpci.c | 5 +++++
6 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index f3d977f2b7..d5ed4ef452 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -188,11 +188,20 @@ virDeviceInfoPCIAddressExtensionIsWanted(const virDomainDeviceInfo
*info)
virZPCIDeviceAddressIsIncomplete(&info->addr.pci.zpci);
}
+bool
+virDeviceExtensionIsPresent(const virPCIDeviceAddress *pci)
+{
+ return ((pci->extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) &&
+ virZPCIDeviceAddressIsPresent(&pci->zpci)) ||
+ ((pci->extFlags & VIR_PCI_ADDRESS_EXTENSION_VFTOKEN) &&
+ pci->token.isSet);
The code alignment is broken here. Also add a function documentation
describing what this function does as it's really not obvious.
+}
+
Code style dictates two empty lines between functions.
bool
virDeviceInfoPCIAddressExtensionIsPresent(const virDomainDeviceInfo *info)
{
- return (info->addr.pci.extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) &&
- virZPCIDeviceAddressIsPresent(&info->addr.pci.zpci);
+ return (info->addr.pci.extFlags != VIR_PCI_ADDRESS_EXTENSION_NONE) &&
+ virDeviceExtensionIsPresent(&info->addr.pci);
This change is suspicious and should be justified. I'm not going to
comment on whether it's correct or not.
}
int
@@ -200,6 +209,7 @@ virPCIDeviceAddressParseXML(xmlNodePtr node,
virPCIDeviceAddress *addr)
{
xmlNodePtr zpci;
+ xmlNodePtr token;
memset(addr, 0, sizeof(*addr));
@@ -231,6 +241,10 @@ virPCIDeviceAddressParseXML(xmlNodePtr node,
return -1;
}
+ if ((token = virXMLNodeGetSubelement(node, "vf-token")))
Multi-line condition bodies dictate use of a { } block around it.
+ if (virPCIDeviceTokenParseXML(token, addr) < 0)
+ return -1;
This one is okay though.
+
return 0;
}
@@ -248,6 +262,19 @@ virPCIDeviceAddressFormat(virBuffer *buf,
addr.function);
}
+int
+virPCIDeviceTokenParseXML(xmlNodePtr node,
+ virPCIDeviceAddress *addr)
+{
+ if (virXMLPropUUID(node, "uuid", VIR_XML_PROP_NONE,
+ addr->token.uuid) < 0)
virXMLPropUUID parses into a buffer which is not a string (NUL-byte
terminated) but a byte array ...
> + return -1;
> +
> + addr->token.isSet = 1;
> +
> + return 0;
+}
+
Two empty lines between functions.
int
virCCWDeviceAddressParseXML(xmlNodePtr node,
virCCWDeviceAddress *addr)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4435ee2ad4..cceb1d3b83 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5389,6 +5389,11 @@ virDomainDeviceInfoFormat(virBuffer *buf,
info->addr.pci.zpci.uid.value,
info->addr.pci.zpci.fid.value);
}
+ if (virPCIVFIOTokenIDIsPresent(&info->addr.pci.token)) {
+ virBufferAsprintf(&childBuf,
+ "<vf-token uuid='%s'/>\n",
+ info->addr.pci.token.uuid);
... but here you use a *string* formatting function. So this will either
not print any reasonable output or potentially crash if the UUID
contains no NUL-bytes.
You would figure this out if you'd add any tests for this feature which
is in fact required. It is okay to add the tests at the end though.
+ }
break;
case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
diff --git a/src/conf/schemas/basictypes.rng b/src/conf/schemas/basictypes.rng
index 26eb538077..bf2c30dd18 100644
--- a/src/conf/schemas/basictypes.rng
+++ b/src/conf/schemas/basictypes.rng
@@ -138,6 +138,17 @@
</element>
</optional>
</define>
+ <define name="vf-tokenid">
+ <optional>
+ <element name="vf-token">
+ <optional>
+ <attribute name="uuid">
+ <ref name="UUID"/>
+ </attribute>
+ </optional>
+ </element>
+ </optional>
+ </define>
<!-- a 6 byte MAC address in ASCII-hex format, eg "12:34:56:78:9A:BC"
-->
<!-- The lowest bit of the 1st byte is the "multicast" bit. a
-->
diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
index a26986b5ce..b228a3ca73 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -7034,6 +7034,7 @@
</attribute>
<ref name="pciaddress"/>
<ref name="zpciaddress"/>
+ <ref name="vf-tokenid"/>
</group>
<group>
<attribute name="type">
Any XML addition _must_ come with correspondign documentation update.
Please document the new flag and how it's supposed to be used in
docs/formatdomain.rst.
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 4e475d5b1a..0d4866cd56 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3138,6 +3138,7 @@ virPCIHeaderTypeToString;
virPCIIsVirtualFunction;
virPCIStubDriverTypeFromString;
virPCIStubDriverTypeToString;
+virPCIVFIOTokenIDIsPresent;
virPCIVirtualFunctionListFree;
virZPCIDeviceAddressIsIncomplete;
virZPCIDeviceAddressIsPresent;
This hunk and all others most likely belong to the patch which was
defining the function or vice-versa.
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 08b82708b1..dfb0f29182 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2312,6 +2312,11 @@ virZPCIDeviceAddressIsPresent(const virZPCIDeviceAddress *addr)
return addr->uid.isSet || addr->fid.isSet;
}
+bool
+virPCIVFIOTokenIDIsPresent(const virPCIDeviceToken *token)
+{
+ return token->isSet;
+}
void
virPCIVirtualFunctionListFree(virPCIVirtualFunctionList *list)
--
2.25.1
6:05 p.m.
New subject: [RFC: vf-token 5/5] qemu: validate and generate vf-token on command line
Introduce a validation function for vf-token support in qemu
and generate vf-token device attribute in qmeu command line.
Signed-off-by: Vivek Kashyap <vivek.kashyap(a)linux.intel.com>
---
src/qemu/qemu_command.c | 27 ++++++++++++++++++++++++---
src/qemu/qemu_validate.c | 19 +++++++++++++++++++
2 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8a7b80719f..91d7836f5c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1979,7 +1979,6 @@ qemuBuildZPCIDevProps(virDomainDeviceInfo *dev)
return props;
}
-
static int
qemuCommandAddExtDevice(virCommand *cmd,
virDomainDeviceInfo *dev,
@@ -4729,6 +4728,8 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def,
pcisrc->addr.slot,
pcisrc->addr.function);
const char *failover_pair_id = NULL;
+ g_autofree char *token = NULL;
+ int ret = 0;
/* caller has to assign proper passthrough backend type */
switch (pcisrc->backend) {
@@ -4755,13 +4756,33 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def,
teaming->persistent)
failover_pair_id = teaming->persistent;
- if (virJSONValueObjectAdd(&props,
+ if ((dev->info->pciAddrExtFlags & VIR_PCI_ADDRESS_EXTENSION_VFTOKEN)
&&
+ pcisrc->addr.token.isSet) {
+ const unsigned char *uuid = pcisrc->addr.token.uuid;
+
+ token = g_strdup_printf(VIR_PCI_DEVICE_TOKEN_FMT,
+ uuid[0], uuid[1], uuid[2], uuid[3],
+ uuid[4], uuid[5], uuid[6], uuid[7],
+ uuid[8], uuid[9], uuid[10], uuid[11],
+ uuid[12], uuid[13], uuid[14], uuid[15]);
+
+ ret = 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,
- NULL) < 0)
+ NULL);
+ } else
+ ret = virJSONValueObjectAdd(&props,
+ "s:driver", "vfio-pci",
+ "s:host", host,
+ "s:id", dev->info->alias,
+ "p:bootindex",
dev->info->effectiveBootIndex,
+ "S:failover_pair_id", failover_pair_id,
+ NULL);
+ if (ret < 0)
return NULL;
if (qemuBuildDeviceAddressProps(props, def, dev->info) < 0)
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 93df9e4c8e..d628949597 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1363,6 +1363,23 @@ qemuValidateDomainDeviceDefZPCIAddress(virDomainDeviceInfo *info,
return 0;
}
+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,
@@ -1483,6 +1500,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.25.1
2:46 a.m.
New subject: [RFC: vf-token 5/5] qemu: validate and generate vf-token on command line
On Thu, Oct 05, 2023 at 16:05:04 -0700, Vivek Kashyap wrote:
Introduce a validation function for vf-token support in qemu
and generate vf-token device attribute in qmeu command line.
Signed-off-by: Vivek Kashyap <vivek.kashyap(a)linux.intel.com>
---
src/qemu/qemu_command.c | 27 ++++++++++++++++++++++++---
src/qemu/qemu_validate.c | 19 +++++++++++++++++++
2 files changed, 43 insertions(+), 3 deletions(-)
Missing qemuxml2argvtest and qemuxml2xmtest additions, which are
absolutely required for anything that adds/changes the commandline syntax.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8a7b80719f..91d7836f5c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1979,7 +1979,6 @@ qemuBuildZPCIDevProps(virDomainDeviceInfo *dev)
return props;
}
-
Spurious whitespace change. As noted we do 2 lines between functions.
static int
qemuCommandAddExtDevice(virCommand *cmd,
virDomainDeviceInfo *dev,
@@ -4729,6 +4728,8 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def,
pcisrc->addr.slot,
pcisrc->addr.function);
const char *failover_pair_id = NULL;
+ g_autofree char *token = NULL;
+ int ret = 0;
/* caller has to assign proper passthrough backend type */
switch (pcisrc->backend) {
@@ -4755,13 +4756,33 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def,
teaming->persistent)
failover_pair_id = teaming->persistent;
- if (virJSONValueObjectAdd(&props,
+ if ((dev->info->pciAddrExtFlags & VIR_PCI_ADDRESS_EXTENSION_VFTOKEN)
&&
+ pcisrc->addr.token.isSet) {
Broken alignment/code style.
+ const unsigned char *uuid = pcisrc->addr.token.uuid;
+
+ token = g_strdup_printf(VIR_PCI_DEVICE_TOKEN_FMT,
+ uuid[0], uuid[1], uuid[2], uuid[3],
+ uuid[4], uuid[5], uuid[6], uuid[7],
+ uuid[8], uuid[9], uuid[10], uuid[11],
+ uuid[12], uuid[13], uuid[14], uuid[15]);
Broken alignment/code style.
+
+ ret = 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,
- NULL) < 0)
+ NULL);
So firstly you break the code style here by your addition.
Secondly there's no need to duplicate the whole virJSONValueObjectAdd
block in the first place as we allow conditional formatting of fields.
You need to fill the 'token' field only optionally what you do, but then
you can unconditionally format it using:
"S:vf-token", token,
argument tuple in virJSONValueObjectAdd. The 'S' modifier formats the
attribute only if the string argument is non-NULL.
+ } else
Trailing whitespace, but this block is not needed ... and in fact not
desired as described above.
+ ret = virJSONValueObjectAdd(&props,
+ "s:driver", "vfio-pci",
+ "s:host", host,
+ "s:id", dev->info->alias,
+ "p:bootindex",
dev->info->effectiveBootIndex,
+ "S:failover_pair_id", failover_pair_id,
+ NULL);
+ if (ret < 0)
return NULL;
if (qemuBuildDeviceAddressProps(props, def, dev->info) < 0)
2:22 a.m.
On Thu, Oct 05, 2023 at 16:04:59 -0700, Vivek Kashyap wrote:
vf token is set by a vfio-pci based PF driver and it must be known to
the
vfio-pci based VF driver. This vf-token is set by the PF driver before VF
drivers can access the device. vfio-pci driver and qemu support vf-token.
This RFC 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"}
This patch is get feedback on the approach. Will post with add
documentation and testcases in follow-up.
Vivek Kashyap (5):
virpci: Define vf-token
qemu: vf-token capability
conf: vf-token flag
conf: vf-token parsing and formatting
qemu: validate and generate vf-token on command line
Please note that my forthcoming review will not include any comments on
whether this feature itself makes sense or whether the XML desing you
are proposing is reasonable ...
src/conf/device_conf.c | 31
+++++++++++++++++++++++++++++--
src/conf/device_conf.h | 3 +++
src/conf/domain_addr.h | 1 +
src/conf/domain_conf.c | 5 +++++
src/conf/schemas/basictypes.rng | 11 +++++++++++
src/conf/schemas/domaincommon.rng | 1 +
src/libvirt_private.syms | 1 +
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 27 ++++++++++++++++++++++++---
src/qemu/qemu_domain_address.c | 3 +++
src/qemu/qemu_validate.c | 19 +++++++++++++++++++
src/util/virpci.c | 5 +++++
src/util/virpci.h | 11 +++++++++++
14 files changed, 117 insertions(+), 5 deletions(-)
This is mostly because the patches do not contain any changes to
documentation that would explain it and I'm not familiar with what the
feature is supposed to do. Thus my comments will be purely for the code
itself and a further review will be required.
2:34 p.m.
On Fri, 6 Oct 2023, Peter Krempa wrote:
...
This is mostly because the patches do not contain any changes to
documentation that would explain it and I'm not familiar with what the
feature is supposed to do. Thus my comments will be purely for the code
itself and a further review will be required.
Thanks! I've gone over your comments on the patches and will repost next week
with the fixes and include documentation and tests.
-vk
414
days inactive
415
days old
12 comments
2 participants
participants (2)
-
Peter Krempa -
Vivek Kashyap