At this mode no buffer forwarding will be performed in SVQ mode: Qemu
will just forward the guest's kicks to the device.
Host memory notifiers regions are left out for simplicity, and they will
not be addressed in this series.
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
hw/virtio/vhost-shadow-virtqueue.h | 14 +++
include/hw/virtio/vhost-vdpa.h | 4 +
hw/virtio/vhost-shadow-virtqueue.c | 52 +++++++++++
hw/virtio/vhost-vdpa.c | 145 ++++++++++++++++++++++++++++-
4 files changed, 213 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
index f1519e3c7b..1cbc87d5d8 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -18,8 +18,22 @@ typedef struct VhostShadowVirtqueue {
EventNotifier hdev_kick;
/* Shadow call notifier, sent to vhost */
EventNotifier hdev_call;
+
+ /*
+ * Borrowed virtqueue's guest to host notifier. To borrow it in this event
+ * notifier allows to recover the VhostShadowVirtqueue from the event loop
+ * easily. If we use the VirtQueue's one, we don't have an easy way to
+ * retrieve VhostShadowVirtqueue.
+ *
+ * So shadow virtqueue must not clean it, or we would lose VirtQueue one.
+ */
+ EventNotifier svq_kick;
} VhostShadowVirtqueue;
+void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd);
+
+void vhost_svq_stop(VhostShadowVirtqueue *svq);
+
VhostShadowVirtqueue *vhost_svq_new(void);
void vhost_svq_free(gpointer vq);
diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index 3ce79a646d..009a9f3b6b 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -12,6 +12,8 @@
#ifndef HW_VIRTIO_VHOST_VDPA_H
#define HW_VIRTIO_VHOST_VDPA_H
+#include <gmodule.h>
+
#include "hw/virtio/virtio.h"
#include "standard-headers/linux/vhost_types.h"
@@ -27,6 +29,8 @@ typedef struct vhost_vdpa {
bool iotlb_batch_begin_sent;
MemoryListener listener;
struct vhost_vdpa_iova_range iova_range;
+ bool shadow_vqs_enabled;
+ GPtrArray *shadow_vqs;
struct vhost_dev *dev;
VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX];
} VhostVDPA;
diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
index 019cf1950f..a5d0659f86 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -11,6 +11,56 @@
#include "hw/virtio/vhost-shadow-virtqueue.h"
#include "qemu/error-report.h"
+#include "qemu/main-loop.h"
+#include "linux-headers/linux/vhost.h"
+
+/** Forward guest notifications */
+static void vhost_handle_guest_kick(EventNotifier *n)
+{
+ VhostShadowVirtqueue *svq = container_of(n, VhostShadowVirtqueue,
+ svq_kick);
+ event_notifier_test_and_clear(n);
+ event_notifier_set(&svq->hdev_kick);
+}
+
+/**
+ * Set a new file descriptor for the guest to kick the SVQ and notify for avail
+ *
+ * @svq The svq
+ * @svq_kick_fd The svq kick fd
+ *
+ * Note that the SVQ will never close the old file descriptor.
+ */
+void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd)
+{
+ EventNotifier *svq_kick = &svq->svq_kick;
+ bool poll_stop = VHOST_FILE_UNBIND != event_notifier_get_fd(svq_kick);
+ bool poll_start = svq_kick_fd != VHOST_FILE_UNBIND;
+
+ if (poll_stop) {
+ event_notifier_set_handler(svq_kick, NULL);
+ }
+
+ /*
+ * event_notifier_set_handler already checks for guest's notifications if
+ * they arrive at the new file descriptor in the switch, so there is no
+ * need to explicitly check for them.
+ */
+ if (poll_start) {
+ event_notifier_init_fd(svq_kick, svq_kick_fd);
+ event_notifier_set(svq_kick);
+ event_notifier_set_handler(svq_kick, vhost_handle_guest_kick);
+ }
+}
+
+/**
+ * Stop the shadow virtqueue operation.
+ * @svq Shadow Virtqueue
+ */
+void vhost_svq_stop(VhostShadowVirtqueue *svq)
+{
+ event_notifier_set_handler(&svq->svq_kick, NULL);
+}
/**
* Creates vhost shadow virtqueue, and instructs the vhost device to use the
@@ -39,6 +89,7 @@ VhostShadowVirtqueue *vhost_svq_new(void)
goto err_init_hdev_call;
}
+ event_notifier_init_fd(&svq->svq_kick, VHOST_FILE_UNBIND);
return g_steal_pointer(&svq);
err_init_hdev_call:
@@ -56,6 +107,7 @@ err_init_hdev_kick:
void vhost_svq_free(gpointer pvq)
{
VhostShadowVirtqueue *vq = pvq;
+ vhost_svq_stop(vq);
event_notifier_cleanup(&vq->hdev_kick);
event_notifier_cleanup(&vq->hdev_call);
g_free(vq);
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 04ea43704f..454bf50735 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -17,12 +17,14 @@
#include "hw/virtio/vhost.h"
#include "hw/virtio/vhost-backend.h"
#include "hw/virtio/virtio-net.h"
+#include "hw/virtio/vhost-shadow-virtqueue.h"
#include "hw/virtio/vhost-vdpa.h"
#include "exec/address-spaces.h"
#include "qemu/main-loop.h"
#include "cpu.h"
#include "trace.h"
#include "qemu-common.h"
+#include "qapi/error.h"
/*
* Return one past the end of the end of section. Be careful with uint64_t
@@ -342,6 +344,30 @@ static bool vhost_vdpa_one_time_request(struct vhost_dev *dev)
return v->index != 0;
}
+static int vhost_vdpa_init_svq(struct vhost_dev *hdev, struct vhost_vdpa *v,
+ Error **errp)
+{
+ g_autoptr(GPtrArray) shadow_vqs = NULL;
+
+ if (!v->shadow_vqs_enabled) {
+ return 0;
+ }
+
+ shadow_vqs = g_ptr_array_new_full(hdev->nvqs, vhost_svq_free);
+ for (unsigned n = 0; n < hdev->nvqs; ++n) {
+ g_autoptr(VhostShadowVirtqueue) svq = vhost_svq_new();
+
+ if (unlikely(!svq)) {
+ error_setg(errp, "Cannot create svq %u", n);
+ return -1;
+ }
+ g_ptr_array_add(shadow_vqs, g_steal_pointer(&svq));
+ }
+
+ v->shadow_vqs = g_steal_pointer(&shadow_vqs);
+ return 0;
+}
+
static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp)
{
struct vhost_vdpa *v;
@@ -364,6 +390,10 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp)
dev->opaque = opaque ;
v->listener = vhost_vdpa_memory_listener;
v->msg_type = VHOST_IOTLB_MSG_V2;
+ ret = vhost_vdpa_init_svq(dev, v, errp);
+ if (ret) {
+ goto err;
+ }
vhost_vdpa_get_iova_range(v);
@@ -375,6 +405,10 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp)
VIRTIO_CONFIG_S_DRIVER);
return 0;
+
+err:
+ ram_block_discard_disable(false);
+ return ret;
}
static void vhost_vdpa_host_notifier_uninit(struct vhost_dev *dev,
@@ -444,8 +478,14 @@ err:
static void vhost_vdpa_host_notifiers_init(struct vhost_dev *dev)
{
+ struct vhost_vdpa *v = dev->opaque;
int i;
+ if (v->shadow_vqs_enabled) {
+ /* FIXME SVQ is not compatible with host notifiers mr */
+ return;
+ }
+
for (i = dev->vq_index; i < dev->vq_index + dev->nvqs; i++) {
if (vhost_vdpa_host_notifier_init(dev, i)) {
goto err;
@@ -459,6 +499,21 @@ err:
return;
}
+static void vhost_vdpa_svq_cleanup(struct vhost_dev *dev)
+{
+ struct vhost_vdpa *v = dev->opaque;
+ size_t idx;
+
+ if (!v->shadow_vqs) {
+ return;
+ }
+
+ for (idx = 0; idx < v->shadow_vqs->len; ++idx) {
+ vhost_svq_stop(g_ptr_array_index(v->shadow_vqs, idx));
+ }
+ g_ptr_array_free(v->shadow_vqs, true);
+}
+
static int vhost_vdpa_cleanup(struct vhost_dev *dev)
{
struct vhost_vdpa *v;
@@ -467,6 +522,7 @@ static int vhost_vdpa_cleanup(struct vhost_dev *dev)
trace_vhost_vdpa_cleanup(dev, v);
vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
memory_listener_unregister(&v->listener);
+ vhost_vdpa_svq_cleanup(dev);
dev->opaque = NULL;
ram_block_discard_disable(false);
@@ -558,11 +614,26 @@ static int vhost_vdpa_get_device_id(struct vhost_dev *dev,
return ret;
}
+static void vhost_vdpa_reset_svq(struct vhost_vdpa *v)
+{
+ if (!v->shadow_vqs_enabled) {
+ return;
+ }
+
+ for (unsigned i = 0; i < v->shadow_vqs->len; ++i) {
+ VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i);
+ vhost_svq_stop(svq);
+ }
+}
+
static int vhost_vdpa_reset_device(struct vhost_dev *dev)
{
+ struct vhost_vdpa *v = dev->opaque;
int ret;
uint8_t status = 0;
+ vhost_vdpa_reset_svq(v);
+
ret = vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &status);
trace_vhost_vdpa_reset_device(dev, status);
return ret;
@@ -646,13 +717,75 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
return ret;
}
+static int vhost_vdpa_set_vring_dev_kick(struct vhost_dev *dev,
+ struct vhost_vring_file *file)
+{
+ trace_vhost_vdpa_set_vring_kick(dev, file->index, file->fd);
+ return vhost_vdpa_call(dev, VHOST_SET_VRING_KICK, file);
+}
+
+/**
+ * Set the shadow virtqueue descriptors to the device
+ *
+ * @dev The vhost device model
+ * @svq The shadow virtqueue
+ * @idx The index of the virtqueue in the vhost device
+ * @errp Error
+ */
+static bool vhost_vdpa_svq_setup(struct vhost_dev *dev,
+ VhostShadowVirtqueue *svq,
+ unsigned idx,
+ Error **errp)
+{
+ struct vhost_vring_file file = {
+ .index = dev->vq_index + idx,
+ };
+ const EventNotifier *event_notifier = &svq->hdev_kick;
+ int r;
+
+ file.fd = event_notifier_get_fd(event_notifier);
+ r = vhost_vdpa_set_vring_dev_kick(dev, &file);
+ if (unlikely(r != 0)) {
+ error_setg_errno(errp, -r, "Can't set device kick fd");
+ }
+
+ return r == 0;
+}
+
+static bool vhost_vdpa_svqs_start(struct vhost_dev *dev)
+{
+ struct vhost_vdpa *v = dev->opaque;
+ Error *err = NULL;
+ unsigned i;
+
+ if (!v->shadow_vqs) {
+ return true;
+ }
+
+ for (i = 0; i < v->shadow_vqs->len; ++i) {
+ VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i);
+ bool ok = vhost_vdpa_svq_setup(dev, svq, i, &err);
+ if (unlikely(!ok)) {
+ error_reportf_err(err, "Cannot setup SVQ %u: ", i);
+ return false;
+ }
+ }
+
+ return true;
+}
+
static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
{
struct vhost_vdpa *v = dev->opaque;
+ bool ok;
trace_vhost_vdpa_dev_start(dev, started);
if (started) {
vhost_vdpa_host_notifiers_init(dev);
+ ok = vhost_vdpa_svqs_start(dev);
+ if (unlikely(!ok)) {
+ return -1;
+ }
vhost_vdpa_set_vring_ready(dev);
} else {
vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
@@ -724,8 +857,16 @@ static int vhost_vdpa_get_vring_base(struct vhost_dev *dev,
static int vhost_vdpa_set_vring_kick(struct vhost_dev *dev,
struct vhost_vring_file *file)
{
- trace_vhost_vdpa_set_vring_kick(dev, file->index, file->fd);
- return vhost_vdpa_call(dev, VHOST_SET_VRING_KICK, file);
+ struct vhost_vdpa *v = dev->opaque;
+ int vdpa_idx = file->index - dev->vq_index;
+
+ if (v->shadow_vqs_enabled) {
+ VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, vdpa_idx);
+ vhost_svq_set_svq_kick_fd(svq, file->fd);
+ return 0;
+ } else {
+ return vhost_vdpa_set_vring_dev_kick(dev, file);
+ }
}
static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
--
2.27.0
在 2022/2/27 下午9:40, Eugenio Pérez 写道:
> At this mode no buffer forwarding will be performed in SVQ mode: Qemu
> will just forward the guest's kicks to the device.
>
> Host memory notifiers regions are left out for simplicity, and they will
> not be addressed in this series.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
> hw/virtio/vhost-shadow-virtqueue.h | 14 +++
> include/hw/virtio/vhost-vdpa.h | 4 +
> hw/virtio/vhost-shadow-virtqueue.c | 52 +++++++++++
> hw/virtio/vhost-vdpa.c | 145 ++++++++++++++++++++++++++++-
> 4 files changed, 213 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
> index f1519e3c7b..1cbc87d5d8 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.h
> +++ b/hw/virtio/vhost-shadow-virtqueue.h
> @@ -18,8 +18,22 @@ typedef struct VhostShadowVirtqueue {
> EventNotifier hdev_kick;
> /* Shadow call notifier, sent to vhost */
> EventNotifier hdev_call;
> +
> + /*
> + * Borrowed virtqueue's guest to host notifier. To borrow it in this event
> + * notifier allows to recover the VhostShadowVirtqueue from the event loop
> + * easily. If we use the VirtQueue's one, we don't have an easy way to
> + * retrieve VhostShadowVirtqueue.
> + *
> + * So shadow virtqueue must not clean it, or we would lose VirtQueue one.
> + */
> + EventNotifier svq_kick;
> } VhostShadowVirtqueue;
>
> +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd);
> +
> +void vhost_svq_stop(VhostShadowVirtqueue *svq);
> +
> VhostShadowVirtqueue *vhost_svq_new(void);
>
> void vhost_svq_free(gpointer vq);
> diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
> index 3ce79a646d..009a9f3b6b 100644
> --- a/include/hw/virtio/vhost-vdpa.h
> +++ b/include/hw/virtio/vhost-vdpa.h
> @@ -12,6 +12,8 @@
> #ifndef HW_VIRTIO_VHOST_VDPA_H
> #define HW_VIRTIO_VHOST_VDPA_H
>
> +#include <gmodule.h>
> +
> #include "hw/virtio/virtio.h"
> #include "standard-headers/linux/vhost_types.h"
>
> @@ -27,6 +29,8 @@ typedef struct vhost_vdpa {
> bool iotlb_batch_begin_sent;
> MemoryListener listener;
> struct vhost_vdpa_iova_range iova_range;
> + bool shadow_vqs_enabled;
> + GPtrArray *shadow_vqs;
> struct vhost_dev *dev;
> VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX];
> } VhostVDPA;
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> index 019cf1950f..a5d0659f86 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -11,6 +11,56 @@
> #include "hw/virtio/vhost-shadow-virtqueue.h"
>
> #include "qemu/error-report.h"
> +#include "qemu/main-loop.h"
> +#include "linux-headers/linux/vhost.h"
> +
> +/** Forward guest notifications */
> +static void vhost_handle_guest_kick(EventNotifier *n)
> +{
> + VhostShadowVirtqueue *svq = container_of(n, VhostShadowVirtqueue,
> + svq_kick);
> + event_notifier_test_and_clear(n);
> + event_notifier_set(&svq->hdev_kick);
> +}
> +
> +/**
> + * Set a new file descriptor for the guest to kick the SVQ and notify for avail
> + *
> + * @svq The svq
> + * @svq_kick_fd The svq kick fd
> + *
> + * Note that the SVQ will never close the old file descriptor.
> + */
> +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd)
> +{
> + EventNotifier *svq_kick = &svq->svq_kick;
> + bool poll_stop = VHOST_FILE_UNBIND != event_notifier_get_fd(svq_kick);
I wonder if this is robust. E.g is there any chance that may end up with
both poll_stop and poll_start are false?
If not, can we simple detect poll_stop as below and treat !poll_start
and poll_stop?
Other looks good.
Thanks
> + bool poll_start = svq_kick_fd != VHOST_FILE_UNBIND;
> +
> + if (poll_stop) {
> + event_notifier_set_handler(svq_kick, NULL);
> + }
> +
> + /*
> + * event_notifier_set_handler already checks for guest's notifications if
> + * they arrive at the new file descriptor in the switch, so there is no
> + * need to explicitly check for them.
> + */
> + if (poll_start) {
> + event_notifier_init_fd(svq_kick, svq_kick_fd);
> + event_notifier_set(svq_kick);
> + event_notifier_set_handler(svq_kick, vhost_handle_guest_kick);
> + }
> +}
> +
> +/**
> + * Stop the shadow virtqueue operation.
> + * @svq Shadow Virtqueue
> + */
> +void vhost_svq_stop(VhostShadowVirtqueue *svq)
> +{
> + event_notifier_set_handler(&svq->svq_kick, NULL);
> +}
>
> /**
> * Creates vhost shadow virtqueue, and instructs the vhost device to use the
> @@ -39,6 +89,7 @@ VhostShadowVirtqueue *vhost_svq_new(void)
> goto err_init_hdev_call;
> }
>
> + event_notifier_init_fd(&svq->svq_kick, VHOST_FILE_UNBIND);
> return g_steal_pointer(&svq);
>
> err_init_hdev_call:
> @@ -56,6 +107,7 @@ err_init_hdev_kick:
> void vhost_svq_free(gpointer pvq)
> {
> VhostShadowVirtqueue *vq = pvq;
> + vhost_svq_stop(vq);
> event_notifier_cleanup(&vq->hdev_kick);
> event_notifier_cleanup(&vq->hdev_call);
> g_free(vq);
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 04ea43704f..454bf50735 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -17,12 +17,14 @@
> #include "hw/virtio/vhost.h"
> #include "hw/virtio/vhost-backend.h"
> #include "hw/virtio/virtio-net.h"
> +#include "hw/virtio/vhost-shadow-virtqueue.h"
> #include "hw/virtio/vhost-vdpa.h"
> #include "exec/address-spaces.h"
> #include "qemu/main-loop.h"
> #include "cpu.h"
> #include "trace.h"
> #include "qemu-common.h"
> +#include "qapi/error.h"
>
> /*
> * Return one past the end of the end of section. Be careful with uint64_t
> @@ -342,6 +344,30 @@ static bool vhost_vdpa_one_time_request(struct vhost_dev *dev)
> return v->index != 0;
> }
>
> +static int vhost_vdpa_init_svq(struct vhost_dev *hdev, struct vhost_vdpa *v,
> + Error **errp)
> +{
> + g_autoptr(GPtrArray) shadow_vqs = NULL;
> +
> + if (!v->shadow_vqs_enabled) {
> + return 0;
> + }
> +
> + shadow_vqs = g_ptr_array_new_full(hdev->nvqs, vhost_svq_free);
> + for (unsigned n = 0; n < hdev->nvqs; ++n) {
> + g_autoptr(VhostShadowVirtqueue) svq = vhost_svq_new();
> +
> + if (unlikely(!svq)) {
> + error_setg(errp, "Cannot create svq %u", n);
> + return -1;
> + }
> + g_ptr_array_add(shadow_vqs, g_steal_pointer(&svq));
> + }
> +
> + v->shadow_vqs = g_steal_pointer(&shadow_vqs);
> + return 0;
> +}
> +
> static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp)
> {
> struct vhost_vdpa *v;
> @@ -364,6 +390,10 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp)
> dev->opaque = opaque ;
> v->listener = vhost_vdpa_memory_listener;
> v->msg_type = VHOST_IOTLB_MSG_V2;
> + ret = vhost_vdpa_init_svq(dev, v, errp);
> + if (ret) {
> + goto err;
> + }
>
> vhost_vdpa_get_iova_range(v);
>
> @@ -375,6 +405,10 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp)
> VIRTIO_CONFIG_S_DRIVER);
>
> return 0;
> +
> +err:
> + ram_block_discard_disable(false);
> + return ret;
> }
>
> static void vhost_vdpa_host_notifier_uninit(struct vhost_dev *dev,
> @@ -444,8 +478,14 @@ err:
>
> static void vhost_vdpa_host_notifiers_init(struct vhost_dev *dev)
> {
> + struct vhost_vdpa *v = dev->opaque;
> int i;
>
> + if (v->shadow_vqs_enabled) {
> + /* FIXME SVQ is not compatible with host notifiers mr */
> + return;
> + }
> +
> for (i = dev->vq_index; i < dev->vq_index + dev->nvqs; i++) {
> if (vhost_vdpa_host_notifier_init(dev, i)) {
> goto err;
> @@ -459,6 +499,21 @@ err:
> return;
> }
>
> +static void vhost_vdpa_svq_cleanup(struct vhost_dev *dev)
> +{
> + struct vhost_vdpa *v = dev->opaque;
> + size_t idx;
> +
> + if (!v->shadow_vqs) {
> + return;
> + }
> +
> + for (idx = 0; idx < v->shadow_vqs->len; ++idx) {
> + vhost_svq_stop(g_ptr_array_index(v->shadow_vqs, idx));
> + }
> + g_ptr_array_free(v->shadow_vqs, true);
> +}
> +
> static int vhost_vdpa_cleanup(struct vhost_dev *dev)
> {
> struct vhost_vdpa *v;
> @@ -467,6 +522,7 @@ static int vhost_vdpa_cleanup(struct vhost_dev *dev)
> trace_vhost_vdpa_cleanup(dev, v);
> vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
> memory_listener_unregister(&v->listener);
> + vhost_vdpa_svq_cleanup(dev);
>
> dev->opaque = NULL;
> ram_block_discard_disable(false);
> @@ -558,11 +614,26 @@ static int vhost_vdpa_get_device_id(struct vhost_dev *dev,
> return ret;
> }
>
> +static void vhost_vdpa_reset_svq(struct vhost_vdpa *v)
> +{
> + if (!v->shadow_vqs_enabled) {
> + return;
> + }
> +
> + for (unsigned i = 0; i < v->shadow_vqs->len; ++i) {
> + VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i);
> + vhost_svq_stop(svq);
> + }
> +}
> +
> static int vhost_vdpa_reset_device(struct vhost_dev *dev)
> {
> + struct vhost_vdpa *v = dev->opaque;
> int ret;
> uint8_t status = 0;
>
> + vhost_vdpa_reset_svq(v);
> +
> ret = vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &status);
> trace_vhost_vdpa_reset_device(dev, status);
> return ret;
> @@ -646,13 +717,75 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
> return ret;
> }
>
> +static int vhost_vdpa_set_vring_dev_kick(struct vhost_dev *dev,
> + struct vhost_vring_file *file)
> +{
> + trace_vhost_vdpa_set_vring_kick(dev, file->index, file->fd);
> + return vhost_vdpa_call(dev, VHOST_SET_VRING_KICK, file);
> +}
> +
> +/**
> + * Set the shadow virtqueue descriptors to the device
> + *
> + * @dev The vhost device model
> + * @svq The shadow virtqueue
> + * @idx The index of the virtqueue in the vhost device
> + * @errp Error
> + */
> +static bool vhost_vdpa_svq_setup(struct vhost_dev *dev,
> + VhostShadowVirtqueue *svq,
> + unsigned idx,
> + Error **errp)
> +{
> + struct vhost_vring_file file = {
> + .index = dev->vq_index + idx,
> + };
> + const EventNotifier *event_notifier = &svq->hdev_kick;
> + int r;
> +
> + file.fd = event_notifier_get_fd(event_notifier);
> + r = vhost_vdpa_set_vring_dev_kick(dev, &file);
> + if (unlikely(r != 0)) {
> + error_setg_errno(errp, -r, "Can't set device kick fd");
> + }
> +
> + return r == 0;
> +}
> +
> +static bool vhost_vdpa_svqs_start(struct vhost_dev *dev)
> +{
> + struct vhost_vdpa *v = dev->opaque;
> + Error *err = NULL;
> + unsigned i;
> +
> + if (!v->shadow_vqs) {
> + return true;
> + }
> +
> + for (i = 0; i < v->shadow_vqs->len; ++i) {
> + VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i);
> + bool ok = vhost_vdpa_svq_setup(dev, svq, i, &err);
> + if (unlikely(!ok)) {
> + error_reportf_err(err, "Cannot setup SVQ %u: ", i);
> + return false;
> + }
> + }
> +
> + return true;
> +}
> +
> static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
> {
> struct vhost_vdpa *v = dev->opaque;
> + bool ok;
> trace_vhost_vdpa_dev_start(dev, started);
>
> if (started) {
> vhost_vdpa_host_notifiers_init(dev);
> + ok = vhost_vdpa_svqs_start(dev);
> + if (unlikely(!ok)) {
> + return -1;
> + }
> vhost_vdpa_set_vring_ready(dev);
> } else {
> vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
> @@ -724,8 +857,16 @@ static int vhost_vdpa_get_vring_base(struct vhost_dev *dev,
> static int vhost_vdpa_set_vring_kick(struct vhost_dev *dev,
> struct vhost_vring_file *file)
> {
> - trace_vhost_vdpa_set_vring_kick(dev, file->index, file->fd);
> - return vhost_vdpa_call(dev, VHOST_SET_VRING_KICK, file);
> + struct vhost_vdpa *v = dev->opaque;
> + int vdpa_idx = file->index - dev->vq_index;
> +
> + if (v->shadow_vqs_enabled) {
> + VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, vdpa_idx);
> + vhost_svq_set_svq_kick_fd(svq, file->fd);
> + return 0;
> + } else {
> + return vhost_vdpa_set_vring_dev_kick(dev, file);
> + }
> }
>
> static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
On Mon, Feb 28, 2022 at 3:57 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2022/2/27 下午9:40, Eugenio Pérez 写道:
> > At this mode no buffer forwarding will be performed in SVQ mode: Qemu
> > will just forward the guest's kicks to the device.
> >
> > Host memory notifiers regions are left out for simplicity, and they will
> > not be addressed in this series.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> > hw/virtio/vhost-shadow-virtqueue.h | 14 +++
> > include/hw/virtio/vhost-vdpa.h | 4 +
> > hw/virtio/vhost-shadow-virtqueue.c | 52 +++++++++++
> > hw/virtio/vhost-vdpa.c | 145 ++++++++++++++++++++++++++++-
> > 4 files changed, 213 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
> > index f1519e3c7b..1cbc87d5d8 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > @@ -18,8 +18,22 @@ typedef struct VhostShadowVirtqueue {
> > EventNotifier hdev_kick;
> > /* Shadow call notifier, sent to vhost */
> > EventNotifier hdev_call;
> > +
> > + /*
> > + * Borrowed virtqueue's guest to host notifier. To borrow it in this event
> > + * notifier allows to recover the VhostShadowVirtqueue from the event loop
> > + * easily. If we use the VirtQueue's one, we don't have an easy way to
> > + * retrieve VhostShadowVirtqueue.
> > + *
> > + * So shadow virtqueue must not clean it, or we would lose VirtQueue one.
> > + */
> > + EventNotifier svq_kick;
> > } VhostShadowVirtqueue;
> >
> > +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd);
> > +
> > +void vhost_svq_stop(VhostShadowVirtqueue *svq);
> > +
> > VhostShadowVirtqueue *vhost_svq_new(void);
> >
> > void vhost_svq_free(gpointer vq);
> > diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
> > index 3ce79a646d..009a9f3b6b 100644
> > --- a/include/hw/virtio/vhost-vdpa.h
> > +++ b/include/hw/virtio/vhost-vdpa.h
> > @@ -12,6 +12,8 @@
> > #ifndef HW_VIRTIO_VHOST_VDPA_H
> > #define HW_VIRTIO_VHOST_VDPA_H
> >
> > +#include <gmodule.h>
> > +
> > #include "hw/virtio/virtio.h"
> > #include "standard-headers/linux/vhost_types.h"
> >
> > @@ -27,6 +29,8 @@ typedef struct vhost_vdpa {
> > bool iotlb_batch_begin_sent;
> > MemoryListener listener;
> > struct vhost_vdpa_iova_range iova_range;
> > + bool shadow_vqs_enabled;
> > + GPtrArray *shadow_vqs;
> > struct vhost_dev *dev;
> > VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX];
> > } VhostVDPA;
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> > index 019cf1950f..a5d0659f86 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > @@ -11,6 +11,56 @@
> > #include "hw/virtio/vhost-shadow-virtqueue.h"
> >
> > #include "qemu/error-report.h"
> > +#include "qemu/main-loop.h"
> > +#include "linux-headers/linux/vhost.h"
> > +
> > +/** Forward guest notifications */
> > +static void vhost_handle_guest_kick(EventNotifier *n)
> > +{
> > + VhostShadowVirtqueue *svq = container_of(n, VhostShadowVirtqueue,
> > + svq_kick);
> > + event_notifier_test_and_clear(n);
> > + event_notifier_set(&svq->hdev_kick);
> > +}
> > +
> > +/**
> > + * Set a new file descriptor for the guest to kick the SVQ and notify for avail
> > + *
> > + * @svq The svq
> > + * @svq_kick_fd The svq kick fd
> > + *
> > + * Note that the SVQ will never close the old file descriptor.
> > + */
> > +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd)
> > +{
> > + EventNotifier *svq_kick = &svq->svq_kick;
> > + bool poll_stop = VHOST_FILE_UNBIND != event_notifier_get_fd(svq_kick);
>
>
> I wonder if this is robust. E.g is there any chance that may end up with
> both poll_stop and poll_start are false?
>
I cannot make that happen in qemu, but the function supports that case
well: It will do nothing. It's more or less the same code as used in
the vhost kernel, and is the expected behaviour if you send two
VHOST_FILE_UNBIND one right after another to me.
> If not, can we simple detect poll_stop as below and treat !poll_start
> and poll_stop?
>
I'm not sure what does it add. Is there an unexpected consequence with
the current do-nothing behavior I've missed?
Thanks!
> Other looks good.
>
> Thanks
>
>
> > + bool poll_start = svq_kick_fd != VHOST_FILE_UNBIND;
> > +
> > + if (poll_stop) {
> > + event_notifier_set_handler(svq_kick, NULL);
> > + }
> > +
> > + /*
> > + * event_notifier_set_handler already checks for guest's notifications if
> > + * they arrive at the new file descriptor in the switch, so there is no
> > + * need to explicitly check for them.
> > + */
> > + if (poll_start) {
> > + event_notifier_init_fd(svq_kick, svq_kick_fd);
> > + event_notifier_set(svq_kick);
> > + event_notifier_set_handler(svq_kick, vhost_handle_guest_kick);
> > + }
> > +}
> > +
> > +/**
> > + * Stop the shadow virtqueue operation.
> > + * @svq Shadow Virtqueue
> > + */
> > +void vhost_svq_stop(VhostShadowVirtqueue *svq)
> > +{
> > + event_notifier_set_handler(&svq->svq_kick, NULL);
> > +}
> >
> > /**
> > * Creates vhost shadow virtqueue, and instructs the vhost device to use the
> > @@ -39,6 +89,7 @@ VhostShadowVirtqueue *vhost_svq_new(void)
> > goto err_init_hdev_call;
> > }
> >
> > + event_notifier_init_fd(&svq->svq_kick, VHOST_FILE_UNBIND);
> > return g_steal_pointer(&svq);
> >
> > err_init_hdev_call:
> > @@ -56,6 +107,7 @@ err_init_hdev_kick:
> > void vhost_svq_free(gpointer pvq)
> > {
> > VhostShadowVirtqueue *vq = pvq;
> > + vhost_svq_stop(vq);
> > event_notifier_cleanup(&vq->hdev_kick);
> > event_notifier_cleanup(&vq->hdev_call);
> > g_free(vq);
> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > index 04ea43704f..454bf50735 100644
> > --- a/hw/virtio/vhost-vdpa.c
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -17,12 +17,14 @@
> > #include "hw/virtio/vhost.h"
> > #include "hw/virtio/vhost-backend.h"
> > #include "hw/virtio/virtio-net.h"
> > +#include "hw/virtio/vhost-shadow-virtqueue.h"
> > #include "hw/virtio/vhost-vdpa.h"
> > #include "exec/address-spaces.h"
> > #include "qemu/main-loop.h"
> > #include "cpu.h"
> > #include "trace.h"
> > #include "qemu-common.h"
> > +#include "qapi/error.h"
> >
> > /*
> > * Return one past the end of the end of section. Be careful with uint64_t
> > @@ -342,6 +344,30 @@ static bool vhost_vdpa_one_time_request(struct vhost_dev *dev)
> > return v->index != 0;
> > }
> >
> > +static int vhost_vdpa_init_svq(struct vhost_dev *hdev, struct vhost_vdpa *v,
> > + Error **errp)
> > +{
> > + g_autoptr(GPtrArray) shadow_vqs = NULL;
> > +
> > + if (!v->shadow_vqs_enabled) {
> > + return 0;
> > + }
> > +
> > + shadow_vqs = g_ptr_array_new_full(hdev->nvqs, vhost_svq_free);
> > + for (unsigned n = 0; n < hdev->nvqs; ++n) {
> > + g_autoptr(VhostShadowVirtqueue) svq = vhost_svq_new();
> > +
> > + if (unlikely(!svq)) {
> > + error_setg(errp, "Cannot create svq %u", n);
> > + return -1;
> > + }
> > + g_ptr_array_add(shadow_vqs, g_steal_pointer(&svq));
> > + }
> > +
> > + v->shadow_vqs = g_steal_pointer(&shadow_vqs);
> > + return 0;
> > +}
> > +
> > static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp)
> > {
> > struct vhost_vdpa *v;
> > @@ -364,6 +390,10 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp)
> > dev->opaque = opaque ;
> > v->listener = vhost_vdpa_memory_listener;
> > v->msg_type = VHOST_IOTLB_MSG_V2;
> > + ret = vhost_vdpa_init_svq(dev, v, errp);
> > + if (ret) {
> > + goto err;
> > + }
> >
> > vhost_vdpa_get_iova_range(v);
> >
> > @@ -375,6 +405,10 @@ static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp)
> > VIRTIO_CONFIG_S_DRIVER);
> >
> > return 0;
> > +
> > +err:
> > + ram_block_discard_disable(false);
> > + return ret;
> > }
> >
> > static void vhost_vdpa_host_notifier_uninit(struct vhost_dev *dev,
> > @@ -444,8 +478,14 @@ err:
> >
> > static void vhost_vdpa_host_notifiers_init(struct vhost_dev *dev)
> > {
> > + struct vhost_vdpa *v = dev->opaque;
> > int i;
> >
> > + if (v->shadow_vqs_enabled) {
> > + /* FIXME SVQ is not compatible with host notifiers mr */
> > + return;
> > + }
> > +
> > for (i = dev->vq_index; i < dev->vq_index + dev->nvqs; i++) {
> > if (vhost_vdpa_host_notifier_init(dev, i)) {
> > goto err;
> > @@ -459,6 +499,21 @@ err:
> > return;
> > }
> >
> > +static void vhost_vdpa_svq_cleanup(struct vhost_dev *dev)
> > +{
> > + struct vhost_vdpa *v = dev->opaque;
> > + size_t idx;
> > +
> > + if (!v->shadow_vqs) {
> > + return;
> > + }
> > +
> > + for (idx = 0; idx < v->shadow_vqs->len; ++idx) {
> > + vhost_svq_stop(g_ptr_array_index(v->shadow_vqs, idx));
> > + }
> > + g_ptr_array_free(v->shadow_vqs, true);
> > +}
> > +
> > static int vhost_vdpa_cleanup(struct vhost_dev *dev)
> > {
> > struct vhost_vdpa *v;
> > @@ -467,6 +522,7 @@ static int vhost_vdpa_cleanup(struct vhost_dev *dev)
> > trace_vhost_vdpa_cleanup(dev, v);
> > vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
> > memory_listener_unregister(&v->listener);
> > + vhost_vdpa_svq_cleanup(dev);
> >
> > dev->opaque = NULL;
> > ram_block_discard_disable(false);
> > @@ -558,11 +614,26 @@ static int vhost_vdpa_get_device_id(struct vhost_dev *dev,
> > return ret;
> > }
> >
> > +static void vhost_vdpa_reset_svq(struct vhost_vdpa *v)
> > +{
> > + if (!v->shadow_vqs_enabled) {
> > + return;
> > + }
> > +
> > + for (unsigned i = 0; i < v->shadow_vqs->len; ++i) {
> > + VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i);
> > + vhost_svq_stop(svq);
> > + }
> > +}
> > +
> > static int vhost_vdpa_reset_device(struct vhost_dev *dev)
> > {
> > + struct vhost_vdpa *v = dev->opaque;
> > int ret;
> > uint8_t status = 0;
> >
> > + vhost_vdpa_reset_svq(v);
> > +
> > ret = vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &status);
> > trace_vhost_vdpa_reset_device(dev, status);
> > return ret;
> > @@ -646,13 +717,75 @@ static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config,
> > return ret;
> > }
> >
> > +static int vhost_vdpa_set_vring_dev_kick(struct vhost_dev *dev,
> > + struct vhost_vring_file *file)
> > +{
> > + trace_vhost_vdpa_set_vring_kick(dev, file->index, file->fd);
> > + return vhost_vdpa_call(dev, VHOST_SET_VRING_KICK, file);
> > +}
> > +
> > +/**
> > + * Set the shadow virtqueue descriptors to the device
> > + *
> > + * @dev The vhost device model
> > + * @svq The shadow virtqueue
> > + * @idx The index of the virtqueue in the vhost device
> > + * @errp Error
> > + */
> > +static bool vhost_vdpa_svq_setup(struct vhost_dev *dev,
> > + VhostShadowVirtqueue *svq,
> > + unsigned idx,
> > + Error **errp)
> > +{
> > + struct vhost_vring_file file = {
> > + .index = dev->vq_index + idx,
> > + };
> > + const EventNotifier *event_notifier = &svq->hdev_kick;
> > + int r;
> > +
> > + file.fd = event_notifier_get_fd(event_notifier);
> > + r = vhost_vdpa_set_vring_dev_kick(dev, &file);
> > + if (unlikely(r != 0)) {
> > + error_setg_errno(errp, -r, "Can't set device kick fd");
> > + }
> > +
> > + return r == 0;
> > +}
> > +
> > +static bool vhost_vdpa_svqs_start(struct vhost_dev *dev)
> > +{
> > + struct vhost_vdpa *v = dev->opaque;
> > + Error *err = NULL;
> > + unsigned i;
> > +
> > + if (!v->shadow_vqs) {
> > + return true;
> > + }
> > +
> > + for (i = 0; i < v->shadow_vqs->len; ++i) {
> > + VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i);
> > + bool ok = vhost_vdpa_svq_setup(dev, svq, i, &err);
> > + if (unlikely(!ok)) {
> > + error_reportf_err(err, "Cannot setup SVQ %u: ", i);
> > + return false;
> > + }
> > + }
> > +
> > + return true;
> > +}
> > +
> > static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
> > {
> > struct vhost_vdpa *v = dev->opaque;
> > + bool ok;
> > trace_vhost_vdpa_dev_start(dev, started);
> >
> > if (started) {
> > vhost_vdpa_host_notifiers_init(dev);
> > + ok = vhost_vdpa_svqs_start(dev);
> > + if (unlikely(!ok)) {
> > + return -1;
> > + }
> > vhost_vdpa_set_vring_ready(dev);
> > } else {
> > vhost_vdpa_host_notifiers_uninit(dev, dev->nvqs);
> > @@ -724,8 +857,16 @@ static int vhost_vdpa_get_vring_base(struct vhost_dev *dev,
> > static int vhost_vdpa_set_vring_kick(struct vhost_dev *dev,
> > struct vhost_vring_file *file)
> > {
> > - trace_vhost_vdpa_set_vring_kick(dev, file->index, file->fd);
> > - return vhost_vdpa_call(dev, VHOST_SET_VRING_KICK, file);
> > + struct vhost_vdpa *v = dev->opaque;
> > + int vdpa_idx = file->index - dev->vq_index;
> > +
> > + if (v->shadow_vqs_enabled) {
> > + VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, vdpa_idx);
> > + vhost_svq_set_svq_kick_fd(svq, file->fd);
> > + return 0;
> > + } else {
> > + return vhost_vdpa_set_vring_dev_kick(dev, file);
> > + }
> > }
> >
> > static int vhost_vdpa_set_vring_call(struct vhost_dev *dev,
>
在 2022/3/2 上午2:49, Eugenio Perez Martin 写道:
> On Mon, Feb 28, 2022 at 3:57 AM Jason Wang<jasowang@redhat.com> wrote:
>> 在 2022/2/27 下午9:40, Eugenio Pérez 写道:
>>> At this mode no buffer forwarding will be performed in SVQ mode: Qemu
>>> will just forward the guest's kicks to the device.
>>>
>>> Host memory notifiers regions are left out for simplicity, and they will
>>> not be addressed in this series.
>>>
>>> Signed-off-by: Eugenio Pérez<eperezma@redhat.com>
>>> ---
>>> hw/virtio/vhost-shadow-virtqueue.h | 14 +++
>>> include/hw/virtio/vhost-vdpa.h | 4 +
>>> hw/virtio/vhost-shadow-virtqueue.c | 52 +++++++++++
>>> hw/virtio/vhost-vdpa.c | 145 ++++++++++++++++++++++++++++-
>>> 4 files changed, 213 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
>>> index f1519e3c7b..1cbc87d5d8 100644
>>> --- a/hw/virtio/vhost-shadow-virtqueue.h
>>> +++ b/hw/virtio/vhost-shadow-virtqueue.h
>>> @@ -18,8 +18,22 @@ typedef struct VhostShadowVirtqueue {
>>> EventNotifier hdev_kick;
>>> /* Shadow call notifier, sent to vhost */
>>> EventNotifier hdev_call;
>>> +
>>> + /*
>>> + * Borrowed virtqueue's guest to host notifier. To borrow it in this event
>>> + * notifier allows to recover the VhostShadowVirtqueue from the event loop
>>> + * easily. If we use the VirtQueue's one, we don't have an easy way to
>>> + * retrieve VhostShadowVirtqueue.
>>> + *
>>> + * So shadow virtqueue must not clean it, or we would lose VirtQueue one.
>>> + */
>>> + EventNotifier svq_kick;
>>> } VhostShadowVirtqueue;
>>>
>>> +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd);
>>> +
>>> +void vhost_svq_stop(VhostShadowVirtqueue *svq);
>>> +
>>> VhostShadowVirtqueue *vhost_svq_new(void);
>>>
>>> void vhost_svq_free(gpointer vq);
>>> diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
>>> index 3ce79a646d..009a9f3b6b 100644
>>> --- a/include/hw/virtio/vhost-vdpa.h
>>> +++ b/include/hw/virtio/vhost-vdpa.h
>>> @@ -12,6 +12,8 @@
>>> #ifndef HW_VIRTIO_VHOST_VDPA_H
>>> #define HW_VIRTIO_VHOST_VDPA_H
>>>
>>> +#include <gmodule.h>
>>> +
>>> #include "hw/virtio/virtio.h"
>>> #include "standard-headers/linux/vhost_types.h"
>>>
>>> @@ -27,6 +29,8 @@ typedef struct vhost_vdpa {
>>> bool iotlb_batch_begin_sent;
>>> MemoryListener listener;
>>> struct vhost_vdpa_iova_range iova_range;
>>> + bool shadow_vqs_enabled;
>>> + GPtrArray *shadow_vqs;
>>> struct vhost_dev *dev;
>>> VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX];
>>> } VhostVDPA;
>>> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
>>> index 019cf1950f..a5d0659f86 100644
>>> --- a/hw/virtio/vhost-shadow-virtqueue.c
>>> +++ b/hw/virtio/vhost-shadow-virtqueue.c
>>> @@ -11,6 +11,56 @@
>>> #include "hw/virtio/vhost-shadow-virtqueue.h"
>>>
>>> #include "qemu/error-report.h"
>>> +#include "qemu/main-loop.h"
>>> +#include "linux-headers/linux/vhost.h"
>>> +
>>> +/** Forward guest notifications */
>>> +static void vhost_handle_guest_kick(EventNotifier *n)
>>> +{
>>> + VhostShadowVirtqueue *svq = container_of(n, VhostShadowVirtqueue,
>>> + svq_kick);
>>> + event_notifier_test_and_clear(n);
>>> + event_notifier_set(&svq->hdev_kick);
>>> +}
>>> +
>>> +/**
>>> + * Set a new file descriptor for the guest to kick the SVQ and notify for avail
>>> + *
>>> + * @svq The svq
>>> + * @svq_kick_fd The svq kick fd
>>> + *
>>> + * Note that the SVQ will never close the old file descriptor.
>>> + */
>>> +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd)
>>> +{
>>> + EventNotifier *svq_kick = &svq->svq_kick;
>>> + bool poll_stop = VHOST_FILE_UNBIND != event_notifier_get_fd(svq_kick);
>> I wonder if this is robust. E.g is there any chance that may end up with
>> both poll_stop and poll_start are false?
>>
> I cannot make that happen in qemu, but the function supports that case
> well: It will do nothing. It's more or less the same code as used in
> the vhost kernel, and is the expected behaviour if you send two
> VHOST_FILE_UNBIND one right after another to me.
I would think it's just stop twice.
>
>> If not, can we simple detect poll_stop as below and treat !poll_start
>> and poll_stop?
>>
> I'm not sure what does it add. Is there an unexpected consequence with
> the current do-nothing behavior I've missed?
I'm not sure, but it feels odd if poll_start is not the reverse value of
poll_stop.
Thanks
On Thu, Mar 3, 2022 at 8:12 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2022/3/2 上午2:49, Eugenio Perez Martin 写道:
> > On Mon, Feb 28, 2022 at 3:57 AM Jason Wang<jasowang@redhat.com> wrote:
> >> 在 2022/2/27 下午9:40, Eugenio Pérez 写道:
> >>> At this mode no buffer forwarding will be performed in SVQ mode: Qemu
> >>> will just forward the guest's kicks to the device.
> >>>
> >>> Host memory notifiers regions are left out for simplicity, and they will
> >>> not be addressed in this series.
> >>>
> >>> Signed-off-by: Eugenio Pérez<eperezma@redhat.com>
> >>> ---
> >>> hw/virtio/vhost-shadow-virtqueue.h | 14 +++
> >>> include/hw/virtio/vhost-vdpa.h | 4 +
> >>> hw/virtio/vhost-shadow-virtqueue.c | 52 +++++++++++
> >>> hw/virtio/vhost-vdpa.c | 145 ++++++++++++++++++++++++++++-
> >>> 4 files changed, 213 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
> >>> index f1519e3c7b..1cbc87d5d8 100644
> >>> --- a/hw/virtio/vhost-shadow-virtqueue.h
> >>> +++ b/hw/virtio/vhost-shadow-virtqueue.h
> >>> @@ -18,8 +18,22 @@ typedef struct VhostShadowVirtqueue {
> >>> EventNotifier hdev_kick;
> >>> /* Shadow call notifier, sent to vhost */
> >>> EventNotifier hdev_call;
> >>> +
> >>> + /*
> >>> + * Borrowed virtqueue's guest to host notifier. To borrow it in this event
> >>> + * notifier allows to recover the VhostShadowVirtqueue from the event loop
> >>> + * easily. If we use the VirtQueue's one, we don't have an easy way to
> >>> + * retrieve VhostShadowVirtqueue.
> >>> + *
> >>> + * So shadow virtqueue must not clean it, or we would lose VirtQueue one.
> >>> + */
> >>> + EventNotifier svq_kick;
> >>> } VhostShadowVirtqueue;
> >>>
> >>> +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd);
> >>> +
> >>> +void vhost_svq_stop(VhostShadowVirtqueue *svq);
> >>> +
> >>> VhostShadowVirtqueue *vhost_svq_new(void);
> >>>
> >>> void vhost_svq_free(gpointer vq);
> >>> diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
> >>> index 3ce79a646d..009a9f3b6b 100644
> >>> --- a/include/hw/virtio/vhost-vdpa.h
> >>> +++ b/include/hw/virtio/vhost-vdpa.h
> >>> @@ -12,6 +12,8 @@
> >>> #ifndef HW_VIRTIO_VHOST_VDPA_H
> >>> #define HW_VIRTIO_VHOST_VDPA_H
> >>>
> >>> +#include <gmodule.h>
> >>> +
> >>> #include "hw/virtio/virtio.h"
> >>> #include "standard-headers/linux/vhost_types.h"
> >>>
> >>> @@ -27,6 +29,8 @@ typedef struct vhost_vdpa {
> >>> bool iotlb_batch_begin_sent;
> >>> MemoryListener listener;
> >>> struct vhost_vdpa_iova_range iova_range;
> >>> + bool shadow_vqs_enabled;
> >>> + GPtrArray *shadow_vqs;
> >>> struct vhost_dev *dev;
> >>> VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX];
> >>> } VhostVDPA;
> >>> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> >>> index 019cf1950f..a5d0659f86 100644
> >>> --- a/hw/virtio/vhost-shadow-virtqueue.c
> >>> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> >>> @@ -11,6 +11,56 @@
> >>> #include "hw/virtio/vhost-shadow-virtqueue.h"
> >>>
> >>> #include "qemu/error-report.h"
> >>> +#include "qemu/main-loop.h"
> >>> +#include "linux-headers/linux/vhost.h"
> >>> +
> >>> +/** Forward guest notifications */
> >>> +static void vhost_handle_guest_kick(EventNotifier *n)
> >>> +{
> >>> + VhostShadowVirtqueue *svq = container_of(n, VhostShadowVirtqueue,
> >>> + svq_kick);
> >>> + event_notifier_test_and_clear(n);
> >>> + event_notifier_set(&svq->hdev_kick);
> >>> +}
> >>> +
> >>> +/**
> >>> + * Set a new file descriptor for the guest to kick the SVQ and notify for avail
> >>> + *
> >>> + * @svq The svq
> >>> + * @svq_kick_fd The svq kick fd
> >>> + *
> >>> + * Note that the SVQ will never close the old file descriptor.
> >>> + */
> >>> +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd)
> >>> +{
> >>> + EventNotifier *svq_kick = &svq->svq_kick;
> >>> + bool poll_stop = VHOST_FILE_UNBIND != event_notifier_get_fd(svq_kick);
> >> I wonder if this is robust. E.g is there any chance that may end up with
> >> both poll_stop and poll_start are false?
> >>
> > I cannot make that happen in qemu, but the function supports that case
> > well: It will do nothing. It's more or less the same code as used in
> > the vhost kernel, and is the expected behaviour if you send two
> > VHOST_FILE_UNBIND one right after another to me.
>
>
> I would think it's just stop twice.
>
>
> >
> >> If not, can we simple detect poll_stop as below and treat !poll_start
> >> and poll_stop?
> >>
> > I'm not sure what does it add. Is there an unexpected consequence with
> > the current do-nothing behavior I've missed?
>
>
> I'm not sure, but it feels odd if poll_start is not the reverse value of
> poll_stop.
>
If we want to not to restrict the inputs, we need to handle for situations:
a) old_fd = -1, new_fd = -1,
This is the situation you described, and it's basically a no-op.
poll_stop == poll_start == false.
If we make poll_stop = true and poll_stop = false, we call
event_notifier_set_handler(-1, ...). Hopefully it will return just an
error.
If we make poll_stop = false and poll_stop = true, we are calling
event_notifier_set(-1) and event_notifier_set_handler(-1,
poll_callback). Same situation, hopefully an error, but unexpected.
b) old_fd = -1, new_fd = >-1,
We need to start polling the new_fd. No need for stop polling the
old_fd, since we are not polling it actually.
c) old_fd = >-1, new_fd = >-1,
We need to stop polling the old_fd and start polling the new one.
If we make poll_stop = true and poll_stop = false, we don't register a
new polling function for the new kick_fd so we will miss guest's
kicks.
If we make poll_stop = false and poll_stop = true, we keep polling the
old file descriptor too, so whatever it gets assigned to could call
vhost_handle_guest_kick if it does not override poll callback.
We *could* detect if old_fd == new_fd so we skip all the work, but I
think it is not worth it to complicate the code, since we're only
being called with the kick_fd at dev start.
d) c) old_fd = >-1, new_fd = -1,
We need to stop polling, or we could get invalid kicks callbacks if it
gets writed after this. No need to poll anything beyond this.
> Thanks
>
>
On Thu, Mar 3, 2022 at 5:25 PM Eugenio Perez Martin <eperezma@redhat.com> wrote:
>
> On Thu, Mar 3, 2022 at 8:12 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> >
> > 在 2022/3/2 上午2:49, Eugenio Perez Martin 写道:
> > > On Mon, Feb 28, 2022 at 3:57 AM Jason Wang<jasowang@redhat.com> wrote:
> > >> 在 2022/2/27 下午9:40, Eugenio Pérez 写道:
> > >>> At this mode no buffer forwarding will be performed in SVQ mode: Qemu
> > >>> will just forward the guest's kicks to the device.
> > >>>
> > >>> Host memory notifiers regions are left out for simplicity, and they will
> > >>> not be addressed in this series.
> > >>>
> > >>> Signed-off-by: Eugenio Pérez<eperezma@redhat.com>
> > >>> ---
> > >>> hw/virtio/vhost-shadow-virtqueue.h | 14 +++
> > >>> include/hw/virtio/vhost-vdpa.h | 4 +
> > >>> hw/virtio/vhost-shadow-virtqueue.c | 52 +++++++++++
> > >>> hw/virtio/vhost-vdpa.c | 145 ++++++++++++++++++++++++++++-
> > >>> 4 files changed, 213 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
> > >>> index f1519e3c7b..1cbc87d5d8 100644
> > >>> --- a/hw/virtio/vhost-shadow-virtqueue.h
> > >>> +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > >>> @@ -18,8 +18,22 @@ typedef struct VhostShadowVirtqueue {
> > >>> EventNotifier hdev_kick;
> > >>> /* Shadow call notifier, sent to vhost */
> > >>> EventNotifier hdev_call;
> > >>> +
> > >>> + /*
> > >>> + * Borrowed virtqueue's guest to host notifier. To borrow it in this event
> > >>> + * notifier allows to recover the VhostShadowVirtqueue from the event loop
> > >>> + * easily. If we use the VirtQueue's one, we don't have an easy way to
> > >>> + * retrieve VhostShadowVirtqueue.
> > >>> + *
> > >>> + * So shadow virtqueue must not clean it, or we would lose VirtQueue one.
> > >>> + */
> > >>> + EventNotifier svq_kick;
> > >>> } VhostShadowVirtqueue;
> > >>>
> > >>> +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd);
> > >>> +
> > >>> +void vhost_svq_stop(VhostShadowVirtqueue *svq);
> > >>> +
> > >>> VhostShadowVirtqueue *vhost_svq_new(void);
> > >>>
> > >>> void vhost_svq_free(gpointer vq);
> > >>> diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
> > >>> index 3ce79a646d..009a9f3b6b 100644
> > >>> --- a/include/hw/virtio/vhost-vdpa.h
> > >>> +++ b/include/hw/virtio/vhost-vdpa.h
> > >>> @@ -12,6 +12,8 @@
> > >>> #ifndef HW_VIRTIO_VHOST_VDPA_H
> > >>> #define HW_VIRTIO_VHOST_VDPA_H
> > >>>
> > >>> +#include <gmodule.h>
> > >>> +
> > >>> #include "hw/virtio/virtio.h"
> > >>> #include "standard-headers/linux/vhost_types.h"
> > >>>
> > >>> @@ -27,6 +29,8 @@ typedef struct vhost_vdpa {
> > >>> bool iotlb_batch_begin_sent;
> > >>> MemoryListener listener;
> > >>> struct vhost_vdpa_iova_range iova_range;
> > >>> + bool shadow_vqs_enabled;
> > >>> + GPtrArray *shadow_vqs;
> > >>> struct vhost_dev *dev;
> > >>> VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX];
> > >>> } VhostVDPA;
> > >>> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> > >>> index 019cf1950f..a5d0659f86 100644
> > >>> --- a/hw/virtio/vhost-shadow-virtqueue.c
> > >>> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > >>> @@ -11,6 +11,56 @@
> > >>> #include "hw/virtio/vhost-shadow-virtqueue.h"
> > >>>
> > >>> #include "qemu/error-report.h"
> > >>> +#include "qemu/main-loop.h"
> > >>> +#include "linux-headers/linux/vhost.h"
> > >>> +
> > >>> +/** Forward guest notifications */
> > >>> +static void vhost_handle_guest_kick(EventNotifier *n)
> > >>> +{
> > >>> + VhostShadowVirtqueue *svq = container_of(n, VhostShadowVirtqueue,
> > >>> + svq_kick);
> > >>> + event_notifier_test_and_clear(n);
> > >>> + event_notifier_set(&svq->hdev_kick);
> > >>> +}
> > >>> +
> > >>> +/**
> > >>> + * Set a new file descriptor for the guest to kick the SVQ and notify for avail
> > >>> + *
> > >>> + * @svq The svq
> > >>> + * @svq_kick_fd The svq kick fd
> > >>> + *
> > >>> + * Note that the SVQ will never close the old file descriptor.
> > >>> + */
> > >>> +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd)
> > >>> +{
> > >>> + EventNotifier *svq_kick = &svq->svq_kick;
> > >>> + bool poll_stop = VHOST_FILE_UNBIND != event_notifier_get_fd(svq_kick);
> > >> I wonder if this is robust. E.g is there any chance that may end up with
> > >> both poll_stop and poll_start are false?
> > >>
> > > I cannot make that happen in qemu, but the function supports that case
> > > well: It will do nothing. It's more or less the same code as used in
> > > the vhost kernel, and is the expected behaviour if you send two
> > > VHOST_FILE_UNBIND one right after another to me.
> >
> >
> > I would think it's just stop twice.
> >
> >
> > >
> > >> If not, can we simple detect poll_stop as below and treat !poll_start
> > >> and poll_stop?
> > >>
> > > I'm not sure what does it add. Is there an unexpected consequence with
> > > the current do-nothing behavior I've missed?
> >
> >
> > I'm not sure, but it feels odd if poll_start is not the reverse value of
> > poll_stop.
> >
>
> If we want to not to restrict the inputs, we need to handle for situations:
>
> a) old_fd = -1, new_fd = -1,
>
> This is the situation you described, and it's basically a no-op.
> poll_stop == poll_start == false.
>
> If we make poll_stop = true and poll_stop = false, we call
> event_notifier_set_handler(-1, ...). Hopefully it will return just an
> error.
>
> If we make poll_stop = false and poll_stop = true, we are calling
> event_notifier_set(-1) and event_notifier_set_handler(-1,
> poll_callback). Same situation, hopefully an error, but unexpected.
>
> b) old_fd = -1, new_fd = >-1,
>
> We need to start polling the new_fd. No need for stop polling the
> old_fd, since we are not polling it actually.
>
> c) old_fd = >-1, new_fd = >-1,
>
> We need to stop polling the old_fd and start polling the new one.
>
> If we make poll_stop = true and poll_stop = false, we don't register a
> new polling function for the new kick_fd so we will miss guest's
> kicks.
>
> If we make poll_stop = false and poll_stop = true, we keep polling the
> old file descriptor too, so whatever it gets assigned to could call
> vhost_handle_guest_kick if it does not override poll callback.
>
> We *could* detect if old_fd == new_fd so we skip all the work, but I
> think it is not worth it to complicate the code, since we're only
> being called with the kick_fd at dev start.
>
> d) c) old_fd = >-1, new_fd = -1,
>
> We need to stop polling, or we could get invalid kicks callbacks if it
> gets writed after this. No need to poll anything beyond this.
I see, thanks for the clarification.
>
> > Thanks
> >
> >
>
© 2016 - 2026 Red Hat, Inc.