On Mon, Nov 04, 2024 at 09:51:24PM +0530, Sahil Siddiq wrote:
Linux commit v5.14-rc1~30^2~8 enabled the vp_vdpa driver to set the

To refer to a commit, please use the SHA-1 id or even better the form
suggested in
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes

So in this case I'd use:
Linux commit 1225c216d954 ("vp_vdpa: allow set vq state to initial state
after reset")

vq state to the device's initial state. This works differently for
split and packed vqs.

With shadow virtqueues enabled, vhost-vdpa sets the vring base using
the VHOST_SET_VRING_BASE ioctl. The payload (vhost_vring_state)
differs for split and packed vqs. The implementation in QEMU currently
uses the payload required for split vqs (i.e., the num field of
vhost_vring_state is set to 0). The kernel throws EOPNOTSUPP when this
payload is used with packed vqs.

This patch sets the num field in the payload appropriately so vhost-vdpa

I'm not very familiar with shadow virtqueue, so can you elaborate what
"appropriately" means here?

(with the vp_vdpa driver) can use packed svqs.

Link: https://lists.nongnu.org/archive/html/qemu-devel/2024-10/msg05106.html
Link: https://lore.kernel.org/r/20210602021536.39525-4-jasow...@redhat.com
Signed-off-by: Sahil Siddiq <sahil...@proton.me>
---
QEMU currently does not support packed vhost shadow virtqueues. I am
working on adding support for packed svqs [1]. The test environment
that I am using [2] requires vhost-vdpa to use the relevant payload
when setting vring base.

[1] https://wiki.qemu.org/Internships/ProjectIdeas/PackedShadowVirtqueue
[2] 
https://www.redhat.com/en/blog/hands-vdpa-what-do-you-do-when-you-aint-got-hardware-part-2

hw/virtio/vhost-vdpa.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 3cdaa12ed5..5f81945109 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1230,6 +1230,10 @@ static bool vhost_vdpa_svq_setup(struct vhost_dev *dev,
    };
    int r;

+    if (virtio_vdev_has_feature(dev->vdev, VIRTIO_F_RING_PACKED)) {
+        s.num = 0x80008000;

Why this magic value?

Looking at the kernel code it looks like we are assgining 0x8000 for
both last_avail_idx and last_used_idx, but why 0x8000?

Thanks,
Stefano

+    }
+
    r = vhost_vdpa_set_dev_vring_base(dev, &s);
    if (unlikely(r)) {
        error_setg_errno(errp, -r, "Cannot set vring base");
--
2.47.0



Reply via email to