qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: sgarzare@redhat.com, raphael@enfabrica.net,
	qemu-devel@nongnu.org, yc-core@yandex-team.ru,
	d-tatianin@yandex-team.ru
Subject: Re: [PATCH v3 07/23] virtio: move common part of _set_guest_notifier to generic code
Date: Fri, 7 Nov 2025 08:01:35 -0500	[thread overview]
Message-ID: <20251107075743-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20251015145808.1112843-8-vsementsov@yandex-team.ru>

On Wed, Oct 15, 2025 at 05:57:51PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> virtio-pci and virtio-mmio 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 <vsementsov@yandex-team.ru>
> Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru>


Breaks virtio-ccw which also uses this.

> ---
>  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 +++++++++---
>  5 files changed, 67 insertions(+), 75 deletions(-)
> 
> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> index fb58c36452..1f1d96129b 100644
> --- a/hw/virtio/virtio-mmio.c
> +++ b/hw/virtio/virtio-mmio.c
> @@ -659,18 +659,11 @@ static int virtio_mmio_set_guest_notifier(DeviceState *d, int n, bool assign,
>      VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
>      VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>      VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> -    VirtQueue *vq = virtio_get_queue(vdev, n);
> -    EventNotifier *notifier = virtio_queue_get_guest_notifier(vq);
> +    int r;
>  
> -    if (assign) {
> -        int r = event_notifier_init(notifier, 0);
> -        if (r < 0) {
> -            return r;
> -        }
> -        virtio_queue_set_guest_notifier_fd_handler(vq, true, with_irqfd);
> -    } else {
> -        virtio_queue_set_guest_notifier_fd_handler(vq, false, with_irqfd);
> -        event_notifier_cleanup(notifier);
> +    r = virtio_queue_set_guest_notifier(vdev, n, assign, with_irqfd);
> +    if (r < 0) {
> +        return r;
>      }
>  
>      if (vdc->guest_notifier_mask && vdev->use_guest_notifier_mask) {
> @@ -679,30 +672,7 @@ static int virtio_mmio_set_guest_notifier(DeviceState *d, int n, bool assign,
>  
>      return 0;
>  }
> -static int virtio_mmio_set_config_guest_notifier(DeviceState *d, bool assign,
> -                                                 bool with_irqfd)
> -{
> -    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
> -    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> -    VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> -    EventNotifier *notifier = virtio_config_get_guest_notifier(vdev);
> -    int r = 0;
>  
> -    if (assign) {
> -        r = event_notifier_init(notifier, 0);
> -        if (r < 0) {
> -            return r;
> -        }
> -        virtio_config_set_guest_notifier_fd_handler(vdev, assign, with_irqfd);
> -    } else {
> -        virtio_config_set_guest_notifier_fd_handler(vdev, assign, with_irqfd);
> -        event_notifier_cleanup(notifier);
> -    }
> -    if (vdc->guest_notifier_mask && vdev->use_guest_notifier_mask) {
> -        vdc->guest_notifier_mask(vdev, VIRTIO_CONFIG_IRQ_IDX, !assign);
> -    }
> -    return r;
> -}
>  static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs,
>                                             bool assign)
>  {
> @@ -724,7 +694,8 @@ static int virtio_mmio_set_guest_notifiers(DeviceState *d, int nvqs,
>              goto assign_error;
>          }
>      }
> -    r = virtio_mmio_set_config_guest_notifier(d, assign, with_irqfd);
> +    r = virtio_mmio_set_guest_notifier(d, VIRTIO_CONFIG_IRQ_IDX, assign,
> +                                       with_irqfd);
>      if (r < 0) {
>          goto assign_error;
>      }
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 937e22f08a..6c4576a17f 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1197,43 +1197,17 @@ static void virtio_pci_vector_poll(PCIDevice *dev,
>      }
>  }
>  
> -void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev, 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);
> -    }
> -}
> -
>  static int virtio_pci_set_guest_notifier(DeviceState *d, int n, bool assign,
>                                           bool with_irqfd)
>  {
>      VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d);
>      VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
>      VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> -    VirtQueue *vq = NULL;
> -    EventNotifier *notifier = NULL;
> +    int r;
>  
> -    if (n == VIRTIO_CONFIG_IRQ_IDX) {
> -        notifier = virtio_config_get_guest_notifier(vdev);
> -    } else {
> -        vq = virtio_get_queue(vdev, n);
> -        notifier = virtio_queue_get_guest_notifier(vq);
> -    }
> -
> -    if (assign) {
> -        int r = event_notifier_init(notifier, 0);
> -        if (r < 0) {
> -            return r;
> -        }
> -        virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, true, with_irqfd);
> -    } else {
> -        virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, false,
> -                                                 with_irqfd);
> -        event_notifier_cleanup(notifier);
> +    r = virtio_queue_set_guest_notifier(vdev, n, assign, with_irqfd);
> +    if (r < 0) {
> +        return r;
>      }
>  
>      if (!msix_enabled(&proxy->pci_dev) &&
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 153ee0a0cf..704bc7943f 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3803,8 +3803,10 @@ static void virtio_config_guest_notifier_read(EventNotifier *n)
>          virtio_notify_config(vdev);
>      }
>  }
> -void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
> -                                                bool with_irqfd)
> +
> +static void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq,
> +                                                       bool assign,
> +                                                       bool with_irqfd)
>  {
>      if (assign && !with_irqfd) {
>          event_notifier_set_handler(&vq->guest_notifier,
> @@ -3819,7 +3821,7 @@ void virtio_queue_set_guest_notifier_fd_handler(VirtQueue *vq, bool assign,
>      }
>  }
>  
> -void virtio_config_set_guest_notifier_fd_handler(VirtIODevice *vdev,
> +static void virtio_config_set_guest_notifier_fd_handler(VirtIODevice *vdev,
>                                                   bool assign, bool with_irqfd)
>  {
>      EventNotifier *n;
> @@ -3836,6 +3838,46 @@ void virtio_config_set_guest_notifier_fd_handler(VirtIODevice *vdev,
>      }
>  }
>  
> +static void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev,
> +                                                     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);
> +    }
> +}
> +
> +int virtio_queue_set_guest_notifier(VirtIODevice *vdev, int n, bool assign,
> +                                    bool with_irqfd)
> +{
> +    VirtQueue *vq = NULL;
> +    EventNotifier *notifier = NULL;
> +
> +    if (n == VIRTIO_CONFIG_IRQ_IDX) {
> +        notifier = virtio_config_get_guest_notifier(vdev);
> +    } else {
> +        vq = virtio_get_queue(vdev, n);
> +        notifier = virtio_queue_get_guest_notifier(vq);
> +    }
> +
> +    if (assign) {
> +        int r = event_notifier_init(notifier, 0);
> +        if (r < 0) {
> +            return r;
> +        }
> +        virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, true, with_irqfd);
> +    } else {
> +        virtio_pci_set_guest_notifier_fd_handler(vdev, vq, n, false,
> +                                                 with_irqfd);
> +        event_notifier_cleanup(notifier);
> +    }
> +
> +    return 0;
> +}
> +
>  EventNotifier *virtio_queue_get_guest_notifier(VirtQueue *vq)
>  {
>      return &vq->guest_notifier;
> diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
> index 639752977e..2a5b65f374 100644
> --- a/include/hw/virtio/virtio-pci.h
> +++ b/include/hw/virtio/virtio-pci.h
> @@ -263,9 +263,6 @@ void virtio_pci_types_register(const VirtioPCIDeviceTypeInfo *t);
>   * @fixed_queues.
>   */
>  unsigned virtio_pci_optimal_num_queues(unsigned fixed_queues);
> -void virtio_pci_set_guest_notifier_fd_handler(VirtIODevice *vdev, VirtQueue *vq,
> -                                              int n, bool assign,
> -                                              bool with_irqfd);
>  
>  int virtio_pci_add_shm_cap(VirtIOPCIProxy *proxy, uint8_t bar, uint64_t offset,
>                             uint64_t length, uint8_t id);
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index d97529c3f1..7db8046766 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
> + * @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);
>  
>  static inline void virtio_add_feature(uint64_t *features, unsigned int fbit)
>  {
> -- 
> 2.48.1



  reply	other threads:[~2025-11-07 13:02 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-15 14:57 [PATCH v3 00/23] vhost refactoring and fixes Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 01/23] vhost-user: rework enabling vrings Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 02/23] vhost: drop backend_features field Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 03/23] vhost-user: introduce vhost_user_has_protocol_feature() helper Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 04/23] vhost: move protocol_features to vhost_user Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 05/23] vhost-user-gpu: drop code duplication Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 06/23] vhost: make vhost_dev.features private Vladimir Sementsov-Ogievskiy
