[PATCH] virtio-balloon-pci: Allow setting nvectors, so we can use MSI-X

[PATCH] virtio-balloon-pci: Allow setting nvectors, so we can use MSI-X

[Reza Arbab]

Most virtio-pci devices allow MSI-X. Add it to virtio-balloon-pci, but
only enable it in new machine types, so we don't break migration of
existing machine types between different qemu versions.

This copies what was done for virtio-rng-pci in:
9ea02e8f1306 ("virtio-rng-pci: Allow setting nvectors, so we can use MSI-X")
bad9c5a5166f ("virtio-rng-pci: fix migration compat for vectors")
62bdb8871512 ("virtio-rng-pci: fix transitional migration compat for vectors")

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 hw/core/machine.c              |  6 +++++-
 hw/virtio/virtio-balloon-pci.c | 13 +++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index e6900b43efa2..db59c7771ec4 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -36,7 +36,11 @@
 #include "hw/virtio/virtio-iommu.h"
 #include "audio/audio.h"
 
-GlobalProperty hw_compat_9_2[] = {};
+GlobalProperty hw_compat_9_2[] = {
+    { "virtio-balloon-pci", "vectors", "0" },
+    { "virtio-balloon-pci-transitional", "vectors", "0" },
+    { "virtio-balloon-pci-non-transitional", "vectors", "0" },
+};
 const size_t hw_compat_9_2_len = G_N_ELEMENTS(hw_compat_9_2);
 
 GlobalProperty hw_compat_9_1[] = {
diff --git a/hw/virtio/virtio-balloon-pci.c b/hw/virtio/virtio-balloon-pci.c
index ce2645ba7187..1c2b071eff0c 100644
--- a/hw/virtio/virtio-balloon-pci.c
+++ b/hw/virtio/virtio-balloon-pci.c
@@ -35,11 +35,23 @@ struct VirtIOBalloonPCI {
     VirtIOBalloon vdev;
 };
 
+static Property virtio_balloon_properties[] = {
+    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
+                       DEV_NVECTORS_UNSPECIFIED),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void virtio_balloon_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
 {
     VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(vpci_dev);
     DeviceState *vdev = DEVICE(&dev->vdev);
 
+    if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
+        vpci_dev->nvectors = 2;
+    }
+
     vpci_dev->class_code = PCI_CLASS_OTHERS;
     qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
 }
@@ -55,6 +67,7 @@ static void virtio_balloon_pci_class_init(ObjectClass *klass, void *data)
     pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_BALLOON;
     pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
     pcidev_k->class_id = PCI_CLASS_OTHERS;
+    device_class_set_props(dc, virtio_balloon_properties);
 }
 
 static void virtio_balloon_pci_instance_init(Object *obj)

base-commit: ca80a5d026a280762e0772615f1988db542b3ade
-- 
2.47.1

Re: [PATCH] virtio-balloon-pci: Allow setting nvectors, so we can use MSI-X

[David Hildenbrand]

On 16.12.24 17:31, Reza Arbab wrote:
> Most virtio-pci devices allow MSI-X. Add it to virtio-balloon-pci, but
> only enable it in new machine types, so we don't break migration of
> existing machine types between different qemu versions.
> 
> This copies what was done for virtio-rng-pci in:
> 9ea02e8f1306 ("virtio-rng-pci: Allow setting nvectors, so we can use MSI-X")
> bad9c5a5166f ("virtio-rng-pci: fix migration compat for vectors")
> 62bdb8871512 ("virtio-rng-pci: fix transitional migration compat for vectors")
> 
> Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
> ---
>   hw/core/machine.c              |  6 +++++-
>   hw/virtio/virtio-balloon-pci.c | 13 +++++++++++++
>   2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index e6900b43efa2..db59c7771ec4 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -36,7 +36,11 @@
>   #include "hw/virtio/virtio-iommu.h"
>   #include "audio/audio.h"
>   
> -GlobalProperty hw_compat_9_2[] = {};
> +GlobalProperty hw_compat_9_2[] = {
> +    { "virtio-balloon-pci", "vectors", "0" },
> +    { "virtio-balloon-pci-transitional", "vectors", "0" },
> +    { "virtio-balloon-pci-non-transitional", "vectors", "0" },
> +};
>   const size_t hw_compat_9_2_len = G_N_ELEMENTS(hw_compat_9_2);
>   
>   GlobalProperty hw_compat_9_1[] = {
> diff --git a/hw/virtio/virtio-balloon-pci.c b/hw/virtio/virtio-balloon-pci.c
> index ce2645ba7187..1c2b071eff0c 100644
> --- a/hw/virtio/virtio-balloon-pci.c
> +++ b/hw/virtio/virtio-balloon-pci.c
> @@ -35,11 +35,23 @@ struct VirtIOBalloonPCI {
>       VirtIOBalloon vdev;
>   };
>   
> +static Property virtio_balloon_properties[] = {
> +    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
> +                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
> +    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
> +                       DEV_NVECTORS_UNSPECIFIED),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>   static void virtio_balloon_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>   {
>       VirtIOBalloonPCI *dev = VIRTIO_BALLOON_PCI(vpci_dev);
>       DeviceState *vdev = DEVICE(&dev->vdev);
>   
> +    if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> +        vpci_dev->nvectors = 2;
> +    }
> +
>       vpci_dev->class_code = PCI_CLASS_OTHERS;
>       qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
>   }
> @@ -55,6 +67,7 @@ static void virtio_balloon_pci_class_init(ObjectClass *klass, void *data)
>       pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_BALLOON;
>       pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
>       pcidev_k->class_id = PCI_CLASS_OTHERS;
> +    device_class_set_props(dc, virtio_balloon_properties);
>   }
>   
>   static void virtio_balloon_pci_instance_init(Object *obj)
> 
> base-commit: ca80a5d026a280762e0772615f1988db542b3ade

No expert on any of that MSi-X / PCI magic, but LGTM

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

-- 
Cheers,

David / dhildenb

Re: [PATCH] virtio-balloon-pci: Allow setting nvectors, so we can use MSI-X

[Reza Arbab]

On Wed, Dec 18, 2024 at 11:37:05AM +0100, David Hildenbrand wrote:
>No expert on any of that MSi-X / PCI magic, but LGTM
>
>Acked-by: David Hildenbrand <david@redhat.com>

Thanks David!

Did anyone else have any comments? Just want to make sure I have things 
in order to get this into the next release.

-- 
Reza Arbab

Re: [PATCH] virtio-balloon-pci: Allow setting nvectors, so we can use MSI-X

[Michael S. Tsirkin]

On Wed, Jan 08, 2025 at 09:14:35AM -0600, Reza Arbab wrote:
> On Wed, Dec 18, 2024 at 11:37:05AM +0100, David Hildenbrand wrote:
> > No expert on any of that MSi-X / PCI magic, but LGTM
> > 
> > Acked-by: David Hildenbrand <david@redhat.com>
> 
> Thanks David!
> 
> Did anyone else have any comments? Just want to make sure I have things in
> order to get this into the next release.

Causes a CI failure:

https://gitlab.com/mstredhat/qemu/-/jobs/8800804175

In file included from ../hw/virtio/virtio-balloon-pci.c:15:
../hw/virtio/virtio-balloon-pci.c: In function ‘virtio_balloon_pci_class_init’:
../include/qemu/osdep.h:261:35: error: call to ‘qemu_build_not_reached_always’ declared with attribute error: code path is reachable
  261 | #define qemu_build_not_reached()  qemu_build_not_reached_always()
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/hw/qdev-core.h:956:13: note: in expansion of macro ‘qemu_build_not_reached’
  956 |             qemu_build_not_reached();                                   \
      |             ^~~~~~~~~~~~~~~~~~~~~~
../hw/virtio/virtio-balloon-pci.c:69:5: note: in expansion of macro ‘device_class_set_props’
   69 |     device_class_set_props(dc, virtio_balloon_properties);
      |     ^~~~~~~~~~~~~~~~~~~~~~
[2493/4296] Compiling C object libqemu-ppc64-softmmu.a.p/hw_virtio_virtio-scsi-pci.c.o
[2494/4296] Compiling C object libqemu-ppc64-softmmu.a.p/hw_virtio_virtio-9p-pci.c.o



> -- 
> Reza Arbab

