Re: [RFC PATCH 09/27] vhost: Route host->guest notification through qemu

From: Stefan Hajnoczi <stefanha@gmail.com>
To: "Eugenio Pérez" <eperezma@redhat.com>
Cc: kvm@vger.kernel.org, "Michael S. Tsirkin" <mst@redhat.com>,
	Jason Wang <jasowang@redhat.com>,
	qemu-devel@nongnu.org, Daniel Daly <dandaly0@gmail.com>,
	virtualization@lists.linux-foundation.org,
	Liran Alon <liralon@gmail.com>, Eli Cohen <eli@mellanox.com>,
	Nitin Shrivastav <nitin.shrivastav@broadcom.com>,
	Alex Barba <alex.barba@broadcom.com>,
	Christophe Fontaine <cfontain@redhat.com>,
	Juan Quintela <quintela@redhat.com>,
	Lee Ballard <ballle98@gmail.com>,
	Lars Ganrot <lars.ganrot@gmail.com>,
	Rob Miller <rob.miller@broadcom.com>,
	Stefano Garzarella <sgarzare@redhat.com>,
	Howard Cai <howard.cai@gmail.com>,
	Parav Pandit <parav@mellanox.com>, vm <vmireyno@marvell.com>,
	Salil Mehta <mehta.salil.lnk@gmail.com>,
	Stephen Finucane <stephenfin@redhat.com>,
	Xiao W Wang <xiao.w.wang@intel.com>,
	Sean Mooney <smooney@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Jim Harford <jim.harford@broadcom.com>,
	Dmytro Kazantsev <dmytro.kazantsev@gmail.com>,
	Siwei Liu <loseweigh@gmail.com>,
	Harpreet Singh Anand <hanand@xilinx.com>,
	Michael Lilja <ml@napatech.com>, Max Gurtovoy <maxgu14@gmail.com>
Subject: Re: [RFC PATCH 09/27] vhost: Route host->guest notification through qemu
Date: Tue, 8 Dec 2020 07:34:46 +0000	[thread overview]
Message-ID: <20201208073446.GP203660@stefanha-x1.localdomain> (raw)
In-Reply-To: <20201120185105.279030-10-eperezma@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2197 bytes --]

On Fri, Nov 20, 2020 at 07:50:47PM +0100, Eugenio Pérez wrote:
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  hw/virtio/vhost-sw-lm-ring.c |  3 +++
>  hw/virtio/vhost.c            | 20 ++++++++++++++++++++
>  2 files changed, 23 insertions(+)

I'm not sure I understand what is going here. The guest notifier masking
feature exists to support MSI masking semantics. It looks like this
patch repurposes the notifier to decouple the vhost hdev from the virtio
device's irqfd? But this breaks MSI masking. I think you need to set up
your own eventfd and assign it to the vhost hdev's call fd instead of
using the mask notifier.

> 
> diff --git a/hw/virtio/vhost-sw-lm-ring.c b/hw/virtio/vhost-sw-lm-ring.c
> index 0192e77831..cbf53965cd 100644
> --- a/hw/virtio/vhost-sw-lm-ring.c
> +++ b/hw/virtio/vhost-sw-lm-ring.c
> @@ -50,6 +50,9 @@ VhostShadowVirtqueue *vhost_sw_lm_shadow_vq(struct vhost_dev *dev, int idx)
>      r = dev->vhost_ops->vhost_set_vring_kick(dev, &file);
>      assert(r == 0);
>  
> +    vhost_virtqueue_mask(dev, dev->vdev, idx, true);
> +    vhost_virtqueue_pending(dev, idx);

Why is the mask notifier cleared? Could we lose a guest notification
here?

> +
>      return svq;
>  }
>  
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 1d55e26d45..9352c56bfa 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -960,12 +960,29 @@ static void handle_sw_lm_vq(VirtIODevice *vdev, VirtQueue *vq)
>      vhost_vring_kick(svq);
>  }
>  
> +static void vhost_handle_call(EventNotifier *n)
> +{
> +    struct vhost_virtqueue *hvq = container_of(n,
> +                                              struct vhost_virtqueue,
> +                                              masked_notifier);
> +    struct vhost_dev *vdev = hvq->dev;
> +    int idx = vdev->vq_index + (hvq == &vdev->vqs[0] ? 0 : 1);

vhost-net-specific hack

> +    VirtQueue *vq = virtio_get_queue(vdev->vdev, idx);
> +
> +    if (event_notifier_test_and_clear(n)) {
> +        virtio_queue_invalidate_signalled_used(vdev->vdev, idx);
> +        virtio_notify_irqfd(vdev->vdev, vq);

/* TODO push used elements into vq? */

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]