2025-10-17 10:32   ` Daniil Tatianin
2025-10-17 17:02     ` Vladimir Sementsov-Ogievskiy
2025-10-31 17:31       ` Daniil Tatianin
2025-10-15 14:57 ` [PATCH v3 07/23] virtio: move common part of _set_guest_notifier to generic code Vladimir Sementsov-Ogievskiy
2025-11-07 13:01   ` Michael S. Tsirkin [this message]
2025-10-15 14:57 ` [PATCH v3 08/23] virtio: drop *_set_guest_notifier_fd_handler() helpers Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 09/23] vhost-user: keep QIOChannelSocket for backend channel Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 10/23] vhost: vhost_virtqueue_start(): fix failure path Vladimir Sementsov-Ogievskiy
2025-10-20 18:24   ` Raphael Norwitz
2025-10-15 14:57 ` [PATCH v3 11/23] vhost: make vhost_memory_unmap() null-safe Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 12/23] vhost: simplify calls to vhost_memory_unmap() Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 13/23] vhost: move vrings mapping to the top of vhost_virtqueue_start() Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 14/23] vhost: vhost_virtqueue_start(): drop extra local variables Vladimir Sementsov-Ogievskiy
2025-10-15 14:57 ` [PATCH v3 15/23] vhost: final refactoring of vhost vrings map/unmap Vladimir Sementsov-Ogievskiy
2025-10-15 14:58 ` [PATCH v3 16/23] vhost: simplify vhost_dev_init() error-path Vladimir Sementsov-Ogievskiy
2025-10-15 14:58 ` [PATCH v3 17/23] vhost: move busyloop timeout initialization to vhost_virtqueue_init() Vladimir Sementsov-Ogievskiy
2025-10-15 14:58 ` [PATCH v3 18/23] vhost: introduce check_memslots() helper Vladimir Sementsov-Ogievskiy
2025-11-03 22:19   ` Michael S. Tsirkin
2025-11-04 10:59     ` Vladimir Sementsov-Ogievskiy
2025-10-15 14:58 ` [PATCH v3 19/23] vhost: vhost_dev_init(): simplify features initialization Vladimir Sementsov-Ogievskiy
2025-10-15 14:58 ` [PATCH v3 20/23] hw/virtio/virtio-bus: refactor virtio_bus_set_host_notifier() Vladimir Sementsov-Ogievskiy
2025-10-15 14:58 ` [PATCH v3 21/23] vhost-user: make trace events more readable Vladimir Sementsov-Ogievskiy
2025-11-03 22:22   ` Michael S. Tsirkin
2025-10-15 14:58 ` [PATCH v3 22/23] vhost-user-blk: add some useful trace-points Vladimir Sementsov-Ogievskiy
2025-11-03 21:28   ` Michael S. Tsirkin
2025-11-04 11:08     ` Vladimir Sementsov-Ogievskiy
2025-10-15 14:58 ` [PATCH v3 23/23] vhost: " Vladimir Sementsov-Ogievskiy
2025-11-03 21:26   ` Michael S. Tsirkin
2025-10-18 15:33 ` [PATCH v3 00/23] vhost refactoring and fixes Lei Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251107075743-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=d-tatianin@yandex-team.ru \
    --cc=qemu-devel@nongnu.org \
    --cc=raphael@enfabrica.net \
    --cc=sgarzare@redhat.com \
    --cc=vsementsov@yandex-team.ru \
    --cc=yc-core@yandex-team.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).