| Message ID | 8de2d4a6407d796d4d793975fc88e2f929f6025d.1720128585.git.mst@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] virtio-balloon: make it spec compliant | expand |
On 04.07.24 23:30, Michael S. Tsirkin wrote: > Currently, if VIRTIO_BALLOON_F_FREE_PAGE_HINT is off but > VIRTIO_BALLOON_F_REPORTING is on, then the reporting vq > gets number 3 while spec says it's number 4. > It happens to work because the linux virtio pci driver > is *also* out of spec. > > To fix: > 1. add vq4 as per spec > 2. to help out the buggy Linux driver, in the above configuration, > also create vq3, and handle it exactly as we do vq4. > > I think that some clever hack is doable to address the issue > for existing machine types (which would get it in user's hands > sooner), but I'm not 100% sure what, exactly. > > This is a simpler, straight-forward approach. > > Reported-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > > I don't think I'll stop here, I want to fix exiting machine types, > but sending this here for comparison. > I'll send a Linux patch later. The downside is that new machine types will stop working with mainline Linux / major distros in that feature combination, right? What's the approach that you are thinking of?
On Wed, Jul 10, 2024 at 05:15:18AM +0200, David Hildenbrand wrote: > On 04.07.24 23:30, Michael S. Tsirkin wrote: > > Currently, if VIRTIO_BALLOON_F_FREE_PAGE_HINT is off but > > VIRTIO_BALLOON_F_REPORTING is on, then the reporting vq > > gets number 3 while spec says it's number 4. > > It happens to work because the linux virtio pci driver > > is *also* out of spec. > > > > To fix: > > 1. add vq4 as per spec > > 2. to help out the buggy Linux driver, in the above configuration, > > also create vq3, and handle it exactly as we do vq4. > > > > I think that some clever hack is doable to address the issue > > for existing machine types (which would get it in user's hands > > sooner), but I'm not 100% sure what, exactly. > > > > This is a simpler, straight-forward approach. > > > > Reported-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > --- > > > > I don't think I'll stop here, I want to fix exiting machine types, > > but sending this here for comparison. > > I'll send a Linux patch later. > > The downside is that new machine types will stop working with mainline Linux > / major distros in that feature combination, right? The should keep working, that's why I put in part 2. above. > What's the approach that you are thinking of? > > -- > Cheers, > > David / dhildenb
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h index 5139cf8ab6..d4426404ed 100644 --- a/include/hw/virtio/virtio-balloon.h +++ b/include/hw/virtio/virtio-balloon.h @@ -70,6 +70,7 @@ struct VirtIOBalloon { uint32_t host_features; bool qemu_4_0_config_size; + bool reporting_vq_is_vq3; uint32_t poison_val; }; diff --git a/hw/core/machine.c b/hw/core/machine.c index f4cba6496c..353a143b2b 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -39,6 +39,7 @@ GlobalProperty hw_compat_9_0[] = { {"scsi-disk-base", "migrate-emulated-scsi-request", "false" }, {"vfio-pci", "skip-vsc-check", "false" }, { "virtio-pci", "x-pcie-pm-no-soft-reset", "off" }, + { "virtio-balloon-device", "x-reporting-vq-is-vq3", "true" }, }; const size_t hw_compat_9_0_len = G_N_ELEMENTS(hw_compat_9_0); diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 50da34d6cc..4712bee521 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -892,7 +892,8 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp) if (virtio_has_feature(s->host_features, VIRTIO_BALLOON_F_REPORTING)) { /* Work around Linux driver which is buggy in this configuration */ - if (!virtio_has_feature(s->host_features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) { + if (!s->reporting_vq_is_vq3 && + !virtio_has_feature(s->host_features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) { s->free_page_vq = virtio_add_queue(vdev, 32, virtio_balloon_handle_report); } @@ -1021,6 +1022,8 @@ static Property virtio_balloon_properties[] = { */ DEFINE_PROP_BOOL("qemu-4-0-config-size", VirtIOBalloon, qemu_4_0_config_size, false), + DEFINE_PROP_BOOL("x-reporting-vq-is-vq3", VirtIOBalloon, + reporting_vq_is_vq3, false), DEFINE_PROP_LINK("iothread", VirtIOBalloon, iothread, TYPE_IOTHREAD, IOThread *), DEFINE_PROP_END_OF_LIST(),
Currently, if VIRTIO_BALLOON_F_FREE_PAGE_HINT is off but VIRTIO_BALLOON_F_REPORTING is on, then the reporting vq gets number 3 while spec says it's number 4. It happens to work because the linux virtio pci driver is *also* out of spec. To fix: 1. add vq4 as per spec 2. to help out the buggy Linux driver, in the above configuration, also create vq3, and handle it exactly as we do vq4. I think that some clever hack is doable to address the issue for existing machine types (which would get it in user's hands sooner), but I'm not 100% sure what, exactly. This is a simpler, straight-forward approach. Reported-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- I don't think I'll stop here, I want to fix exiting machine types, but sending this here for comparison. I'll send a Linux patch later. include/hw/virtio/virtio-balloon.h | 1 + hw/core/machine.c | 1 + hw/virtio/virtio-balloon.c | 5 ++++- 3 files changed, 6 insertions(+), 1 deletion(-)