| Message ID | 20200529161338.456017-1-stefanha@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | libvhost-user: advertise vring features | expand |
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 >
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;
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(-)