[PATCH] Add page_per_vq flag to the 'driver' element of virtio devices
Peter Krempa
pkrempa at redhat.com
Thu Apr 29 11:51:47 UTC 2021
On Thu, Apr 29, 2021 at 14:12:44 +0300, Gavi Teitz wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1925363
>
> Add support for setting the page-per-vq flag, which is important for
> vdpa with vhost-user performance.
>
> Signed-off-by: Gavi Teitz <gavi at 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.
> + **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
> 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.
> 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.
More information about the libvir-list
mailing list