1. Patchew
  2. linux
  3. [PATCH 0/2] virtio-balloon: make it spec compliant
  4. View patch

[PATCH 1/2] virtio_balloon: add work around for out of spec QEMU

Michael S. Tsirkin posted 2 patches 1 year, 7 months ago
There is a newer version of this series
[PATCH 1/2] virtio_balloon: add work around for out of spec QEMU
Posted by Michael S. Tsirkin 1 year, 7 months ago 
QEMU implemented the configuration
	VIRTIO_BALLOON_F_REPORTING && ! VIRTIO_BALLOON_F_FREE_PAGE_HINT
incorrectly: it then uses vq3 for reporting, spec says it is always 4.

This is masked by a corresponding bug in driver:
add a work around as I'm going to try and fix the driver bug.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_balloon.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 9a61febbd2f7..7dc3fcd56238 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -597,8 +597,23 @@ static int init_vqs(struct virtio_balloon *vb)
 
 	err = virtio_find_vqs(vb->vdev, VIRTIO_BALLOON_VQ_MAX, vqs,
 			      callbacks, names, NULL);
-	if (err)
-		return err;
+	if (err) {
+		/*
+		 * Try to work around QEMU bug which since 2020 confused vq numbers
+		 * when VIRTIO_BALLOON_F_REPORTING but not
+		 * VIRTIO_BALLOON_F_FREE_PAGE_HINT are offered.
+		 */
+		if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING) &&
+		    !virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
+			names[VIRTIO_BALLOON_VQ_FREE_PAGE] = "reporting_vq";
+			callbacks[VIRTIO_BALLOON_VQ_FREE_PAGE] = balloon_ack;
+			err = virtio_find_vqs(vb->vdev,
+					      VIRTIO_BALLOON_VQ_REPORTING, vqs, callbacks, names, NULL);
+		}
+
+		if (err)
+			return err;
+	}
 
 	vb->inflate_vq = vqs[VIRTIO_BALLOON_VQ_INFLATE];
 	vb->deflate_vq = vqs[VIRTIO_BALLOON_VQ_DEFLATE];
-- 
MST
Re: [PATCH 1/2] virtio_balloon: add work around for out of spec QEMU
Posted by Jason Wang 1 year, 7 months ago 
On Fri, Jul 5, 2024 at 6:09 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> QEMU implemented the configuration
>         VIRTIO_BALLOON_F_REPORTING && ! VIRTIO_BALLOON_F_FREE_PAGE_HINT
> incorrectly: it then uses vq3 for reporting, spec says it is always 4.
>
> This is masked by a corresponding bug in driver:
> add a work around as I'm going to try and fix the driver bug.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  drivers/virtio/virtio_balloon.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 9a61febbd2f7..7dc3fcd56238 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -597,8 +597,23 @@ static int init_vqs(struct virtio_balloon *vb)
>
>         err = virtio_find_vqs(vb->vdev, VIRTIO_BALLOON_VQ_MAX, vqs,
>                               callbacks, names, NULL);
> -       if (err)
> -               return err;
> +       if (err) {
> +               /*
> +                * Try to work around QEMU bug which since 2020 confused vq numbers
> +                * when VIRTIO_BALLOON_F_REPORTING but not
> +                * VIRTIO_BALLOON_F_FREE_PAGE_HINT are offered.
> +                */
> +               if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING) &&
> +                   !virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
> +                       names[VIRTIO_BALLOON_VQ_FREE_PAGE] = "reporting_vq";
> +                       callbacks[VIRTIO_BALLOON_VQ_FREE_PAGE] = balloon_ack;
> +                       err = virtio_find_vqs(vb->vdev,
> +                                             VIRTIO_BALLOON_VQ_REPORTING, vqs, callbacks, names, NULL);
> +               }
> +
> +               if (err)
> +                       return err;
> +       }
>
>         vb->inflate_vq = vqs[VIRTIO_BALLOON_VQ_INFLATE];
>         vb->deflate_vq = vqs[VIRTIO_BALLOON_VQ_DEFLATE];
> --
> MST
>

Acked-by: Jason Wang <jasowang@redhat.com>

Do we need a spec to say this is something that needs to be considered
by the driver?

