On Fri, Nov 18, 2022 at 3:32 PM Stefano Garzarella <sgarz...@redhat.com> wrote:
>
> Hi,
> starting from this commit 69e1c14aa2 ("virtio: core: vq reset feature
> negotation support"), vhost-user-vsock and vhost-vsock fails while
> setting the device features, because VIRTIO_F_RING_RESET is not masked.vhost-vsock issue also reported here: https://gitlab.com/qemu-project/qemu/-/issues/1318 > > I'm not sure vsock is the only one affected. > > We could fix in two ways: > > 1) Masking VIRTIO_F_RING_RESET when we call vhost_get_features: > > diff --git a/hw/virtio/vhost-vsock-common.c b/hw/virtio/vhost-vsock-common.c > index 29b9ab4f72..e671cc695f 100644 > --- a/hw/virtio/vhost-vsock-common.c > +++ b/hw/virtio/vhost-vsock-common.c > @@ -21,6 +21,7 @@ > > const int feature_bits[] = { > VIRTIO_VSOCK_F_SEQPACKET, > + VIRTIO_F_RING_RESET, > VHOST_INVALID_FEATURE_BIT > }; > > > 2) Or using directly the features of the device. That way we also handle > other features that we may have already had to mask but never did. > > diff --git a/hw/virtio/vhost-vsock-common.c b/hw/virtio/vhost-vsock-common.c > index 29b9ab4f72..41a665082c 100644 > --- a/hw/virtio/vhost-vsock-common.c > +++ b/hw/virtio/vhost-vsock-common.c > @@ -33,7 +33,7 @@ uint64_t vhost_vsock_common_get_features(VirtIODevice > *vdev, uint64_t features, > virtio_add_feature(&features, VIRTIO_VSOCK_F_SEQPACKET); > } > > - features = vhost_get_features(&vvc->vhost_dev, feature_bits, features); > + features &= vvc->vhost_dev.features; > > if (vvc->seqpacket == ON_OFF_AUTO_ON && > !virtio_has_feature(features, VIRTIO_VSOCK_F_SEQPACKET)) { > > > I may be missing the real reason for calling vhost_get_features(), > though. > > @Michael what do you recommend we do? > > Thanks, > Stefano > > On Tue, Nov 8, 2022 at 12:06 AM Michael S. Tsirkin <m...@redhat.com> wrote: > > > > From: Kangjie Xu <kangjie...@linux.alibaba.com> > > > > A a new command line parameter "queue_reset" is added. > > > > Meanwhile, the vq reset feature is disabled for pre-7.2 machines. > > > > Signed-off-by: Kangjie Xu <kangjie...@linux.alibaba.com> > > Signed-off-by: Xuan Zhuo <xuanz...@linux.alibaba.com> > > Acked-by: Jason Wang <jasow...@redhat.com> > > Message-Id: <20221017092558.111082-5-xuanz...@linux.alibaba.com> > > Reviewed-by: Michael S. Tsirkin <m...@redhat.com> > > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > > --- > > include/hw/virtio/virtio.h | 4 +++- > > hw/core/machine.c | 4 +++- > > 2 files changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > > index b00b3fcf31..1423dba379 100644 > > --- a/include/hw/virtio/virtio.h > > +++ b/include/hw/virtio/virtio.h > > @@ -313,7 +313,9 @@ typedef struct VirtIORNGConf VirtIORNGConf; > > DEFINE_PROP_BIT64("iommu_platform", _state, _field, \ > > VIRTIO_F_IOMMU_PLATFORM, false), \ > > DEFINE_PROP_BIT64("packed", _state, _field, \ > > - VIRTIO_F_RING_PACKED, false) > > + VIRTIO_F_RING_PACKED, false), \ > > + DEFINE_PROP_BIT64("queue_reset", _state, _field, \ > > + VIRTIO_F_RING_RESET, true) > > > > hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n); > > bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n); > > diff --git a/hw/core/machine.c b/hw/core/machine.c > > index aa520e74a8..907fa78ff0 100644 > > --- a/hw/core/machine.c > > +++ b/hw/core/machine.c > > @@ -40,7 +40,9 @@ > > #include "hw/virtio/virtio-pci.h" > > #include "qom/object_interfaces.h" > > > > -GlobalProperty hw_compat_7_1[] = {}; > > +GlobalProperty hw_compat_7_1[] = { > > + { "virtio-device", "queue_reset", "false" }, > > +}; > > const size_t hw_compat_7_1_len = G_N_ELEMENTS(hw_compat_7_1); > > > > GlobalProperty hw_compat_7_0[] = { > > -- > > MST > > > >
