Linux virtualization list
 help / color / mirror / Atom feed
* Re: [RFC v2 1/7] vhost: Delete trailing dot in errpr_setg argument
       [not found]     ` <CAJaqyWd+NuY3B94HX8J_EzFf4sxeZnFCcw=aXfT-KZ7nfmL6DQ@mail.gmail.com>
@ 2021-02-11 16:40       ` Stefano Garzarella
  0 siblings, 0 replies; 8+ messages in thread
From: Stefano Garzarella @ 2021-02-11 16:40 UTC (permalink / raw)
  To: Eugenio Perez Martin
  Cc: Parav Pandit, Michael S. Tsirkin, qemu-level,
	Harpreet Singh Anand, Xiao W Wang, Stefan Hajnoczi, Eli Cohen,
	virtualization, Eric Blake, Michael Lilja, Jim Harford,
	Rob Miller

On Tue, Feb 09, 2021 at 07:11:41PM +0100, Eugenio Perez Martin wrote:
>On Tue, Feb 9, 2021 at 5:25 PM Eric Blake <eblake@redhat.com> wrote:
>>
>> On 2/9/21 9:37 AM, Eugenio Pérez wrote:
>> > As error_setg points
>>
>> Incomplete sentence?
>>
>> Missing Signed-off-by.
>>
>
>Sorry, I should have paid more attention.
>
>Maybe it is better to send this though qemu-trivial, so it does not
>mess with this series?

Yes, I agree that it can go regardless of this series.

Thanks,
Stefano

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v2 2/7] virtio: Add virtio_queue_host_notifier_status
       [not found] ` <20210209153757.1653598-3-eperezma@redhat.com>
@ 2021-02-17 12:41   ` Stefan Hajnoczi
  0 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2021-02-17 12:41 UTC (permalink / raw)
  To: Eugenio Pérez
  Cc: Rob Miller, Parav Pandit, Michael S. Tsirkin, qemu-devel,
	Harpreet Singh Anand, Xiao W Wang, Eli Cohen, virtualization,
	Eric Blake, Michael Lilja, Jim Harford


[-- Attachment #1.1: Type: text/plain, Size: 1831 bytes --]

On Tue, Feb 09, 2021 at 04:37:52PM +0100, Eugenio Pérez wrote:
> This allows shadow virtqueue code to assert the queue status before
> making changes.
> 
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  include/hw/virtio/virtio.h | 1 +
>  hw/virtio/virtio.c         | 5 +++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b7ece7a6a8..227cec13a8 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -316,6 +316,7 @@ void virtio_device_release_ioeventfd(VirtIODevice *vdev);
>  bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev);
>  EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
>  void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled);
> +bool virtio_queue_host_notifier_status(const VirtQueue *vq);
>  void virtio_queue_host_notifier_read(EventNotifier *n);
>  void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
>                                                  VirtIOHandleAIOOutput handle_output);
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 1fd1917ca0..53473ae4df 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3594,6 +3594,11 @@ EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq)
>      return &vq->host_notifier;
>  }
>  
> +bool virtio_queue_host_notifier_status(const VirtQueue *vq)
> +{
> +    return vq->host_notifier_enabled;
> +}
> +
>  void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled)

Since there is a virtio_queue_set_host_notifier_enabled() I suggest
calling this function virtio_queue_is_host_notifier_enabled() or
virtio_queue_get_host_notifier_enabled(). That way it's clear they
set/get the same thing.

Stefan

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

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v2 3/7] vhost: Save masked_notifier state
       [not found] ` <20210209153757.1653598-4-eperezma@redhat.com>
@ 2021-02-17 12:44   ` Stefan Hajnoczi
  0 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2021-02-17 12:44 UTC (permalink / raw)
  To: Eugenio Pérez
  Cc: Rob Miller, Parav Pandit, Michael S. Tsirkin, qemu-devel,
	Harpreet Singh Anand, Xiao W Wang, Eli Cohen, virtualization,
	Eric Blake, Michael Lilja, Jim Harford