Thanks
Re: [PATCH 1/2] virtio_balloon: add work around for out of spec QEMU
Posted by Michael S. Tsirkin 1 year, 7 months ago 
On Wed, Jul 10, 2024 at 11:23:20AM +0800, Jason Wang wrote:
> On Fri, Jul 5, 2024 at 6:09 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > QEMU implemented the configuration
> >         VIRTIO_BALLOON_F_REPORTING && ! VIRTIO_BALLOON_F_FREE_PAGE_HINT
> > incorrectly: it then uses vq3 for reporting, spec says it is always 4.
> >
> > This is masked by a corresponding bug in driver:
> > add a work around as I'm going to try and fix the driver bug.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  drivers/virtio/virtio_balloon.c | 19 +++++++++++++++++--
> >  1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > index 9a61febbd2f7..7dc3fcd56238 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -597,8 +597,23 @@ static int init_vqs(struct virtio_balloon *vb)
> >
> >         err = virtio_find_vqs(vb->vdev, VIRTIO_BALLOON_VQ_MAX, vqs,
> >                               callbacks, names, NULL);
> > -       if (err)
> > -               return err;
> > +       if (err) {
> > +               /*
> > +                * Try to work around QEMU bug which since 2020 confused vq numbers
> > +                * when VIRTIO_BALLOON_F_REPORTING but not
> > +                * VIRTIO_BALLOON_F_FREE_PAGE_HINT are offered.
> > +                */
> > +               if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING) &&
> > +                   !virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
> > +                       names[VIRTIO_BALLOON_VQ_FREE_PAGE] = "reporting_vq";
> > +                       callbacks[VIRTIO_BALLOON_VQ_FREE_PAGE] = balloon_ack;
> > +                       err = virtio_find_vqs(vb->vdev,
> > +                                             VIRTIO_BALLOON_VQ_REPORTING, vqs, callbacks, names, NULL);
> > +               }
> > +
> > +               if (err)
> > +                       return err;
> > +       }
> >
> >         vb->inflate_vq = vqs[VIRTIO_BALLOON_VQ_INFLATE];
> >         vb->deflate_vq = vqs[VIRTIO_BALLOON_VQ_DEFLATE];
> > --
> > MST
> >
> 
> Acked-by: Jason Wang <jasowang@redhat.com>
> 
> Do we need a spec to say this is something that needs to be considered
> by the driver?
> 
> Thanks

I'd say it's a temporary situation that we won't need to bother
about in several years.

-- 
MST
Re: [PATCH 1/2] virtio_balloon: add work around for out of spec QEMU
Posted by Jason Wang 1 year, 7 months ago 
On Wed, Jul 10, 2024 at 2:16 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jul 10, 2024 at 11:23:20AM +0800, Jason Wang wrote:
> > On Fri, Jul 5, 2024 at 6:09 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > QEMU implemented the configuration
> > >         VIRTIO_BALLOON_F_REPORTING && ! VIRTIO_BALLOON_F_FREE_PAGE_HINT
> > > incorrectly: it then uses vq3 for reporting, spec says it is always 4.
> > >
> > > This is masked by a corresponding bug in driver:
> > > add a work around as I'm going to try and fix the driver bug.
> > >
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >  drivers/virtio/virtio_balloon.c | 19 +++++++++++++++++--
> > >  1 file changed, 17 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > > index 9a61febbd2f7..7dc3fcd56238 100644
> > > --- a/drivers/virtio/virtio_balloon.c
> > > +++ b/drivers/virtio/virtio_balloon.c
> > > @@ -597,8 +597,23 @@ static int init_vqs(struct virtio_balloon *vb)
> > >
> > >         err = virtio_find_vqs(vb->vdev, VIRTIO_BALLOON_VQ_MAX, vqs,
> > >                               callbacks, names, NULL);
> > > -       if (err)
> > > -               return err;
> > > +       if (err) {
> > > +               /*
> > > +                * Try to work around QEMU bug which since 2020 confused vq numbers
> > > +                * when VIRTIO_BALLOON_F_REPORTING but not
> > > +                * VIRTIO_BALLOON_F_FREE_PAGE_HINT are offered.
> > > +                */
> > > +               if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING) &&
> > > +                   !virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
> > > +                       names[VIRTIO_BALLOON_VQ_FREE_PAGE] = "reporting_vq";
> > > +                       callbacks[VIRTIO_BALLOON_VQ_FREE_PAGE] = balloon_ack;
> > > +                       err = virtio_find_vqs(vb->vdev,
> > > +                                             VIRTIO_BALLOON_VQ_REPORTING, vqs, callbacks, names, NULL);
> > > +               }
> > > +
> > > +               if (err)
> > > +                       return err;
> > > +       }
> > >
> > >         vb->inflate_vq = vqs[VIRTIO_BALLOON_VQ_INFLATE];
> > >         vb->deflate_vq = vqs[VIRTIO_BALLOON_VQ_DEFLATE];
> > > --
> > > MST
> > >
> >
> > Acked-by: Jason Wang <jasowang@redhat.com>
> >
> > Do we need a spec to say this is something that needs to be considered
> > by the driver?
> >
> > Thanks
>
> I'd say it's a temporary situation that we won't need to bother
> about in several years.

I mean, should a newly-written virtio-balloon driver care about this?
If not, it means it can't work for several Qemu versions.

Thanks

