diff --git a/debian/patches/extra/0010-virtio-scsi-Attach-event-vq-notifier-with-no_poll.patch b/debian/patches/extra/0010-virtio-scsi-Attach-event-vq-notifier-with-no_poll.patch new file mode 100644 index 0000000..85d80e8 --- /dev/null +++ b/debian/patches/extra/0010-virtio-scsi-Attach-event-vq-notifier-with-no_poll.patch @@ -0,0 +1,65 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Hanna Czenczek +Date: Fri, 2 Feb 2024 16:31:56 +0100 +Subject: [PATCH] virtio-scsi: Attach event vq notifier with no_poll + +As of commit 38738f7dbbda90fbc161757b7f4be35b52205552 ("virtio-scsi: +don't waste CPU polling the event virtqueue"), we only attach an io_read +notifier for the virtio-scsi event virtqueue instead, and no polling +notifiers. During operation, the event virtqueue is typically +non-empty, but none of the buffers are intended to be used immediately. +Instead, they only get used when certain events occur. Therefore, it +makes no sense to continuously poll it when non-empty, because it is +supposed to be and stay non-empty. + +We do this by using virtio_queue_aio_attach_host_notifier_no_poll() +instead of virtio_queue_aio_attach_host_notifier() for the event +virtqueue. + +Commit 766aa2de0f29b657148e04599320d771c36fd126 ("virtio-scsi: implement +BlockDevOps->drained_begin()") however has virtio_scsi_drained_end() use +virtio_queue_aio_attach_host_notifier() for all virtqueues, including +the event virtqueue. This can lead to it being polled again, undoing +the benefit of commit 38738f7dbbda90fbc161757b7f4be35b52205552. + +Fix it by using virtio_queue_aio_attach_host_notifier_no_poll() for the +event virtqueue. + + ("virtio-scsi: implement BlockDevOps->drained_begin()") + +Reported-by: Fiona Ebner +Fixes: 766aa2de0f29b657148e04599320d771c36fd126 +Reviewed-by: Stefan Hajnoczi +Tested-by: Fiona Ebner +Reviewed-by: Fiona Ebner +Signed-off-by: Hanna Czenczek +Signed-off-by: Thomas Lamprecht +--- + hw/scsi/virtio-scsi.c | 7 ++++++- + 1 file changed, 6 insertions(+), 1 deletion(-) + +diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c +index 45b95ea070..ad24a882fd 100644 +--- a/hw/scsi/virtio-scsi.c ++++ b/hw/scsi/virtio-scsi.c +@@ -1148,6 +1148,7 @@ static void virtio_scsi_drained_begin(SCSIBus *bus) + static void virtio_scsi_drained_end(SCSIBus *bus) + { + VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus); ++ VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s); + VirtIODevice *vdev = VIRTIO_DEVICE(s); + uint32_t total_queues = VIRTIO_SCSI_VQ_NUM_FIXED + + s->parent_obj.conf.num_queues; +@@ -1165,7 +1166,11 @@ static void virtio_scsi_drained_end(SCSIBus *bus) + + for (uint32_t i = 0; i < total_queues; i++) { + VirtQueue *vq = virtio_get_queue(vdev, i); +- virtio_queue_aio_attach_host_notifier(vq, s->ctx); ++ if (vq == vs->event_vq) { ++ virtio_queue_aio_attach_host_notifier_no_poll(vq, s->ctx); ++ } else { ++ virtio_queue_aio_attach_host_notifier(vq, s->ctx); ++ } + } + } + diff --git a/debian/patches/extra/0011-virtio-Re-enable-notifications-after-drain.patch b/debian/patches/extra/0011-virtio-Re-enable-notifications-after-drain.patch new file mode 100644 index 0000000..618ccb2 --- /dev/null +++ b/debian/patches/extra/0011-virtio-Re-enable-notifications-after-drain.patch @@ -0,0 +1,125 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Hanna Czenczek +Date: Fri, 2 Feb 2024 16:31:57 +0100 +Subject: [PATCH] virtio: Re-enable notifications after drain + +During drain, we do not care about virtqueue notifications, which is why +we remove the handlers on it. When removing those handlers, whether vq +notifications are enabled or not depends on whether we were in polling +mode or not; if not, they are enabled (by default); if so, they have +been disabled by the io_poll_start callback. + +Because we do not care about those notifications after removing the +handlers, this is fine. However, we have to explicitly ensure they are +enabled when re-attaching the handlers, so we will resume receiving +notifications. We do this in virtio_queue_aio_attach_host_notifier*(). +If such a function is called while we are in a polling section, +attaching the notifiers will then invoke the io_poll_start callback, +re-disabling notifications. + +Because we will always miss virtqueue updates in the drained section, we +also need to poll the virtqueue once after attaching the notifiers. + +Buglink: https://issues.redhat.com/browse/RHEL-3934 +Signed-off-by: Hanna Czenczek +Signed-off-by: Thomas Lamprecht +--- + hw/virtio/virtio.c | 42 ++++++++++++++++++++++++++++++++++++++++++ + include/block/aio.h | 7 ++++++- + 2 files changed, 48 insertions(+), 1 deletion(-) + +diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c +index 969c25f4cf..02cce83111 100644 +--- a/hw/virtio/virtio.c ++++ b/hw/virtio/virtio.c +@@ -3526,6 +3526,17 @@ static void virtio_queue_host_notifier_aio_poll_end(EventNotifier *n) + + void virtio_queue_aio_attach_host_notifier(VirtQueue *vq, AioContext *ctx) + { ++ /* ++ * virtio_queue_aio_detach_host_notifier() can leave notifications disabled. ++ * Re-enable them. (And if detach has not been used before, notifications ++ * being enabled is still the default state while a notifier is attached; ++ * see virtio_queue_host_notifier_aio_poll_end(), which will always leave ++ * notifications enabled once the polling section is left.) ++ */ ++ if (!virtio_queue_get_notification(vq)) { ++ virtio_queue_set_notification(vq, 1); ++ } ++ + aio_set_event_notifier(ctx, &vq->host_notifier, + virtio_queue_host_notifier_read, + virtio_queue_host_notifier_aio_poll, +@@ -3533,6 +3544,13 @@ void virtio_queue_aio_attach_host_notifier(VirtQueue *vq, AioContext *ctx) + aio_set_event_notifier_poll(ctx, &vq->host_notifier, + virtio_queue_host_notifier_aio_poll_begin, + virtio_queue_host_notifier_aio_poll_end); ++ ++ /* ++ * We will have ignored notifications about new requests from the guest ++ * while no notifiers were attached, so "kick" the virt queue to process ++ * those requests now. ++ */ ++ event_notifier_set(&vq->host_notifier); + } + + /* +@@ -3543,14 +3561,38 @@ void virtio_queue_aio_attach_host_notifier(VirtQueue *vq, AioContext *ctx) + */ + void virtio_queue_aio_attach_host_notifier_no_poll(VirtQueue *vq, AioContext *ctx) + { ++ /* See virtio_queue_aio_attach_host_notifier() */ ++ if (!virtio_queue_get_notification(vq)) { ++ virtio_queue_set_notification(vq, 1); ++ } ++ + aio_set_event_notifier(ctx, &vq->host_notifier, + virtio_queue_host_notifier_read, + NULL, NULL); ++ ++ /* ++ * See virtio_queue_aio_attach_host_notifier(). ++ * Note that this may be unnecessary for the type of virtqueues this ++ * function is used for. Still, it will not hurt to have a quick look into ++ * whether we can/should process any of the virtqueue elements. ++ */ ++ event_notifier_set(&vq->host_notifier); + } + + void virtio_queue_aio_detach_host_notifier(VirtQueue *vq, AioContext *ctx) + { + aio_set_event_notifier(ctx, &vq->host_notifier, NULL, NULL, NULL); ++ ++ /* ++ * aio_set_event_notifier_poll() does not guarantee whether io_poll_end() ++ * will run after io_poll_begin(), so by removing the notifier, we do not ++ * know whether virtio_queue_host_notifier_aio_poll_end() has run after a ++ * previous virtio_queue_host_notifier_aio_poll_begin(), i.e. whether ++ * notifications are enabled or disabled. It does not really matter anyway; ++ * we just removed the notifier, so we do not care about notifications until ++ * we potentially re-attach it. The attach_host_notifier functions will ++ * ensure that notifications are enabled again when they are needed. ++ */ + } + + void virtio_queue_host_notifier_read(EventNotifier *n) +diff --git a/include/block/aio.h b/include/block/aio.h +index 32042e8905..79efadfa48 100644 +--- a/include/block/aio.h ++++ b/include/block/aio.h +@@ -498,9 +498,14 @@ void aio_set_event_notifier(AioContext *ctx, + AioPollFn *io_poll, + EventNotifierHandler *io_poll_ready); + +-/* Set polling begin/end callbacks for an event notifier that has already been ++/* ++ * Set polling begin/end callbacks for an event notifier that has already been + * registered with aio_set_event_notifier. Do nothing if the event notifier is + * not registered. ++ * ++ * Note that if the io_poll_end() callback (or the entire notifier) is removed ++ * during polling, it will not be called, so an io_poll_begin() is not ++ * necessarily always followed by an io_poll_end(). + */ + void aio_set_event_notifier_poll(AioContext *ctx, + EventNotifier *notifier, diff --git a/debian/patches/series b/debian/patches/series index 381ff8c..ee0028d 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -7,6 +7,8 @@ extra/0006-migration-states-workaround-snapshot-performance-reg.patch extra/0007-Revert-x86-acpi-workaround-Windows-not-handling-name.patch extra/0008-target-i386-the-sgx_epc_get_section-stub-is-reachabl.patch extra/0009-ui-clipboard-mark-type-as-not-available-when-there-i.patch +extra/0010-virtio-scsi-Attach-event-vq-notifier-with-no_poll.patch +extra/0011-virtio-Re-enable-notifications-after-drain.patch bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch