On 5/16/23 12:49 AM, Stefano Garzarella wrote:
On Mon, May 15, 2023 at 01:41:12PM -0700, Shannon Nelson wrote:
On 5/9/23 1:46 AM, Stefano Garzarella wrote:
On Mon, Apr 24, 2023 at 03:50:30PM -0700, Shannon Nelson via
Virtualization wrote:
Use the right structs for PACKED or split vqs when setting and
getting the vring base.
Signed-off-by: Shannon Nelson <shannon.nelson@xxxxxxx>
---
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;
The changes LGTM, but since we are changing the UAPI, should we
update the documentation of VHOST_SET_VRING_BASE and
VHOST_GET_VRING_BASE in include/uapi/linux/vhost.h?
Correct me if I'm wrong, but I don't think we're changing anything in
the UAPI here, just fixing code to work correctly with what is already
happening.
IIUC before this patch VHOST_GET_VRING_BASE and VHOST_SET_VRING_BASE
never worked with packed virtqueue, since we were only handling
last_avail_idx. Now we are supporting packed virtqueue, handling
in vhost_vring_state.num both last_avail_idx and last_used_idx (with
wrap counters).
For example for VHOST_GET_VRING_BASE where is documented that the first
15 bits are last_avail_idx, the 16th the avail_wrap_counter, and the
others are last_used_idx and used_wrap_counter?
Maybe I missed something, but since this is UAPI, IMHO we should
document the parameters of ioctls at least in
include/uapi/linux/vhost.h.
Perhaps Jason already has something written up that could be put in here
from when he first added the wrap_counter a couple of years ago?
sln
Thanks,
Stefano
--
You received this message because you are subscribed to the Google
Groups "Pensando Drivers" group.
To unsubscribe from this group and stop receiving emails from it, send
an email to drivers+unsubscribe@xxxxxxxxxxx.
To view this discussion on the web visit
https://groups.google.com/a/pensando.io/d/msgid/drivers/q6cmfha36sdkgflwwd3pr4sw7rgajag4ahgjbpfjrr76w4o2b6%403yc7zs5u65s4.
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
