On Mon, Feb 14, 2022 at 4:15 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote:
>
> Added helper virtio_set_max_ring_num() to set the upper limit of ring
> num when creating a virtqueue.
>
> Can be used to limit ring num before find_vqs() call. Or change ring num
> when re-enable reset queue.
Do we have a chance that RX and TX may want different ring size? If
yes, it might be even better to have per vq limit via find_vqs()?
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> ---
> drivers/virtio/virtio_ring.c | 6 ++++++
> include/linux/virtio.h | 1 +
> include/linux/virtio_config.h | 30 ++++++++++++++++++++++++++++++
> 3 files changed, 37 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 1a123b5e5371..a77a82883e44 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -943,6 +943,9 @@ static struct virtqueue *vring_create_virtqueue_split(
> size_t queue_size_in_bytes;
> struct vring vring;
>
> + if (vdev->max_ring_num && num > vdev->max_ring_num)
> + num = vdev->max_ring_num;
> +
> /* We assume num is a power of 2. */
> if (num & (num - 1)) {
> dev_warn(&vdev->dev, "Bad virtqueue length %u\n", num);
> @@ -1692,6 +1695,9 @@ static struct virtqueue *vring_create_virtqueue_packed(
> dma_addr_t ring_dma_addr, driver_event_dma_addr, device_event_dma_addr;
> size_t ring_size_in_bytes, event_size_in_bytes;
>
> + if (vdev->max_ring_num && num > vdev->max_ring_num)
> + num = vdev->max_ring_num;
> +
> ring_size_in_bytes = num * sizeof(struct vring_packed_desc);
>
> ring = vring_alloc_queue(vdev, ring_size_in_bytes,
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index 1153b093c53d..45525beb2ec4 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -127,6 +127,7 @@ struct virtio_device {
> struct list_head vqs;
> u64 features;
> void *priv;
> + u16 max_ring_num;
> };
>
> static inline struct virtio_device *dev_to_virtio(struct device *_dev)
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index cd7f7f44ce38..d7cb2d0341ee 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -200,6 +200,36 @@ static inline bool virtio_has_dma_quirk(const struct virtio_device *vdev)
> return !virtio_has_feature(vdev, VIRTIO_F_ACCESS_PLATFORM);
> }
>
> +/**
> + * virtio_set_max_ring_num - set max ring num
> + * @vdev: the device
> + * @num: max ring num. Zero clear the limit.
> + *
> + * When creating a virtqueue, use this value as the upper limit of ring num.
> + *
> + * Returns 0 on success or error status
> + */
> +static inline
> +int virtio_set_max_ring_num(struct virtio_device *vdev, u16 num)
> +{
Having a dedicated helper for a per device parameter usually means the
use cases are greatly limited. For example, this seems can only be
used when DRIVER_OK is not set?
And in patch 17 this function is called even if we only modify the RX
size, this is probably another call for a more flexible API as I
suggest like exporting vring allocation/deallocation helper and extend
find_vqs()?
Thanks
> + if (!num) {
> + vdev->max_ring_num = num;
> + return 0;
> + }
> +
> + if (!virtio_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
> + if (!is_power_of_2(num)) {
> + num = __rounddown_pow_of_two(num);
> +
> + if (!num)
> + return -EINVAL;
> + }
> + }
> +
> + vdev->max_ring_num = num;
> + return 0;
> +}
> +
> static inline
> struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev,
> vq_callback_t *c, const char *n)
> --
> 2.31.0
>