>
> --
> MST
>
Re: [PATCH 1/2] virtio_balloon: add work around for out of spec QEMU
Posted by Michael S. Tsirkin 1 year, 7 months ago 
On Wed, Jul 10, 2024 at 03:37:49PM +0800, Jason Wang wrote:
> On Wed, Jul 10, 2024 at 2:16 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Wed, Jul 10, 2024 at 11:23:20AM +0800, Jason Wang wrote:
> > > On Fri, Jul 5, 2024 at 6:09 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> > > >
> > > > QEMU implemented the configuration
> > > >         VIRTIO_BALLOON_F_REPORTING && ! VIRTIO_BALLOON_F_FREE_PAGE_HINT
> > > > incorrectly: it then uses vq3 for reporting, spec says it is always 4.
> > > >
> > > > This is masked by a corresponding bug in driver:
> > > > add a work around as I'm going to try and fix the driver bug.
> > > >
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > >  drivers/virtio/virtio_balloon.c | 19 +++++++++++++++++--
> > > >  1 file changed, 17 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > > > index 9a61febbd2f7..7dc3fcd56238 100644
> > > > --- a/drivers/virtio/virtio_balloon.c
> > > > +++ b/drivers/virtio/virtio_balloon.c
> > > > @@ -597,8 +597,23 @@ static int init_vqs(struct virtio_balloon *vb)
> > > >
> > > >         err = virtio_find_vqs(vb->vdev, VIRTIO_BALLOON_VQ_MAX, vqs,
> > > >                               callbacks, names, NULL);
> > > > -       if (err)
> > > > -               return err;
> > > > +       if (err) {
> > > > +               /*
> > > > +                * Try to work around QEMU bug which since 2020 confused vq numbers
> > > > +                * when VIRTIO_BALLOON_F_REPORTING but not
> > > > +                * VIRTIO_BALLOON_F_FREE_PAGE_HINT are offered.
> > > > +                */
> > > > +               if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING) &&
> > > > +                   !virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
> > > > +                       names[VIRTIO_BALLOON_VQ_FREE_PAGE] = "reporting_vq";
> > > > +                       callbacks[VIRTIO_BALLOON_VQ_FREE_PAGE] = balloon_ack;
> > > > +                       err = virtio_find_vqs(vb->vdev,
> > > > +                                             VIRTIO_BALLOON_VQ_REPORTING, vqs, callbacks, names, NULL);
> > > > +               }
> > > > +
> > > > +               if (err)
> > > > +                       return err;
> > > > +       }
> > > >
> > > >         vb->inflate_vq = vqs[VIRTIO_BALLOON_VQ_INFLATE];
> > > >         vb->deflate_vq = vqs[VIRTIO_BALLOON_VQ_DEFLATE];
> > > > --
> > > > MST
> > > >
> > >
> > > Acked-by: Jason Wang <jasowang@redhat.com>
> > >
> > > Do we need a spec to say this is something that needs to be considered
> > > by the driver?
> > >
> > > Thanks
> >
> > I'd say it's a temporary situation that we won't need to bother
> > about in several years.
> 
> I mean, should a newly-written virtio-balloon driver care about this?
> If not, it means it can't work for several Qemu versions.
> 
> Thanks

True - I could not find a way to make it work in a way that
would be compatible with old qemu.


> >
> > --
> > MST
> >
Re: [PATCH 1/2] virtio_balloon: add work around for out of spec QEMU
Posted by David Hildenbrand 1 year, 7 months ago 
On 05.07.24 12:08, Michael S. Tsirkin wrote:
> QEMU implemented the configuration
> 	VIRTIO_BALLOON_F_REPORTING && ! VIRTIO_BALLOON_F_FREE_PAGE_HINT
> incorrectly: it then uses vq3 for reporting, spec says it is always 4.
> 
> This is masked by a corresponding bug in driver:
> add a work around as I'm going to try and fix the driver bug.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>   drivers/virtio/virtio_balloon.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 9a61febbd2f7..7dc3fcd56238 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -597,8 +597,23 @@ static int init_vqs(struct virtio_balloon *vb)
>   
>   	err = virtio_find_vqs(vb->vdev, VIRTIO_BALLOON_VQ_MAX, vqs,
>   			      callbacks, names, NULL);
> -	if (err)
> -		return err;
> +	if (err) {
> +		/*
> +		 * Try to work around QEMU bug which since 2020 confused vq numbers
> +		 * when VIRTIO_BALLOON_F_REPORTING but not
> +		 * VIRTIO_BALLOON_F_FREE_PAGE_HINT are offered.
> +		 */
> +		if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING) &&
> +		    !virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
> +			names[VIRTIO_BALLOON_VQ_FREE_PAGE] = "reporting_vq";
> +			callbacks[VIRTIO_BALLOON_VQ_FREE_PAGE] = balloon_ack;
> +			err = virtio_find_vqs(vb->vdev,
> +					      VIRTIO_BALLOON_VQ_REPORTING, vqs, callbacks, names, NULL);
> +		}
> +
> +		if (err)
> +			return err;
> +	}
>   
>   	vb->inflate_vq = vqs[VIRTIO_BALLOON_VQ_INFLATE];
>   	vb->deflate_vq = vqs[VIRTIO_BALLOON_VQ_DEFLATE];

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.