Re: [PATCH v2 1/3] virtio_ring: Allow non power of 2 sizes for packed virtqueue
On 2023-03-16 p.m.11:16, Jason Wang wrote:
External email: Use caution opening links or attachments
On Thu, Mar 16, 2023 at 2:55 AM Feng Liu wrote:
According to the Virtio Specification, the Queue Size parameter of a
virtqueue corresponds to the maximum number of descriptors in that
queue, and it does not have to be a power of 2 for packed virtqueues.
However, the virtio_pci_modern driver enforced a power of 2 check for
virtqueue sizes, which is unnecessary and restrictive for packed
virtuqueue.
Split virtqueue still needs to check the virtqueue size is power_of_2
which has been done in vring_alloc_queue_split of the virtio_ring layer.
To validate this change, we tested various virtqueue sizes for packed
rings, including 128, 256, 512, 100, 200, 500, and 1000, with
CONFIG_PAGE_POISONING enabled, and all tests passed successfully.
Signed-off-by: Feng Liu
Reviewed-by: Jiri Pirko
Acked-by: Jason Wang
Thanks
Hi Michael & Jason
Could you please help to take these reviewed/acked patches forward?
Thanks so much
---
v0 -> v1
feedbacks from Jason Wang and Michael S. Tsirkin
- remove power_of_2 check of virtqueue size
v1 -> v2
feedbacks from Parav Pandit and Jiri Pirko
- keep power_of_2 check of split virtqueue in vring_alloc_queue_split of
virtio_ring layer.
---
drivers/virtio/virtio_pci_modern.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c
b/drivers/virtio/virtio_pci_modern.c
index 9e496e288cfa..6e713904d8e8 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -310,11 +310,6 @@ static struct virtqueue *setup_vq(struct virtio_pci_device
*vp_dev,
if (!num || vp_modern_get_queue_enable(mdev, index))
return ERR_PTR(-ENOENT);
- if (!is_power_of_2(num)) {
- dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u", num);
- return ERR_PTR(-EINVAL);
- }
-
info->msix_vector = msix_vec;
/* create the vring */
--
2.34.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 1/3] virtio_ring: Allow non power of 2 sizes for packed virtqueue
Feng Liu writes:
> According to the Virtio Specification, the Queue Size parameter of a
> virtqueue corresponds to the maximum number of descriptors in that
> queue, and it does not have to be a power of 2 for packed virtqueues.
> However, the virtio_pci_modern driver enforced a power of 2 check for
> virtqueue sizes, which is unnecessary and restrictive for packed
> virtuqueue.
>
> Split virtqueue still needs to check the virtqueue size is power_of_2
> which has been done in vring_alloc_queue_split of the virtio_ring layer.
>
> To validate this change, we tested various virtqueue sizes for packed
> rings, including 128, 256, 512, 100, 200, 500, and 1000, with
> CONFIG_PAGE_POISONING enabled, and all tests passed successfully.
>
> Signed-off-by: Feng Liu
> Reviewed-by: Jiri Pirko
>
Reviewed-by: David Edmondson
> ---
> v0 -> v1
> feedbacks from Jason Wang and Michael S. Tsirkin
> - remove power_of_2 check of virtqueue size
>
> v1 -> v2
> feedbacks from Parav Pandit and Jiri Pirko
> - keep power_of_2 check of split virtqueue in vring_alloc_queue_split of
> virtio_ring layer.
> ---
> drivers/virtio/virtio_pci_modern.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_modern.c
> b/drivers/virtio/virtio_pci_modern.c
> index 9e496e288cfa..6e713904d8e8 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -310,11 +310,6 @@ static struct virtqueue *setup_vq(struct
> virtio_pci_device *vp_dev,
> if (!num || vp_modern_get_queue_enable(mdev, index))
> return ERR_PTR(-ENOENT);
>
> - if (!is_power_of_2(num)) {
> - dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u", num);
> - return ERR_PTR(-EINVAL);
> - }
> -
> info->msix_vector = msix_vec;
>
> /* create the vring */
> --
> 2.34.1
--
Do not leave the building.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 1/3] virtio_ring: Allow non power of 2 sizes for packed virtqueue
On Thu, Mar 16, 2023 at 2:55 AM Feng Liu wrote:
>
> According to the Virtio Specification, the Queue Size parameter of a
> virtqueue corresponds to the maximum number of descriptors in that
> queue, and it does not have to be a power of 2 for packed virtqueues.
> However, the virtio_pci_modern driver enforced a power of 2 check for
> virtqueue sizes, which is unnecessary and restrictive for packed
> virtuqueue.
>
> Split virtqueue still needs to check the virtqueue size is power_of_2
> which has been done in vring_alloc_queue_split of the virtio_ring layer.
>
> To validate this change, we tested various virtqueue sizes for packed
> rings, including 128, 256, 512, 100, 200, 500, and 1000, with
> CONFIG_PAGE_POISONING enabled, and all tests passed successfully.
>
> Signed-off-by: Feng Liu
> Reviewed-by: Jiri Pirko
Acked-by: Jason Wang
Thanks
>
> ---
> v0 -> v1
> feedbacks from Jason Wang and Michael S. Tsirkin
> - remove power_of_2 check of virtqueue size
>
> v1 -> v2
> feedbacks from Parav Pandit and Jiri Pirko
> - keep power_of_2 check of split virtqueue in vring_alloc_queue_split of
> virtio_ring layer.
> ---
> drivers/virtio/virtio_pci_modern.c | 5 -
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_modern.c
> b/drivers/virtio/virtio_pci_modern.c
> index 9e496e288cfa..6e713904d8e8 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -310,11 +310,6 @@ static struct virtqueue *setup_vq(struct
> virtio_pci_device *vp_dev,
> if (!num || vp_modern_get_queue_enable(mdev, index))
> return ERR_PTR(-ENOENT);
>
> - if (!is_power_of_2(num)) {
> - dev_warn(&vp_dev->pci_dev->dev, "bad queue size %u", num);
> - return ERR_PTR(-EINVAL);
> - }
> -
> info->msix_vector = msix_vec;
>
> /* create the vring */
> --
> 2.34.1
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