Re: [PATCH] virtio-balloon-pci: Allow setting nvectors, so we can use MSI-X

[Reza Arbab]

On Wed, Jan 15, 2025 at 03:54:27AM -0500, Michael S. Tsirkin wrote:
>In file included from ../hw/virtio/virtio-balloon-pci.c:15:
>../hw/virtio/virtio-balloon-pci.c: In function ‘virtio_balloon_pci_class_init’:
>../include/qemu/osdep.h:261:35: error: call to ‘qemu_build_not_reached_always’ declared with attribute error: code path is reachable
>  261 | #define qemu_build_not_reached()  qemu_build_not_reached_always()
>      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>../include/hw/qdev-core.h:956:13: note: in expansion of macro ‘qemu_build_not_reached’
>  956 |             qemu_build_not_reached();                                   \
>      |             ^~~~~~~~~~~~~~~~~~~~~~
>../hw/virtio/virtio-balloon-pci.c:69:5: note: in expansion of macro ‘device_class_set_props’
>   69 |     device_class_set_props(dc, virtio_balloon_properties);
>      |     ^~~~~~~~~~~~~~~~~~~~~~

Ah, I need to declare virtio_balloon_properties as const.

-- 
Reza Arbab

Re: [PATCH] virtio-balloon-pci: Allow setting nvectors, so we can use MSI-X

[Philippe Mathieu-Daudé]

Hi Reza,

On 16/12/24 17:31, Reza Arbab wrote:
> Most virtio-pci devices allow MSI-X. Add it to virtio-balloon-pci, but
> only enable it in new machine types, so we don't break migration of
> existing machine types between different qemu versions.
> 
> This copies what was done for virtio-rng-pci in:
> 9ea02e8f1306 ("virtio-rng-pci: Allow setting nvectors, so we can use MSI-X")
> bad9c5a5166f ("virtio-rng-pci: fix migration compat for vectors")
> 62bdb8871512 ("virtio-rng-pci: fix transitional migration compat for vectors")
> 
> Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
> ---
>   hw/core/machine.c              |  6 +++++-
>   hw/virtio/virtio-balloon-pci.c | 13 +++++++++++++
>   2 files changed, 18 insertions(+), 1 deletion(-)


> diff --git a/hw/virtio/virtio-balloon-pci.c b/hw/virtio/virtio-balloon-pci.c
> index ce2645ba7187..1c2b071eff0c 100644
> --- a/hw/virtio/virtio-balloon-pci.c
> +++ b/hw/virtio/virtio-balloon-pci.c
> @@ -35,11 +35,23 @@ struct VirtIOBalloonPCI {
>       VirtIOBalloon vdev;
>   };
>   
> +static Property virtio_balloon_properties[] = {
> +    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
> +                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
> +    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
> +                       DEV_NVECTORS_UNSPECIFIED),
> +    DEFINE_PROP_END_OF_LIST(),

You shouldn't use that anymore since commit 5fcabe628b8
("include/hw/qdev-properties: Remove DEFINE_PROP_END_OF_LIST").

Also, this array can be declared const.

> +};

Re: [PATCH] virtio-balloon-pci: Allow setting nvectors, so we can use MSI-X

[Reza Arbab]

On Wed, Jan 15, 2025 at 10:06:26AM +0100, Philippe Mathieu-Daudé wrote:
>>--- a/hw/virtio/virtio-balloon-pci.c
>>+++ b/hw/virtio/virtio-balloon-pci.c
>>@@ -35,11 +35,23 @@ struct VirtIOBalloonPCI {
>>      VirtIOBalloon vdev;
>>  };
>>+static Property virtio_balloon_properties[] = {
>>+    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
>>+                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
>>+    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
>>+                       DEV_NVECTORS_UNSPECIFIED),
>>+    DEFINE_PROP_END_OF_LIST(),
>
>You shouldn't use that anymore since commit 5fcabe628b8
>("include/hw/qdev-properties: Remove DEFINE_PROP_END_OF_LIST").

Ah, yes, that happened after I sent this.

>Also, this array can be declared const.

Funny, commit 1577a9180f37 ("hw/virtio: Constify all Property") also 
happened not long after I sent.

Thanks for your review! v2 incoming.

-- 
Reza Arbab