Re: [PATCH] virtio: avoid cost of -ftrivial-auto-var-init in hot path

[Philippe Mathieu-Daudé]

On 5/6/25 10:34, Daniel P. Berrangé wrote:
On Wed, Jun 04, 2025 at 03:18:43PM -0400, Stefan Hajnoczi wrote:
Since commit 7ff9ff039380 ("meson: mitigate against use of uninitialize
stack for exploits") the -ftrivial-auto-var-init=zero compiler option is
used to zero local variables. While this reduces security risks
associated with uninitialized stack data, it introduced a measurable
bottleneck in the virtqueue_split_pop() and virtqueue_packed_pop()
functions.
These virtqueue functions are in the hot path. They are called for each
element (request) that is popped from a VIRTIO device's virtqueue. Using
__attribute__((uninitialized)) on large stack variables in these
functions improves fio randread bs=4k iodepth=64 performance from 304k
to 332k IOPS (+9%).

IIUC, the 'hwaddr addr' variable is 8k in size, and the 'struct iovec iov'
array is 16k in size, so we have 24k on the stack that we're clearing and
then later writing the real value. Makes sense that this would have a
perf impact in a hotpath.

This issue was found using perf-top(1). virtqueue_split_pop() was one of
the top CPU consumers and the "annotate" feature showed that the memory
zeroing instructions at the beginning of the functions were hot.

When you say you found it with 'perf-top' was that just discovered by
accident, or was this usage of perf-top in response to users reporting
a performance degradation vs earlier QEMU ?

Would it make sense to move these to VirtQueue (since the structure
definition is local anyway)?

-- >8 --
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 85110bce374..b96c6ec603c 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -153,6 +153,12 @@ struct VirtQueue
     EventNotifier host_notifier;
     bool host_notifier_enabled;
     QLIST_ENTRY(VirtQueue) node;
+
+    /* Only used by virtqueue_pop() */
+    struct {
+        hwaddr addr[VIRTQUEUE_MAX_SIZE];
+        struct iovec iov[VIRTQUEUE_MAX_SIZE];
+    } pop;
 };

 const char *virtio_device_names[] = {
@@ -1680,8 +1686,8 @@ static void *virtqueue_split_pop(VirtQueue *vq, 
size_t sz)
     VirtIODevice *vdev = vq->vdev;
     VirtQueueElement *elem = NULL;
     unsigned out_num, in_num, elem_entries;
-    hwaddr addr[VIRTQUEUE_MAX_SIZE];
-    struct iovec iov[VIRTQUEUE_MAX_SIZE];
+    hwaddr *addr = vq->pop.addr;
+    struct iovec *iov = vq->pop.iov;
     VRingDesc desc;
     int rc;

@@ -1826,8 +1832,8 @@ static void *virtqueue_packed_pop(VirtQueue *vq, 
size_t sz)
     VirtIODevice *vdev = vq->vdev;
     VirtQueueElement *elem = NULL;
     unsigned out_num, in_num, elem_entries;
-    hwaddr addr[VIRTQUEUE_MAX_SIZE];
-    struct iovec iov[VIRTQUEUE_MAX_SIZE];
+    hwaddr *addr = vq->pop.addr;
+    struct iovec *iov = vq->pop.iov;
     VRingPackedDesc desc;
     uint16_t id;
     int rc;
---



Fixes: 7ff9ff039380 ("meson: mitigate against use of uninitialize stack for 
exploits")
Cc: Daniel P. Berrangé <berra...@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
---
  include/qemu/compiler.h | 12 ++++++++++++
  hw/virtio/virtio.c      |  8 ++++----
  2 files changed, 16 insertions(+), 4 deletions(-)