Add packed format to shadow virtqueue

[RFC v5 7/7] vdpa: Support setting vring_base for packed SVQ

Posted by Sahil Siddiq 10 months, 3 weeks ago

This commit is the first in a series to add support for packed
virtqueues in vhost_shadow_virtqueue.

Linux commit 1225c216d954 ("vp_vdpa: allow set vq state to initial
state after reset") enabled the vp_vdpa driver to set the 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
(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-jasowang@redhat.com
Link: 1225c216d954 ("vp_vdpa: allow set vq state to initial state after reset")
Signed-off-by: Sahil Siddiq <sahilcdq@proton.me>
Acked-by: Eugenio Pérez <eperezma@redhat.com>
---
Changes from v4 -> v5:
- Initially commit #5 in v4.
- Fix coding style of commit block as stated by checkpatch.pl.

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

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 58c8931d89..0625e349b3 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1265,6 +1265,21 @@ static bool vhost_vdpa_svq_setup(struct vhost_dev *dev,
     };
     int r;
 
+    /*
+     * In Linux, the upper 16 bits of s.num is encoded as
+     * the last used idx while the lower 16 bits is encoded
+     * as the last avail idx when using packed vqs. The most
+     * significant bit for each idx represents the counter
+     * and should be set in both cases while the remaining
+     * bits are cleared.
+     */
+    if (virtio_vdev_has_feature(dev->vdev, VIRTIO_F_RING_PACKED)) {
+        uint32_t last_avail_idx = 0 | (1 << VRING_PACKED_EVENT_F_WRAP_CTR);
+        uint32_t last_used_idx = 0 | (1 << VRING_PACKED_EVENT_F_WRAP_CTR);
+
+        s.num = (last_used_idx << 16) | last_avail_idx;
+    }
+
     r = vhost_vdpa_set_dev_vring_base(dev, &s);
     if (unlikely(r)) {
         error_setg_errno(errp, -r, "Cannot set vring base");
-- 
2.48.1

Re: [RFC v5 7/7] vdpa: Support setting vring_base for packed SVQ

Posted by Eugenio Perez Martin 10 months, 2 weeks ago

On Mon, Mar 24, 2025 at 3:00 PM Sahil Siddiq <icegambit91@gmail.com> wrote:
>
> This commit is the first in a series to add support for packed
> virtqueues in vhost_shadow_virtqueue.
>
> Linux commit 1225c216d954 ("vp_vdpa: allow set vq state to initial
> state after reset") enabled the vp_vdpa driver to set the 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
> (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-jasowang@redhat.com
> Link: 1225c216d954 ("vp_vdpa: allow set vq state to initial state after reset")
> Signed-off-by: Sahil Siddiq <sahilcdq@proton.me>
> Acked-by: Eugenio Pérez <eperezma@redhat.com>
> ---
> Changes from v4 -> v5:
> - Initially commit #5 in v4.
> - Fix coding style of commit block as stated by checkpatch.pl.
>
>  hw/virtio/vhost-vdpa.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 58c8931d89..0625e349b3 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -1265,6 +1265,21 @@ static bool vhost_vdpa_svq_setup(struct vhost_dev *dev,
>      };
>      int r;
>
> +    /*
> +     * In Linux, the upper 16 bits of s.num is encoded as
> +     * the last used idx while the lower 16 bits is encoded
> +     * as the last avail idx when using packed vqs. The most
> +     * significant bit for each idx represents the counter
> +     * and should be set in both cases while the remaining
> +     * bits are cleared.
> +     */
> +    if (virtio_vdev_has_feature(dev->vdev, VIRTIO_F_RING_PACKED)) {
> +        uint32_t last_avail_idx = 0 | (1 << VRING_PACKED_EVENT_F_WRAP_CTR);
> +        uint32_t last_used_idx = 0 | (1 << VRING_PACKED_EVENT_F_WRAP_CTR);
> +
> +        s.num = (last_used_idx << 16) | last_avail_idx;
> +    }
> +

This should be added before 6/7 so we don't declare we support packed
without this.

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

Re: [RFC v5 7/7] vdpa: Support setting vring_base for packed SVQ

Posted by Sahil Siddiq 10 months, 2 weeks ago

Hi,

On 3/26/25 5:38 PM, Eugenio Perez Martin wrote:
> On Mon, Mar 24, 2025 at 3:00 PM Sahil Siddiq <icegambit91@gmail.com> wrote:
>> [...]
>> Link: https://lists.nongnu.org/archive/html/qemu-devel/2024-10/msg05106.html
>> Link: https://lore.kernel.org/r/20210602021536.39525-4-jasowang@redhat.com
>> Link: 1225c216d954 ("vp_vdpa: allow set vq state to initial state after reset")
>> Signed-off-by: Sahil Siddiq <sahilcdq@proton.me>
>> Acked-by: Eugenio Pérez <eperezma@redhat.com>
>> ---
>> Changes from v4 -> v5:
>> - Initially commit #5 in v4.
>> - Fix coding style of commit block as stated by checkpatch.pl.
>>
>>   hw/virtio/vhost-vdpa.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
>> index 58c8931d89..0625e349b3 100644
>> --- a/hw/virtio/vhost-vdpa.c
>> +++ b/hw/virtio/vhost-vdpa.c
>> @@ -1265,6 +1265,21 @@ static bool vhost_vdpa_svq_setup(struct vhost_dev *dev,
>>       };
>>       int r;
>>
>> +    /*
>> +     * In Linux, the upper 16 bits of s.num is encoded as
>> +     * the last used idx while the lower 16 bits is encoded
>> +     * as the last avail idx when using packed vqs. The most
>> +     * significant bit for each idx represents the counter
>> +     * and should be set in both cases while the remaining
>> +     * bits are cleared.
>> +     */
>> +    if (virtio_vdev_has_feature(dev->vdev, VIRTIO_F_RING_PACKED)) {
>> +        uint32_t last_avail_idx = 0 | (1 << VRING_PACKED_EVENT_F_WRAP_CTR);
>> +        uint32_t last_used_idx = 0 | (1 << VRING_PACKED_EVENT_F_WRAP_CTR);
>> +
>> +        s.num = (last_used_idx << 16) | last_avail_idx;
>> +    }
>> +
> 
> This should be added before 6/7 so we don't declare we support packed
> without this.

Sure, I'll change the ordering in the next patch series.

Thanks,
Sahil

[RFC v5 1/7] vhost: Refactor vhost_svq_add_split
[RFC v5 2/7] vhost: Data structure changes to support packed vqs
[RFC v5 3/7] vhost: Forward descriptors to device via packed SVQ
[RFC v5 4/7] vdpa: Allocate memory for SVQ and map them to vdpa
[RFC v5 5/7] vhost: Forward descriptors to guest via packed vqs
[RFC v5 6/7] vhost: Validate transport device features for packed vqs
[RFC v5 7/7] vdpa: Support setting vring_base for packed SVQ