While testing our vDPA driver with QEMU we found that vhost_vdpa was missing some support for PACKED VQs. Adding these helped us get further in our testing. The first patch makes sure that the vhost_vdpa vqs are given the feature flags, as done in other vhost variants. The second and third patches use the feature flags to properly format and pass set/get_vq requests. v1: - included wrap counter in packing answers for VHOST_GET_VRING_BASE - added comments to vhost_virtqueue fields last_avail_idx and last_used_idx v0: https://lists.linuxfoundation.org/pipermail/virtualization/2023-April/066253.html - initial version Shannon Nelson (3): vhost_vdpa: tell vqs about the negotiated vhost: support PACKED when setting-getting vring_base vhost_vdpa: support PACKED when setting-getting vring_base drivers/vhost/vdpa.c | 34 ++++++++++++++++++++++++++++++---- drivers/vhost/vhost.c | 18 +++++++++++++----- drivers/vhost/vhost.h | 8 ++++++-- 3 files changed, 49 insertions(+), 11 deletions(-) -- 2.17.1
Shannon Nelson
2023-Apr-24 22:50 UTC
[PATCH v2 1/3] vhost_vdpa: tell vqs about the negotiated
As is done in the net, iscsi, and vsock vhost support, let the vdpa vqs
know about the features that have been negotiated. This allows vhost
to more safely make decisions based on the features, such as when using
PACKED vs split queues.
Signed-off-by: Shannon Nelson <shannon.nelson at amd.com>
Acked-by: Jason Wang <jasowang at redhat.com>
---
drivers/vhost/vdpa.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 7be9d9d8f01c..599b8cc238c7 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -385,7 +385,10 @@ static long vhost_vdpa_set_features(struct vhost_vdpa *v,
u64 __user *featurep)
{
struct vdpa_device *vdpa = v->vdpa;
const struct vdpa_config_ops *ops = vdpa->config;
+ struct vhost_dev *d = &v->vdev;
+ u64 actual_features;
u64 features;
+ int i;
/*
* It's not allowed to change the features after they have
@@ -400,6 +403,16 @@ static long vhost_vdpa_set_features(struct vhost_vdpa *v,
u64 __user *featurep)
if (vdpa_set_features(vdpa, features))
return -EINVAL;
+ /* let the vqs know what has been configured */
+ actual_features = ops->get_driver_features(vdpa);
+ for (i = 0; i < d->nvqs; ++i) {
+ struct vhost_virtqueue *vq = d->vqs[i];
+
+ mutex_lock(&vq->mutex);
+ vq->acked_features = actual_features;
+ mutex_unlock(&vq->mutex);
+ }
+
return 0;
}
--
2.17.1
Shannon Nelson
2023-Apr-24 22:50 UTC
[PATCH v2 2/3] vhost: support PACKED when setting-getting vring_base
Use the right structs for PACKED or split vqs when setting and
getting the vring base.
Signed-off-by: Shannon Nelson <shannon.nelson at amd.com>
---
drivers/vhost/vhost.c | 18 +++++++++++++-----
drivers/vhost/vhost.h | 8 ++++++--
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index f11bdbe4c2c5..f64efda48f21 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1633,17 +1633,25 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int
ioctl, void __user *arg
r = -EFAULT;
break;
}
- if (s.num > 0xffff) {
- r = -EINVAL;
- break;
+ if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED)) {
+ vq->last_avail_idx = s.num & 0xffff;
+ vq->last_used_idx = (s.num >> 16) & 0xffff;
+ } else {
+ if (s.num > 0xffff) {
+ r = -EINVAL;
+ break;
+ }
+ vq->last_avail_idx = s.num;
}
- vq->last_avail_idx = s.num;
/* Forget the cached index value. */
vq->avail_idx = vq->last_avail_idx;
break;
case VHOST_GET_VRING_BASE:
s.index = idx;
- s.num = vq->last_avail_idx;
+ if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED))
+ s.num = (u32)vq->last_avail_idx | ((u32)vq->last_used_idx <<
16);
+ else
+ s.num = vq->last_avail_idx;
if (copy_to_user(argp, &s, sizeof s))
r = -EFAULT;
break;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 1647b750169c..6f73f29d5979 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -85,13 +85,17 @@ struct vhost_virtqueue {
/* The routine to call when the Guest pings us, or timeout. */
vhost_work_fn_t handle_kick;
- /* Last available index we saw. */
+ /* Last available index we saw.
+ * Values are limited to 0x7fff, and the high bit is used as
+ * a wrap counter when using VIRTIO_F_RING_PACKED. */
u16 last_avail_idx;
/* Caches available index value from user. */
u16 avail_idx;
- /* Last index we used. */
+ /* Last index we used.
+ * Values are limited to 0x7fff, and the high bit is used as
+ * a wrap counter when using VIRTIO_F_RING_PACKED. */
u16 last_used_idx;
/* Used flags */
--
2.17.1
Shannon Nelson
2023-Apr-24 22:50 UTC
[PATCH v2 3/3] vhost_vdpa: support PACKED when setting-getting vring_base
Use the right structs for PACKED or split vqs when setting and
getting the vring base.
Signed-off-by: Shannon Nelson <shannon.nelson at amd.com>
---
drivers/vhost/vdpa.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 599b8cc238c7..fe392b67d5be 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -585,7 +585,14 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v,
unsigned int cmd,
if (r)
return r;
- vq->last_avail_idx = vq_state.split.avail_index;
+ if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED)) {
+ vq->last_avail_idx = vq_state.packed.last_avail_idx |
+ (vq_state.packed.last_avail_counter << 15);
+ vq->last_used_idx = vq_state.packed.last_used_idx |
+ (vq_state.packed.last_used_counter << 15);
+ } else {
+ vq->last_avail_idx = vq_state.split.avail_index;
+ }
break;
}
@@ -603,9 +610,15 @@ static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v,
unsigned int cmd,
break;
case VHOST_SET_VRING_BASE:
- vq_state.split.avail_index = vq->last_avail_idx;
- if (ops->set_vq_state(vdpa, idx, &vq_state))
- r = -EINVAL;
+ if (vhost_has_feature(vq, VIRTIO_F_RING_PACKED)) {
+ vq_state.packed.last_avail_idx = vq->last_avail_idx & 0x7fff;
+ vq_state.packed.last_avail_counter = !!(vq->last_avail_idx & 0x8000);
+ vq_state.packed.last_used_idx = vq->last_used_idx & 0x7fff;
+ vq_state.packed.last_used_counter = !!(vq->last_used_idx & 0x8000);
+ } else {
+ vq_state.split.avail_index = vq->last_avail_idx;
+ }
+ r = ops->set_vq_state(vdpa, idx, &vq_state);
break;
case VHOST_SET_VRING_CALL:
--
2.17.1
On Mon, Apr 24, 2023 at 03:50:28PM -0700, Shannon Nelson wrote:> While testing our vDPA driver with QEMU we found that vhost_vdpa was > missing some support for PACKED VQs. Adding these helped us get further > in our testing. The first patch makes sure that the vhost_vdpa vqs are > given the feature flags, as done in other vhost variants. The second > and third patches use the feature flags to properly format and pass > set/get_vq requests.This missed the merge window most likely, unless there's a reason to send a second pull towards the end of it. We'll see.> v1: > - included wrap counter in packing answers for VHOST_GET_VRING_BASE > - added comments to vhost_virtqueue fields last_avail_idx and last_used_idx > > v0: > https://lists.linuxfoundation.org/pipermail/virtualization/2023-April/066253.html > - initial version > > Shannon Nelson (3): > vhost_vdpa: tell vqs about the negotiated > vhost: support PACKED when setting-getting vring_base > vhost_vdpa: support PACKED when setting-getting vring_base > > drivers/vhost/vdpa.c | 34 ++++++++++++++++++++++++++++++---- > drivers/vhost/vhost.c | 18 +++++++++++++----- > drivers/vhost/vhost.h | 8 ++++++-- > 3 files changed, 49 insertions(+), 11 deletions(-) > > -- > 2.17.1