[-- Attachment #1.1: Type: text/plain, Size: 337 bytes --]

On Tue, Feb 09, 2021 at 04:37:53PM +0100, Eugenio Pérez wrote:
> It will be used to recover call eventfd.
> 
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  include/hw/virtio/vhost.h | 1 +
>  hw/virtio/vhost.c         | 2 ++
>  2 files changed, 3 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

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

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v2 4/7] vhost: Add VhostShadowVirtqueue
       [not found] ` <20210209153757.1653598-5-eperezma@redhat.com>
@ 2021-02-17 13:01   ` Stefan Hajnoczi
  0 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2021-02-17 13:01 UTC (permalink / raw)
  To: Eugenio Pérez
  Cc: Rob Miller, Parav Pandit, Michael S. Tsirkin, qemu-devel,
	Harpreet Singh Anand, Xiao W Wang, Eli Cohen, virtualization,
	Eric Blake, Michael Lilja, Jim Harford


[-- Attachment #1.1: Type: text/plain, Size: 1077 bytes --]

On Tue, Feb 09, 2021 at 04:37:54PM +0100, Eugenio Pérez wrote:
> +/*
> + * Creates vhost shadow virtqueue, and instruct vhost device to use the shadow
> + * methods and file descriptors.
> + */
> +VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx)
> +{
> +    g_autofree VhostShadowVirtqueue *svq = g_new0(VhostShadowVirtqueue, 1);
> +    int r;
> +
> +    r = event_notifier_init(&svq->kick_notifier, 0);
> +    if (r != 0) {
> +        error_report("Couldn't create kick event notifier: %s",
> +                     strerror(errno));
> +        goto err_init_kick_notifier;
> +    }
> +
> +    r = event_notifier_init(&svq->call_notifier, 0);
> +    if (r != 0) {
> +        error_report("Couldn't create call event notifier: %s",
> +                     strerror(errno));
> +        goto err_init_call_notifier;
> +    }
> +
> +    return svq;

Use-after-free due to g_autofree. I think this should be:

  return g_steal_pointer(&svq)

https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html#g-steal-pointer

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

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v2 5/7] vhost: Add x-vhost-enable-shadow-vq qmp
       [not found] ` <20210209153757.1653598-6-eperezma@redhat.com>
@ 2021-02-17 15:26   ` Stefan Hajnoczi
  0 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2021-02-17 15:26 UTC (permalink / raw)
  To: Eugenio Pérez
  Cc: Rob Miller, Parav Pandit, Michael S. Tsirkin, qemu-devel,
	Harpreet Singh Anand, Xiao W Wang, Eli Cohen, virtualization,
	Eric Blake, Michael Lilja, Jim Harford


[-- Attachment #1.1: Type: text/plain, Size: 1190 bytes --]

On Tue, Feb 09, 2021 at 04:37:55PM +0100, Eugenio Pérez wrote:
> diff --git a/qapi/net.json b/qapi/net.json
> index c31748c87f..a1cdffb0f9 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -77,6 +77,28 @@
>  ##
>  { 'command': 'netdev_del', 'data': {'id': 'str'} }
>  
> +##
> +# @x-vhost-enable-shadow-vq:
> +#
> +# Use vhost shadow virtqueue.

Is this command for testing only or do you expect it to be invoked by
libvirt in production? I think the shadow virtqueue can be an internal
QEMU feature that is hidden from management tools.

> +#
> +# @name: the device name of the virtual network adapter
> +#
> +# @enable: true to use he alternate shadow VQ notification path
> +#
> +# Returns: Error if failure, or 'no error' for success
> +#
> +# Since: 6.0

Is this a generic feature for any vhost or vDPA device? If yes, please
replace "virtual network adapter" in the doc comment.

Does this only apply to vhost-net devices? If so, please put "vhost-net"
in the name since there are other non-net vhost devices.

> +#
> +# Example:
> +#
> +# -> { "execute": "x-vhost-enable-shadow-vq", "arguments": {"enable": true} }

Missing "name" field?

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

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v2 6/7] vhost: Route guest->host notification through shadow virtqueue
       [not found] ` <20210209153757.1653598-7-eperezma@redhat.com>
@ 2021-02-17 16:56   ` Stefan Hajnoczi
  0 siblings, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2021-02-17 16:56 UTC (permalink / raw)
  To: Eugenio Pérez
  Cc: Rob Miller, Parav Pandit, Michael S. Tsirkin, qemu-devel,
	Harpreet Singh Anand, Xiao W Wang, Eli Cohen, virtualization,
	Eric Blake, Michael Lilja, Jim Harford


