1. Patchew
  2. QEMU
  3. View series

[PATCH] libvhost-user: advertise vring features

Stefan Hajnoczi posted 1 patch 3 years, 11 months ago
Test docker-mingw@fedora passed
Test checkpatch passed
Test asan passed
Test docker-quick@centos7 passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200529161338.456017-1-stefanha@redhat.com
contrib/libvhost-user/libvhost-user.c   | 10 ++++++++++
contrib/vhost-user-blk/vhost-user-blk.c |  4 +---
2 files changed, 11 insertions(+), 3 deletions(-)
[PATCH] libvhost-user: advertise vring features
Posted by Stefan Hajnoczi 3 years, 11 months ago 
libvhost-user implements several vring features without advertising
them. There is no way for the vhost-user master to detect support for
these features.

Things more or less work today because QEMU assumes the vhost-user
backend always implements certain feature bits like
VIRTIO_RING_F_EVENT_IDX. This is not documented anywhere.

This patch explicitly advertises features implemented in libvhost-user
so that the vhost-user master does not need to make undocumented
assumptions.

Feature bits that libvhost-user now advertises can be removed from
vhost-user-blk.c. Devices should not be responsible for advertising
vring feature bits, that is libvhost-user's job.

Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
I have tested make check and virtiofsd.
---
 contrib/libvhost-user/libvhost-user.c   | 10 ++++++++++
 contrib/vhost-user-blk/vhost-user-blk.c |  4 +---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c
index 3bca996c62..b43874ba12 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -495,6 +495,16 @@ static bool
 vu_get_features_exec(VuDev *dev, VhostUserMsg *vmsg)
 {
     vmsg->payload.u64 =
+        /*
+         * The following VIRTIO feature bits are supported by our virtqueue
+         * implementation:
+         */
+        1ULL << VIRTIO_F_NOTIFY_ON_EMPTY |
+        1ULL << VIRTIO_RING_F_INDIRECT_DESC |
+        1ULL << VIRTIO_RING_F_EVENT_IDX |
+        1ULL << VIRTIO_F_VERSION_1 |
+
+        /* vhost-user feature bits */
         1ULL << VHOST_F_LOG_ALL |
         1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
 
diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
index 6fd91c7e99..25eccd02b5 100644
--- a/contrib/vhost-user-blk/vhost-user-blk.c
+++ b/contrib/vhost-user-blk/vhost-user-blk.c
@@ -382,9 +382,7 @@ vub_get_features(VuDev *dev)
                1ull << VIRTIO_BLK_F_DISCARD |
                1ull << VIRTIO_BLK_F_WRITE_ZEROES |
                #endif
-               1ull << VIRTIO_BLK_F_CONFIG_WCE |
-               1ull << VIRTIO_F_VERSION_1 |
-               1ull << VHOST_USER_F_PROTOCOL_FEATURES;
+               1ull << VIRTIO_BLK_F_CONFIG_WCE;
 
     if (vdev_blk->enable_ro) {
         features |= 1ull << VIRTIO_BLK_F_RO;
-- 
2.25.4
Re: [PATCH] libvhost-user: advertise vring features
Posted by Marc-André Lureau 3 years, 11 months ago 
Hi

On Fri, May 29, 2020 at 6:13 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> libvhost-user implements several vring features without advertising
> them. There is no way for the vhost-user master to detect support for
> these features.
>
> Things more or less work today because QEMU assumes the vhost-user
> backend always implements certain feature bits like
> VIRTIO_RING_F_EVENT_IDX. This is not documented anywhere.
>
> This patch explicitly advertises features implemented in libvhost-user
> so that the vhost-user master does not need to make undocumented
> assumptions.
>
> Feature bits that libvhost-user now advertises can be removed from
> vhost-user-blk.c. Devices should not be responsible for advertising
> vring feature bits, that is libvhost-user's job.
>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
> I have tested make check and virtiofsd.
> ---
>  contrib/libvhost-user/libvhost-user.c   | 10 ++++++++++
>  contrib/vhost-user-blk/vhost-user-blk.c |  4 +---
>  2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c
> index 3bca996c62..b43874ba12 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -495,6 +495,16 @@ static bool
>  vu_get_features_exec(VuDev *dev, VhostUserMsg *vmsg)
>  {
>      vmsg->payload.u64 =
> +        /*
> +         * The following VIRTIO feature bits are supported by our virtqueue
> +         * implementation:
> +         */
> +        1ULL << VIRTIO_F_NOTIFY_ON_EMPTY |
> +        1ULL << VIRTIO_RING_F_INDIRECT_DESC |
> +        1ULL << VIRTIO_RING_F_EVENT_IDX |
> +        1ULL << VIRTIO_F_VERSION_1 |
> +
> +        /* vhost-user feature bits */
>          1ULL << VHOST_F_LOG_ALL |
>          1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
>
> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c b/contrib/vhost-user-blk/vhost-user-blk.c
> index 6fd91c7e99..25eccd02b5 100644
> --- a/contrib/vhost-user-blk/vhost-user-blk.c
> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
> @@ -382,9 +382,7 @@ vub_get_features(VuDev *dev)
>                 1ull << VIRTIO_BLK_F_DISCARD |
>                 1ull << VIRTIO_BLK_F_WRITE_ZEROES |
>                 #endif
> -               1ull << VIRTIO_BLK_F_CONFIG_WCE |
> -               1ull << VIRTIO_F_VERSION_1 |
> -               1ull << VHOST_USER_F_PROTOCOL_FEATURES;
> +               1ull << VIRTIO_BLK_F_CONFIG_WCE;
>
>      if (vdev_blk->enable_ro) {
>          features |= 1ull << VIRTIO_BLK_F_RO;
> --
> 2.25.4
>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.