4:53 p.m.
On 4/29/21 2:51 PM, Peter Krempa wrote:
On Thu, Apr 29, 2021 at 14:12:44 +0300, Gavi Teitz wrote:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzill...
Add support for setting the page-per-vq flag, which is important for
vdpa with vhost-user performance.
Signed-off-by: Gavi Teitz <gavi@nvidia.com><mailto:gavi@nvidia.com>
---
docs/formatdomain.rst | 11 +++++-
docs/schemas/domaincommon.rng | 5 +++
src/conf/domain_conf.c | 15 ++++++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 5 +++
src/qemu/qemu_hotplug.c | 1 +
tests/qemuxml2argvdata/net-virtio-page-per-vq.args | 31 ++++++++++++++++
tests/qemuxml2argvdata/net-virtio-page-per-vq.xml | 29 +++++++++++++++
tests/qemuxml2argvtest.c | 2 +
.../qemuxml2xmloutdata/net-virtio-page-per-vq.xml | 43 ++++++++++++++++++++++
tests/qemuxml2xmltest.c | 2 +
13 files changed, 147 insertions(+), 1 deletion(-)
create mode 100644 tests/qemuxml2argvdata/net-virtio-page-per-vq.args
create mode 100644 tests/qemuxml2argvdata/net-virtio-page-per-vq.xml
create mode 100644 tests/qemuxml2xmloutdata/net-virtio-page-per-vq.xml
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 282176c..f5a9c70 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -5120,7 +5120,7 @@ Setting NIC driver-specific options
<source network='default'/>
<target dev='vnet1'/>
<model type='virtio'/>
- <driver name='vhost' txmode='iothread' ioeventfd='on'
event_idx='off' queues='5' rx_queue_size='256'
tx_queue_size='256'>
+ <driver name='vhost' txmode='iothread' ioeventfd='on'
event_idx='off' queues='5' rx_queue_size='256'
tx_queue_size='256' page_per_vq='on'>
<host csum='off' gso='off' tso4='off'
tso6='off' ecn='off' ufo='off' mrg_rxbuf='off'/>
<guest csum='off' tso4='off' tso6='off'
ecn='off' ufo='off'/>
</driver>
@@ -5215,6 +5215,15 @@ following attributes are available for the ``"virtio"``
NIC driver:
only for ``vhostuser`` type. :since:`Since 3.7.0 (QEMU and KVM only)`
**In general you should leave this option alone, unless you are very certain
you know what you are doing.**
+``page_per_vq``
+ This optional attribute controls the layout of the notification capabilities
+ exposed to the guest. When enabled, each virtio queue will have a dedicated
+ page on the device BAR exposed to the guest. It is recommended to be used when
+ vDPA is enabled on the hypervisor, as it enables mapping the notification area
+ to the physical device, which is only supported in page granularity. The
+ default is determined by QEMU; as off. :since:`Since 2.8.0 (QEMU only)`
The 'since' tag is primarily used for the libvirt version which
introduces the feature thus you must include that.
Also note that starting from the next release the minimum supported qemu
version will be bumped to qemu-2.10 thus the qemu version note is not
needed.
It isn't really clear what the 'since' tag refers to, if I understand your
comment correctly it refers to the libvirt version, so I will change it to 7.4.0 and
remove the '(QEMU only)' part.
+ **In general you should leave this option alone, unless you are very certain
+ you know what you are doing.**
virtio options
For virtio interfaces, `Virtio-specific options <#elementsVirtio>`__ can also
be set. ( :since:`Since 3.5.0` )
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index a2e5c50..e61ad67 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3463,6 +3463,11 @@
</attribute>
</optional>
<optional>
+ <attribute name="page_per_vq">
+ <ref name="virOnOff"/>
+ </attribute>
+ </optional>
+ <optional>
<attribute name="txmode">
<choice>
<value>iothread</value>
[...]
@@ -1405,6 +1406,7 @@ static struct virQEMUCapsDevicePropsFlags
virQEMUCapsDevicePropsVirtioNet[] = {
{ "event_idx", QEMU_CAPS_VIRTIO_NET_EVENT_IDX, NULL },
{ "rx_queue_size", QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE, NULL },
{ "tx_queue_size", QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE, NULL },
+ { "page_per_vq", QEMU_CAPS_VIRTIO_NET_PAGE_PER_VQ, NULL },
{ "host_mtu", QEMU_CAPS_VIRTIO_NET_HOST_MTU, NULL },
{ "disable-legacy", QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, NULL },
{ "iommu_platform", QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM, NULL },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index f54aad5..75feed4 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -609,6 +609,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check
*/
/* 400 */
QEMU_CAPS_COMPAT_DEPRECATED, /* -compat deprecated-(input|output) is supported */
QEMU_CAPS_ACPI_INDEX, /* PCI device 'acpi-index' property */
+ QEMU_CAPS_VIRTIO_NET_PAGE_PER_VQ, /* virtio-net-*.page_per_vq */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
1) qemu capabilities changes are usually separated into a separate commit
2) missing update to the capability data, either this doesn't detect
anything or tests would be broken.
3) the capability is not necessary since all qemus supported since
libvirt-7.4 will have it
Since this is not necessary I will remove the changes to qemu_capabilities*.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d7f1c71..7b5834f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3630,6 +3630,11 @@ qemuBuildNicDevStr(virDomainDef *def,
if (net->driver.virtio.tx_queue_size)
virBufferAsprintf(&buf, ",tx_queue_size=%u",
net->driver.virtio.tx_queue_size);
+ if (net->driver.virtio.page_per_vq) {
+ virBufferAsprintf(&buf, ",page-per-vq=%s",
+
virTristateSwitchTypeToString(net->driver.virtio.page_per_vq));
+ }
+
if (net->mtu)
virBufferAsprintf(&buf, ",host_mtu=%u", net->mtu);
[...]
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index f0efe98..f6b8f34 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1654,6 +1654,8 @@ mymain(void)
QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE);
DO_TEST_PARSE_ERROR("net-virtio-rxqueuesize-invalid-size",
QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE);
+ DO_TEST("net-virtio-page-per-vq",
+ QEMU_CAPS_VIRTIO_NET_PAGE_PER_VQ);
New positive tests should not use DO_TEST, but DO_TEST_CAPS_LATEST.
Ack.
DO_TEST("net-virtio-teaming",
QEMU_CAPS_VIRTIO_NET_FAILOVER,
QEMU_CAPS_DEVICE_VFIO_PCI);
[...]
According to the docs this is also a guest-visible change, thus we are
missing a check in the ABI stability checker code which ensures that the
guest ABI doesn't change.
See virDomainDefCheckABIStabilityFlags to find the appropriate place.
I couldn't find a related function for comparing driver options for virDomainNetDef (I
expected to find something in virDomainNetDefCheckABIStability(), but didn't), please
clarify what needs to be added here.