@@ -112,6 +112,7 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n,
bool mask);
uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
uint64_t features);
+uint64_t vhost_get_default_features(struct vhost_dev *hdev, uint64_t features);
void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
uint64_t features);
bool vhost_has_free_slot(void);
@@ -192,6 +192,8 @@ bool virtio_gpu_base_device_realize(DeviceState *qdev,
void virtio_gpu_base_reset(VirtIOGPUBase *g);
void virtio_gpu_base_fill_display_info(VirtIOGPUBase *g,
struct virtio_gpu_resp_display_info *dpy_info);
+uint64_t virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features,
+ Error **errp);
/* virtio-gpu.c */
void virtio_gpu_ctrl_response(VirtIOGPU *g,
@@ -122,6 +122,17 @@ int cryptodev_vhost_start(VirtIODevice *dev, int total_queues);
*/
void cryptodev_vhost_stop(VirtIODevice *dev, int total_queues);
+/**
+ * cryptodev_vhost_get_features:
+ * @dev: the virtio crypto object
+ * @requested_features: the features being offered
+ *
+ * Returns: the requested features bits that are supported by the vhost device,
+ * or the original request feature bits if vhost is disabled
+ *
+ */
+uint64_t cryptodev_vhost_get_features(VirtIODevice *dev, uint64_t features);
+
/**
* cryptodev_vhost_virtqueue_mask:
* @dev: the virtio crypto object
@@ -266,6 +266,20 @@ void cryptodev_vhost_stop(VirtIODevice *dev, int total_queues)
assert(r >= 0);
}
+uint64_t cryptodev_vhost_get_features(VirtIODevice *dev, uint64_t features)
+{
+ VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(dev);
+ CryptoDevBackend *b = vcrypto->cryptodev;
+ CryptoDevBackendClient *cc = b->conf.peers.ccs[0];
+ CryptoDevBackendVhost *vhost_crypto = cryptodev_get_vhost(cc, b, 0);
+
+ if (!vhost_crypto) {
+ return features; /* vhost disabled */
+ }
+
+ return vhost_get_default_features(&vhost_crypto->dev, features);
+}
+
void cryptodev_vhost_virtqueue_mask(VirtIODevice *dev,
int queue,
int idx, bool mask)
@@ -333,6 +347,11 @@ void cryptodev_vhost_stop(VirtIODevice *dev, int total_queues)
{
}
+uint64_t cryptodev_vhost_get_features(VirtIODevice *dev, uint64_t features)
+{
+ return features;
+}
+
void cryptodev_vhost_virtqueue_mask(VirtIODevice *dev,
int queue,
int idx, bool mask)
@@ -466,6 +466,22 @@ vhost_user_gpu_set_config(VirtIODevice *vdev,
}
}
+static uint64_t
+vhost_user_gpu_get_features(VirtIODevice *vdev, uint64_t features,
+ Error **errp)
+{
+ VhostUserGPU *g = VHOST_USER_GPU(vdev);
+ Error *local_err = NULL;
+
+ features = virtio_gpu_base_get_features(vdev, features, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return 0;
+ }
+
+ return vhost_get_default_features(&g->vhost->dev, features);
+}
+
static void
vhost_user_gpu_set_status(VirtIODevice *vdev, uint8_t val)
{
@@ -582,6 +598,7 @@ vhost_user_gpu_class_init(ObjectClass *klass, void *data)
vdc->realize = vhost_user_gpu_device_realize;
vdc->reset = vhost_user_gpu_reset;
+ vdc->get_features = vhost_user_gpu_get_features;
vdc->set_status = vhost_user_gpu_set_status;
vdc->guest_notifier_mask = vhost_user_gpu_guest_notifier_mask;
vdc->guest_notifier_pending = vhost_user_gpu_guest_notifier_pending;
@@ -176,7 +176,7 @@ virtio_gpu_base_device_realize(DeviceState *qdev,
return true;
}
-static uint64_t
+uint64_t
virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features,
Error **errp)
{
@@ -45,6 +45,14 @@ static void vhost_input_change_active(VirtIOInput *vinput)
}
}
+static uint64_t vhost_input_get_features(VirtIODevice *vdev, uint64_t features,
+ Error **errp)
+{
+ VHostUserInput *vhi = VHOST_USER_INPUT(vdev);
+
+ return vhost_get_default_features(&vhi->vhost->dev, features);
+}
+
static void vhost_input_get_config(VirtIODevice *vdev, uint8_t *config_data)
{
VirtIOInput *vinput = VIRTIO_INPUT(vdev);
@@ -89,6 +97,7 @@ static void vhost_input_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
dc->vmsd = &vmstate_vhost_input;
+ vdc->get_features = vhost_input_get_features;
vdc->get_config = vhost_input_get_config;
vdc->set_config = vhost_input_set_config;
vic->realize = vhost_input_realize;
@@ -130,8 +130,9 @@ static uint64_t vuf_get_features(VirtIODevice *vdev,
uint64_t requested_features,
Error **errp)
{
- /* No feature bits used yet */
- return requested_features;
+ VHostUserFS *fs = VHOST_USER_FS(vdev);
+
+ return vhost_get_default_features(&fs->vhost_dev, requested_features);
}
static void vuf_handle_output(VirtIODevice *vdev, VirtQueue *vq)
@@ -180,8 +180,9 @@ static uint64_t vhost_vsock_get_features(VirtIODevice *vdev,
uint64_t requested_features,
Error **errp)
{
- /* No feature bits used yet */
- return requested_features;
+ VHostVSock *vsock = VHOST_VSOCK(vdev);
+
+ return vhost_get_default_features(&vsock->vhost_dev, requested_features);
}
static void vhost_vsock_handle_output(VirtIODevice *vdev, VirtQueue *vq)
@@ -48,6 +48,23 @@ static unsigned int used_memslots;
static QLIST_HEAD(, vhost_dev) vhost_devices =
QLIST_HEAD_INITIALIZER(vhost_devices);
+/*
+ * Feature bits that device backends must explicitly report. Feature bits not
+ * listed here maybe set by QEMU without checking with the device backend.
+ * Ideally all feature bits would be listed here but existing vhost device
+ * implementations do not explicitly report bits like VIRTIO_F_VERSION_1, so we
+ * can only assume they are supported.
+ *
+ * New feature bits added to the VIRTIO spec should usually be included here
+ * so that existing vhost device backends that do not support them yet continue
+ * to work.
+ */
+static const int vhost_default_feature_bits[] = {
+ VIRTIO_F_IOMMU_PLATFORM,
+ VIRTIO_F_RING_PACKED,
+ VHOST_INVALID_FEATURE_BIT
+};
+
bool vhost_has_free_slot(void)
{
unsigned int slots_limit = ~0U;
@@ -1468,6 +1485,11 @@ uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits,
return features;
}
+uint64_t vhost_get_default_features(struct vhost_dev *hdev, uint64_t features)
+{
+ return vhost_get_features(hdev, vhost_default_feature_bits, features);
+}
+
void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits,
uint64_t features)
{
@@ -739,7 +739,8 @@ static uint64_t virtio_crypto_get_features(VirtIODevice *vdev,
uint64_t features,
Error **errp)
{
- return features;
+ /* Just returns features when vhost is disabled */
+ return cryptodev_vhost_get_features(vdev, features);
}
static void virtio_crypto_reset(VirtIODevice *vdev)
Many vhost devices in QEMU currently do not involve the device backend in feature negotiation. This seems fine at first glance for device types without their own feature bits (virtio-net has many but other device types have none). This overlooks the fact that QEMU's virtqueue implementation and the device backend's implementation may support different features. QEMU must not report features to the guest that the the device backend doesn't support. For example, QEMU supports VIRTIO 1.1 packed virtqueues while many existing vhost device backends do not. When the user sets packed=on the device backend breaks. This should have been handled gracefully by feature negotiation instead. Introduce vhost_get_default_features() and update all vhost devices in QEMU to involve the device backend in feature negotiation. This patch fixes the following error: $ x86_64-softmmu/qemu-system-x86_64 \ -drive if=virtio,file=test.img,format=raw \ -chardev socket,path=/tmp/vhost-user-blk.sock,id=char0 \ -device vhost-user-blk-pci,chardev=char0,packed=on \ -object memory-backend-memfd,size=1G,share=on,id=ram0 \ -M accel=kvm,memory-backend=ram0 qemu-system-x86_64: Failed to set msg fds. qemu-system-x86_64: vhost VQ 0 ring restore failed: -1: Success (0) The vhost-user-blk backend failed as follows: $ ./vhost-user-blk --socket-path=/tmp/vhost-user-blk.sock -b test2.img vu_panic: virtio: zero sized buffers are not allowed virtio-blk request missing headers Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- include/hw/virtio/vhost.h | 1 + include/hw/virtio/virtio-gpu.h | 2 ++ include/sysemu/cryptodev-vhost.h | 11 +++++++++++ backends/cryptodev-vhost.c | 19 +++++++++++++++++++ hw/display/vhost-user-gpu.c | 17 +++++++++++++++++ hw/display/virtio-gpu-base.c | 2 +- hw/input/vhost-user-input.c | 9 +++++++++ hw/virtio/vhost-user-fs.c | 5 +++-- hw/virtio/vhost-vsock.c | 5 +++-- hw/virtio/vhost.c | 22 ++++++++++++++++++++++ hw/virtio/virtio-crypto.c | 3 ++- 11 files changed, 90 insertions(+), 6 deletions(-)