Re: [PATCH v2 2/2] virtio: fix vq # for balloon

From: Mathieu Poirier
Date: Wed Jul 10 2024 - 11:29:40 EST


On Wed, 10 Jul 2024 at 05:43, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:
>
> virtio balloon communicates to the core that in some
> configurations vq #s are non-contiguous by setting name
> pointer to NULL.
>
> Unfortunately, core then turned around and just made them
> contiguous again. Result is that driver is out of spec.
>
> Implement what the API was supposed to do
> in the 1st place. Compatibility with buggy hypervisors
> is handled inside virtio-balloon, which is the only driver
> making use of this facility, so far.
>
> Message-ID: <cover.1720173841.git.mst@xxxxxxxxxx>
> Fixes: b0c504f15471 ("virtio-balloon: add support for providing free page reports to host")
> Cc: "Alexander Duyck" <alexander.h.duyck@xxxxxxxxxxxxxxx>
> Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> ---
> arch/um/drivers/virtio_uml.c | 4 ++--
> drivers/remoteproc/remoteproc_virtio.c | 4 ++--

Reviewed-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>

> drivers/s390/virtio/virtio_ccw.c | 4 ++--
> drivers/virtio/virtio_mmio.c | 4 ++--
> drivers/virtio/virtio_pci_common.c | 11 ++++++++---
> drivers/virtio/virtio_vdpa.c | 4 ++--
> 6 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c
> index 2b6e701776b6..c903e4959f51 100644
> --- a/arch/um/drivers/virtio_uml.c
> +++ b/arch/um/drivers/virtio_uml.c
> @@ -1019,7 +1019,7 @@ static int vu_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> struct irq_affinity *desc)
> {
> struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev);
> - int i, queue_idx = 0, rc;
> + int i, rc;
> struct virtqueue *vq;
>
> /* not supported for now */
> @@ -1038,7 +1038,7 @@ static int vu_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> continue;
> }
>
> - vqs[i] = vu_setup_vq(vdev, queue_idx++, vqi->callback,
> + vqs[i] = vu_setup_vq(vdev, i, vqi->callback,
> vqi->name, vqi->ctx);
> if (IS_ERR(vqs[i])) {
> rc = PTR_ERR(vqs[i]);
> diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
> index d3f39009b28e..1019b2825c26 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
> @@ -185,7 +185,7 @@ static int rproc_virtio_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
> struct virtqueue_info vqs_info[],
> struct irq_affinity *desc)
> {
> - int i, ret, queue_idx = 0;
> + int i, ret;
>
> for (i = 0; i < nvqs; ++i) {
> struct virtqueue_info *vqi = &vqs_info[i];
> @@ -195,7 +195,7 @@ static int rproc_virtio_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
> continue;
> }
>
> - vqs[i] = rp_find_vq(vdev, queue_idx++, vqi->callback,
> + vqs[i] = rp_find_vq(vdev, i, vqi->callback,
> vqi->name, vqi->ctx);
> if (IS_ERR(vqs[i])) {
> ret = PTR_ERR(vqs[i]);
> diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
> index 62eca9419ad7..82a3440bbabb 100644
> --- a/drivers/s390/virtio/virtio_ccw.c
> +++ b/drivers/s390/virtio/virtio_ccw.c
> @@ -694,7 +694,7 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> {
> struct virtio_ccw_device *vcdev = to_vc_device(vdev);
> dma64_t *indicatorp = NULL;
> - int ret, i, queue_idx = 0;
> + int ret, i;
> struct ccw1 *ccw;
> dma32_t indicatorp_dma = 0;
>
> @@ -710,7 +710,7 @@ static int virtio_ccw_find_vqs(struct virtio_device *vdev, unsigned nvqs,
> continue;
> }
>
> - vqs[i] = virtio_ccw_setup_vq(vdev, queue_idx++, vqi->callback,
> + vqs[i] = virtio_ccw_setup_vq(vdev, i, vqi->callback,
> vqi->name, vqi->ctx, ccw);
> if (IS_ERR(vqs[i])) {
> ret = PTR_ERR(vqs[i]);
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 90e784e7b721..db6a0366f082 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -494,7 +494,7 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
> {
> struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
> int irq = platform_get_irq(vm_dev->pdev, 0);
> - int i, err, queue_idx = 0;
> + int i, err;
>
> if (irq < 0)
> return irq;
> @@ -515,7 +515,7 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
> continue;
> }
>
> - vqs[i] = vm_setup_vq(vdev, queue_idx++, vqi->callback,
> + vqs[i] = vm_setup_vq(vdev, i, vqi->callback,
> vqi->name, vqi->ctx);
> if (IS_ERR(vqs[i])) {
> vm_del_vqs(vdev);
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 7d82facafd75..fa606e7321ad 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -293,7 +293,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs,
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> struct virtqueue_info *vqi;
> u16 msix_vec;
> - int i, err, nvectors, allocated_vectors, queue_idx = 0;
> + int i, err, nvectors, allocated_vectors;
>
> vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL);
> if (!vp_dev->vqs)
> @@ -332,7 +332,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs,
> msix_vec = allocated_vectors++;
> else
> msix_vec = VP_MSIX_VQ_VECTOR;
> - vqs[i] = vp_setup_vq(vdev, queue_idx++, vqi->callback,
> + vqs[i] = vp_setup_vq(vdev, i, vqi->callback,
> vqi->name, vqi->ctx, msix_vec);
> if (IS_ERR(vqs[i])) {
> err = PTR_ERR(vqs[i]);
> @@ -368,7 +368,7 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs,
> struct virtqueue_info vqs_info[])
> {
> struct virtio_pci_device *vp_dev = to_vp_device(vdev);
> - int i, err, queue_idx = 0;
> + int i, err;
>
> vp_dev->vqs = kcalloc(nvqs, sizeof(*vp_dev->vqs), GFP_KERNEL);
> if (!vp_dev->vqs)
> @@ -388,8 +388,13 @@ static int vp_find_vqs_intx(struct virtio_device *vdev, unsigned int nvqs,
> vqs[i] = NULL;
> continue;
> }
> +<<<<<<< HEAD
> vqs[i] = vp_setup_vq(vdev, queue_idx++, vqi->callback,
> vqi->name, vqi->ctx,
> +=======
> + vqs[i] = vp_setup_vq(vdev, i, callbacks[i], names[i],
> + ctx ? ctx[i] : false,
> +>>>>>>> f814759f80b7... virtio: fix vq # for balloon
> VIRTIO_MSI_NO_VECTOR);
> if (IS_ERR(vqs[i])) {
> err = PTR_ERR(vqs[i]);
> diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> index 7364bd53e38d..149e893583e9 100644
> --- a/drivers/virtio/virtio_vdpa.c
> +++ b/drivers/virtio/virtio_vdpa.c
> @@ -368,7 +368,7 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
> struct cpumask *masks;
> struct vdpa_callback cb;
> bool has_affinity = desc && ops->set_vq_affinity;
> - int i, err, queue_idx = 0;
> + int i, err;
>
> if (has_affinity) {
> masks = create_affinity_masks(nvqs, desc ? desc : &default_affd);
> @@ -384,7 +384,7 @@ static int virtio_vdpa_find_vqs(struct virtio_device *vdev, unsigned int nvqs,
> continue;
> }
>
> - vqs[i] = virtio_vdpa_setup_vq(vdev, queue_idx++, vqi->callback,
> + vqs[i] = virtio_vdpa_setup_vq(vdev, i, vqi->callback,
> vqi->name, vqi->ctx);
> if (IS_ERR(vqs[i])) {
> err = PTR_ERR(vqs[i]);
> --
> MST
>