[-- Attachment #1.1: Type: text/plain, Size: 7068 bytes --]

On Tue, Feb 09, 2021 at 04:37:56PM +0100, Eugenio Pérez wrote:
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index ac963bf23d..884818b109 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -55,6 +55,8 @@ struct vhost_iommu {
>      QLIST_ENTRY(vhost_iommu) iommu_next;
>  };
>  
> +typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;

There is already another declaration in
hw/virtio/vhost-shadow-virtqueue.h. Should vhost.h include
vhost-shadow-virtqueue.h?

This is becoming confusing:
1. typedef in vhost-shadow-virtqueue.h
2. typedef in vhost.h
3. typedef in vhost-shadow-virtqueue.c

3 typedefs is a bit much :). I suggest:
1. typedef in vhost-shadow-virtqueue.h
2. #include "vhost-shadow-virtqueue.h" in vhost.h
3. struct VhostShadowVirtqueue (no typedef redefinition) in vhost-shadow-virtqueue.c

That should make the code easier to understand, navigate with tools, and
if a change is made (e.g. renaming the struct) then it won't be
necessary to change things in 3 places.

> +
>  typedef struct VhostDevConfigOps {
>      /* Vhost device config space changed callback
>       */
> @@ -83,7 +85,9 @@ struct vhost_dev {
>      uint64_t backend_cap;
>      bool started;
>      bool log_enabled;
> +    bool sw_lm_enabled;

Rename to shadow_vqs_enabled?

>      uint64_t log_size;
> +    VhostShadowVirtqueue **shadow_vqs;
>      Error *migration_blocker;
>      const VhostOps *vhost_ops;
>      void *opaque;
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> index b5d2645ae0..01f282d434 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -8,9 +8,12 @@
>   */
>  
>  #include "hw/virtio/vhost-shadow-virtqueue.h"
> +#include "hw/virtio/vhost.h"
> +
> +#include "standard-headers/linux/vhost_types.h"
>  
>  #include "qemu/error-report.h"
> -#include "qemu/event_notifier.h"
> +#include "qemu/main-loop.h"
>  
>  /* Shadow virtqueue to relay notifications */
>  typedef struct VhostShadowVirtqueue {
> @@ -18,8 +21,95 @@ typedef struct VhostShadowVirtqueue {
>      EventNotifier kick_notifier;
>      /* Shadow call notifier, sent to vhost */
>      EventNotifier call_notifier;
> +
> +    /* Borrowed virtqueue's guest to host notifier. */
> +    EventNotifier host_notifier;

The purpose of these EventNotifier fields is not completely clear to me.
Here is how I interpret the comments:

1. The vhost device is set up to use kick_notifier/call_notifier when
   the shadow vq is enabled.

2. host_notifier is the guest-visible vq's host notifier. This is set up
   when the shadow vq is enabled.

But I'm not confident this is correct. Maybe you could expand the
comment to make it clear what is happening?

> +
> +    /* Virtio queue shadowing */
> +    VirtQueue *vq;
>  } VhostShadowVirtqueue;
>  
> +/* Forward guest notifications */
> +static void vhost_handle_guest_kick(EventNotifier *n)
> +{
> +    VhostShadowVirtqueue *svq = container_of(n, VhostShadowVirtqueue,
> +                                             host_notifier);
> +
> +    if (event_notifier_test_and_clear(n)) {
> +        event_notifier_set(&svq->kick_notifier);
> +    }
> +}

This function looks incomplete. You can make review easier by indicating
the state of the code:

  /* TODO pop requests from vq and put them onto vhost vq */

I'm not sure why it's useful to include this incomplete function in the
patch. Maybe the host notifier is already intercepted by the
guest-visible vq is still mapped directly to the vhost vq so this works?
An explanation in comments or the commit description would be helpful.

> +
> +/*
> + * Start shadow virtqueue operation.
> + * @dev vhost device
> + * @hidx vhost virtqueue index
> + * @svq Shadow Virtqueue
> + *
> + * Run in RCU context
> + */
> +bool vhost_shadow_vq_start_rcu(struct vhost_dev *dev,
> +                               unsigned idx,
> +                               VhostShadowVirtqueue *svq)
> +{
> +    EventNotifier *vq_host_notifier = virtio_queue_get_host_notifier(svq->vq);
> +    struct vhost_vring_file kick_file = {
> +        .index = idx,
> +        .fd = event_notifier_get_fd(&svq->kick_notifier),
> +    };
> +    int r;
> +
> +    /* Check that notifications are still going directly to vhost dev */
> +    assert(virtio_queue_host_notifier_status(svq->vq));
> +
> +    event_notifier_init_fd(&svq->host_notifier,
> +                           event_notifier_get_fd(vq_host_notifier));
> +    event_notifier_set_handler(&svq->host_notifier, vhost_handle_guest_kick);

If I understand correctly svq->host_notifier only exists as an easy way
to use container_of() in vhost_handle_guest_kick?

svq->host_notifier does not actually own the fd and therefore
event_notifier_cleanup() must never be called on it?

Please document this.

> +
> +    r = dev->vhost_ops->vhost_set_vring_kick(dev, &kick_file);
> +    if (unlikely(r != 0)) {
> +        error_report("Couldn't set kick fd: %s", strerror(errno));
> +        goto err_set_vring_kick;
> +    }
> +
> +    /* Check for pending notifications from the guest */
> +    vhost_handle_guest_kick(&svq->host_notifier);
> +
> +    return true;

host_notifier is still registered with the vhost device so now the
kernel vhost thread and QEMU are both monitoring the ioeventfd at the
same time? Did I miss a vhost_set_vring_call() somewhere?

> +
> +err_set_vring_kick:
> +    event_notifier_set_handler(&svq->host_notifier, NULL);
> +
> +    return false;
> +}
> +
> +/*
> + * Stop shadow virtqueue operation.
> + * @dev vhost device
> + * @idx vhost queue index
> + * @svq Shadow Virtqueue
> + *
> + * Run in RCU context
> + */
> +void vhost_shadow_vq_stop_rcu(struct vhost_dev *dev,
> +                              unsigned idx,
> +                              VhostShadowVirtqueue *svq)
> +{
> +    EventNotifier *vq_host_notifier = virtio_queue_get_host_notifier(svq->vq);
> +    struct vhost_vring_file kick_file = {
> +        .index = idx,
> +        .fd = event_notifier_get_fd(vq_host_notifier),
> +    };
> +    int r;
> +
> +    /* Restore vhost kick */
> +    r = dev->vhost_ops->vhost_set_vring_kick(dev, &kick_file);
> +    /* Cannot do a lot of things */
> +    assert(r == 0);
> +
> +    event_notifier_set_handler(&svq->host_notifier, NULL);

It may be necessary to call event_notifier_set(vq_host_notifier) before
vhost_set_vring_kick() so that the vhost kernel thread looks at the
vring immediately. That covers the case where svq->kick_notifier was
just set but not yet handled by the vhost kernel thread.

I'm not 100% sure this race condition can occur, but couldn't find
anything that prevents it.

> +err:
> +    for (; idx >= 0; --idx) {
> +        vhost_shadow_vq_free(dev->shadow_vqs[idx]);
> +    }
> +    g_free(dev->shadow_vqs[idx]);

Should this be g_free(dev->shadow_vqs)?

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

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v2 7/7] vhost: Route host->guest notification through shadow virtqueue
       [not found] ` <20210209153757.1653598-8-eperezma@redhat.com>
@ 2021-02-17 17:24   ` Stefan Hajnoczi
  2021-03-01  6:24   ` Jason Wang
  1 sibling, 0 replies; 8+ messages in thread
From: Stefan Hajnoczi @ 2021-02-17 17:24 UTC (permalink / raw)
  To: Eugenio Pérez
  Cc: Rob Miller, Parav Pandit, Michael S. Tsirkin, qemu-devel,
	Harpreet Singh Anand, Xiao W Wang, Eli Cohen, virtualization,
	Eric Blake, Michael Lilja, Jim Harford


[-- Attachment #1.1: Type: text/plain, Size: 1605 bytes --]

On Tue, Feb 09, 2021 at 04:37:57PM +0100, Eugenio Pérez wrote:
> @@ -40,6 +42,26 @@ static void vhost_handle_guest_kick(EventNotifier *n)
>      }
>  }
>  
> +/* Forward vhost notifications */
> +static void vhost_handle_call(EventNotifier *n)

The name vhost_shadow_vq_handle_call() expresses the purpose of the
function more clearly.

> @@ -75,8 +102,19 @@ bool vhost_shadow_vq_start_rcu(struct vhost_dev *dev,
>      /* Check for pending notifications from the guest */
>      vhost_handle_guest_kick(&svq->host_notifier);
>  
> +    r = dev->vhost_ops->vhost_set_vring_call(dev, &call_file);
> +    if (r != 0) {
> +        error_report("Couldn't set call fd: %s", strerror(errno));
> +        goto err_set_vring_call;
> +    }

This ignores notifier_is_masked and always unmasks.

> @@ -1608,6 +1607,10 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
>      if (mask) {
>          assert(vdev->use_guest_notifier_mask);
>          file.fd = event_notifier_get_fd(&hdev->vqs[index].masked_notifier);
> +    } else if (hdev->sw_lm_enabled) {
> +        VhostShadowVirtqueue *svq = hdev->shadow_vqs[n];
> +        EventNotifier *e = vhost_shadow_vq_get_call_notifier(svq);
> +        file.fd = event_notifier_get_fd(e);
>      } else {
>          file.fd = event_notifier_get_fd(virtio_queue_get_guest_notifier(vvq));
>      }

Maybe you can extend this function so it can be called unconditionally
from both vhost_shadow_vq_start_rcu() and vhost_shadow_vq_stop_rcu(). It
would be a single place that invokes vhost_set_vring_call().

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

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC v2 7/7] vhost: Route host->guest notification through shadow virtqueue
       [not found] ` <20210209153757.1653598-8-eperezma@redhat.com>
  2021-02-17 17:24   ` [RFC v2 7/7] vhost: Route host->guest " Stefan Hajnoczi
@ 2021-03-01  6:24   ` Jason Wang
  1 sibling, 0 replies; 8+ messages in thread
From: Jason Wang @ 2021-03-01  6:24 UTC (permalink / raw)
  To: Eugenio Pérez, qemu-devel
  Cc: Parav Pandit, Michael S. Tsirkin, virtualization,
	Harpreet Singh Anand, Xiao W Wang, Stefan Hajnoczi, Eli Cohen,
	Michael Lilja, Jim Harford, Rob Miller


On 2021/2/9 11:37 下午, Eugenio Pérez wrote:
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>   hw/virtio/vhost-shadow-virtqueue.h |  2 ++
>   hw/virtio/vhost-shadow-virtqueue.c | 49 ++++++++++++++++++++++++++++++
>   hw/virtio/vhost.c                  |  5 ++-
>   3 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
> index c45035c4b7..210133434c 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.h
> +++ b/hw/virtio/vhost-shadow-virtqueue.h
> @@ -17,6 +17,8 @@
>   
>   typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
>   
> +EventNotifier *vhost_shadow_vq_get_call_notifier(VhostShadowVirtqueue *vq);
> +
>   bool vhost_shadow_vq_start_rcu(struct vhost_dev *dev,
>                                  unsigned idx,
>                                  VhostShadowVirtqueue *svq);
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> index 01f282d434..61d98ae652 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -24,6 +24,8 @@ typedef struct VhostShadowVirtqueue {
>   
>       /* Borrowed virtqueue's guest to host notifier. */
>       EventNotifier host_notifier;
> +    /* Host to guest notifier */
> +    EventNotifier *guest_notifier;
>   
>       /* Virtio queue shadowing */
>       VirtQueue *vq;
> @@ -40,6 +42,26 @@ static void vhost_handle_guest_kick(EventNotifier *n)
>       }
>   }
>   
> +/* Forward vhost notifications */
> +static void vhost_handle_call(EventNotifier *n)
> +{
> +    VhostShadowVirtqueue *svq = container_of(n, VhostShadowVirtqueue,
> +                                             call_notifier);
> +
> +    if (event_notifier_test_and_clear(n)) {
> +        event_notifier_set(svq->guest_notifier);
> +    }
> +}


So I wonder how this is synchonized with virtqueue mask/unmask. Or the 
masking is totally transparent to shadow virtqueue?

Thanks


> +
> +/*
> + * Get the vhost call notifier of the shadow vq
> + * @vq Shadow virtqueue
> + */
> +EventNotifier *vhost_shadow_vq_get_call_notifier(VhostShadowVirtqueue *vq)
> +{
> +    return &vq->call_notifier;
> +}
> +
>   /*
>    * Start shadow virtqueue operation.
>    * @dev vhost device
> @@ -57,6 +79,10 @@ bool vhost_shadow_vq_start_rcu(struct vhost_dev *dev,
>           .index = idx,
>           .fd = event_notifier_get_fd(&svq->kick_notifier),
>       };
> +    struct vhost_vring_file call_file = {
> +        .index = idx,
> +        .fd = event_notifier_get_fd(&svq->call_notifier),
> +    };
>       int r;
>   
>       /* Check that notifications are still going directly to vhost dev */
> @@ -66,6 +92,7 @@ bool vhost_shadow_vq_start_rcu(struct vhost_dev *dev,
>                              event_notifier_get_fd(vq_host_notifier));
>       event_notifier_set_handler(&svq->host_notifier, vhost_handle_guest_kick);
>   
> +    svq->guest_notifier = virtio_queue_get_guest_notifier(svq->vq);
>       r = dev->vhost_ops->vhost_set_vring_kick(dev, &kick_file);
>       if (unlikely(r != 0)) {
>           error_report("Couldn't set kick fd: %s", strerror(errno));
> @@ -75,8 +102,19 @@ bool vhost_shadow_vq_start_rcu(struct vhost_dev *dev,
>       /* Check for pending notifications from the guest */
>       vhost_handle_guest_kick(&svq->host_notifier);
>   
> +    r = dev->vhost_ops->vhost_set_vring_call(dev, &call_file);
> +    if (r != 0) {
> +        error_report("Couldn't set call fd: %s", strerror(errno));
> +        goto err_set_vring_call;
> +    }
> +
>       return true;
>   
> +err_set_vring_call:
> +    kick_file.fd = event_notifier_get_fd(vq_host_notifier);
> +    r = dev->vhost_ops->vhost_set_vring_kick(dev, &kick_file);
> +    assert(r == 0);
> +
>   err_set_vring_kick:
>       event_notifier_set_handler(&svq->host_notifier, NULL);
>   
> @@ -108,6 +146,16 @@ void vhost_shadow_vq_stop_rcu(struct vhost_dev *dev,
>       assert(r == 0);
>   
>       event_notifier_set_handler(&svq->host_notifier, NULL);
> +
> +    if (!dev->vqs[idx].notifier_is_masked) {
> +        EventNotifier *e = vhost_shadow_vq_get_call_notifier(svq);
> +
> +        /* Restore vhost call */
> +        vhost_virtqueue_mask(dev, dev->vdev, dev->vq_index + idx, false);
> +
> +        /* Check for pending calls */
> +        vhost_handle_call(e);
> +    }
>   }
>   
>   /*
> @@ -136,6 +184,7 @@ VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx)
>           goto err_init_call_notifier;
>       }
>   
> +    event_notifier_set_handler(&svq->call_notifier, vhost_handle_call);
>       return g_steal_pointer(&svq);
>   
>   err_init_call_notifier:
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 9d4728e545..0dc95679e9 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -975,7 +975,6 @@ static int vhost_sw_live_migration_start(struct vhost_dev *dev)
>           for (idx = 0; idx < dev->nvqs; ++idx) {
>               bool ok = vhost_shadow_vq_start_rcu(dev, idx,
>                                                   dev->shadow_vqs[idx]);
> -
>               if (!ok) {
>                   int stop_idx = idx;
>   
> @@ -1608,6 +1607,10 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
>       if (mask) {
>           assert(vdev->use_guest_notifier_mask);
>           file.fd = event_notifier_get_fd(&hdev->vqs[index].masked_notifier);
> +    } else if (hdev->sw_lm_enabled) {
> +        VhostShadowVirtqueue *svq = hdev->shadow_vqs[n];
> +        EventNotifier *e = vhost_shadow_vq_get_call_notifier(svq);
> +        file.fd = event_notifier_get_fd(e);
>       } else {
>           file.fd = event_notifier_get_fd(virtio_queue_get_guest_notifier(vvq));
>       }

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-03-01  6:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20210209153757.1653598-1-eperezma@redhat.com>
     [not found] ` <20210209153757.1653598-2-eperezma@redhat.com>
     [not found]   ` <14b2637e-9610-356e-ad18-27a9a8b82508@redhat.com>
     [not found]     ` <CAJaqyWd+NuY3B94HX8J_EzFf4sxeZnFCcw=aXfT-KZ7nfmL6DQ@mail.gmail.com>
2021-02-11 16:40       ` [RFC v2 1/7] vhost: Delete trailing dot in errpr_setg argument Stefano Garzarella
     [not found] ` <20210209153757.1653598-3-eperezma@redhat.com>
2021-02-17 12:41   ` [RFC v2 2/7] virtio: Add virtio_queue_host_notifier_status Stefan Hajnoczi
     [not found] ` <20210209153757.1653598-4-eperezma@redhat.com>
2021-02-17 12:44   ` [RFC v2 3/7] vhost: Save masked_notifier state Stefan Hajnoczi
     [not found] ` <20210209153757.1653598-5-eperezma@redhat.com>
2021-02-17 13:01   ` [RFC v2 4/7] vhost: Add VhostShadowVirtqueue Stefan Hajnoczi
     [not found] ` <20210209153757.1653598-6-eperezma@redhat.com>
2021-02-17 15:26   ` [RFC v2 5/7] vhost: Add x-vhost-enable-shadow-vq qmp Stefan Hajnoczi
     [not found] ` <20210209153757.1653598-7-eperezma@redhat.com>
2021-02-17 16:56   ` [RFC v2 6/7] vhost: Route guest->host notification through shadow virtqueue Stefan Hajnoczi
     [not found] ` <20210209153757.1653598-8-eperezma@redhat.com>
2021-02-17 17:24   ` [RFC v2 7/7] vhost: Route host->guest " Stefan Hajnoczi
2021-03-01  6:24   ` Jason Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox