On 24.01.26 02:06, Halil Pasic wrote:
On Mon, 19 Jan 2026 21:52:12 +0300
Vladimir Sementsov-Ogievskiy <[email protected]> wrote:
virtio-pci, virtio-mmio and virtio-ccw handle config notifier equally
but with different code (mmio adds a separate function, when pci use
common function). Let's chose the more compact way (pci) and reuse it
for mmio.
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Ultimately is up to Michael to decide, but I'm not sure patch makes
things cleaner. I would even say, its current form, it does not.
I do like the idea of getting rid of duplicated logic though.
---
hw/s390x/virtio-ccw.c | 35 ++++++++++++-------------
hw/virtio/virtio-mmio.c | 41 +++++------------------------
hw/virtio/virtio-pci.c | 34 +++---------------------
hw/virtio/virtio.c | 48 +++++++++++++++++++++++++++++++---
include/hw/virtio/virtio-pci.h | 3 ---
include/hw/virtio/virtio.h | 16 +++++++++---
6 files changed, 84 insertions(+), 93 deletions(-)
[..]
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 3dc9423eae..ea43ea620c 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
[..]
+static void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev,
Function name says 'pci' but it actually is not PCI specific.
Agree. Is "virtio_set_guest_notifier_fd_handler()" OK?
+ VirtQueue *vq,
+ int n, bool assign,
+ bool with_irqfd)
+{
+ if (n == VIRTIO_CONFIG_IRQ_IDX) {
+ virtio_config_set_guest_notifier_fd_handler(vdev, assign, with_irqfd);
+ } else {
+ virtio_queue_set_guest_notifier_fd_handler(vq, assign, with_irqfd);
+ }
+}
+
[..]
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 27cd98d2fe..25ef7c9dff 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -420,8 +420,6 @@ void virtio_queue_update_used_idx(VirtIODevice *vdev, int
n);
VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n);
uint16_t virtio_get_queue_index(VirtQueue *vq);
EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq);
-void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
- bool with_irqfd);
int virtio_device_start_ioeventfd(VirtIODevice *vdev);
int virtio_device_grab_ioeventfd(VirtIODevice *vdev);
void virtio_device_release_ioeventfd(VirtIODevice *vdev);
@@ -435,8 +433,18 @@ void virtio_queue_aio_detach_host_notifier(VirtQueue *vq,
AioContext *ctx);
VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t vector);
VirtQueue *virtio_vector_next_queue(VirtQueue *vq);
EventNotifier *virtio_config_get_guest_notifier(VirtIODevice *vdev);
-void virtio_config_set_guest_notifier_fd_handler(VirtIODevice *vdev,
- bool assign, bool with_irqfd);
+
+/**
+ * virtio_queue_set_guest_notifier - set/unset queue or config guest
+ * notifier
+ *
+ * @vdev: the VirtIO device
+ * @n: queue number, or VIRTIO_CONFIG_IRQ_IDX to set config notifer
Semantically are mixing queue numbers and interrupt indexes here.
That's preexisting logic in virtio_pci_set_guest_notifier_fd_handler().
But, you are right, that's doesn't answer the question, is it good to share
the logic with mmio and ccw.
Is config notifier only pci specific, or similar feature may appear in future
for mmio and/or ccw?
I can try to keep config-notifier logic only in pci code.
Michael, what's better?
+ * @assign: true to set notifier, false to unset
+ * @with_irqfd: irqfd enabled
+ */
+int virtio_queue_set_guest_notifier(VirtIODevice *vdev, int n, bool assign,
+ bool with_irqfd);
The name says 'queue' but it is not always queue any more.
Agree, should be "virtio_set_guest_notifier()" if we go that way.
Regards,
Halil
Thanks for reviewing!
--
Best regards,
Vladimir
