[PATCH v2 0/2] s390x: support virtio-mem-pci

[PATCH v2 0/2] s390x: support virtio-mem-pci

[David Hildenbrand]

This is based-on [1], which adds MSI-X support to virtio-balloon-pci,
but can be applied independently.

Turns out it is fairly easy to get virtio-mem-pci running on s390x. We
only have to add MSI-X support to virtio-mem-pci, and wire-up the
(un)plugging in the machine.

Tried some simple stuff (hotplug/hotunplug/resize/reboot), and all seems
to be working as expected.

The kernel in the VM needs both, CONFIG_VIRTIO_PCI and CONFIG_VIRTIO_MEM
for it to work.

[1] https://lkml.kernel.org/r/20250115161425.246348-1-arbab@linux.ibm.com

v1 -> v2:
* There are no transitional/non_transitional devices for virtio-mem
* Spell out removal of "return;" in second patch

Cc: Eduardo Habkost <eduardo@habkost.net>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Richard Henderson <richard.henderson@linaro.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Ilya Leoshkevich <iii@linux.ibm.com>
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Eric Farman <farman@linux.ibm.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Boris Fiuczynski <fiuczy@linux.ibm.com>
Cc: Michal Privoznik <mprivozn@redhat.com>
Cc: Mario Casquero <mcasquer@redhat.com>

David Hildenbrand (2):
  virtio-mem-pci: Allow setting nvectors, so we can use MSI-X
  s390x/s390-virtio-ccw: Support plugging PCI-based virtio memory
    devices

 hw/core/machine.c          |  1 +
 hw/s390x/s390-virtio-ccw.c | 20 ++++++++++++++------
 hw/virtio/virtio-mem-pci.c | 12 ++++++++++++
 3 files changed, 27 insertions(+), 6 deletions(-)

-- 
2.48.1

[PATCH v2 1/2] virtio-mem-pci: Allow setting nvectors, so we can use MSI-X

[David Hildenbrand]

Let's do it similar as virtio-balloon-pci. With this change, we can
use virtio-mem-pci on s390x, although plugging will still fail until
properly wired up in the machine.

No need to worry about transitional/non_transitional devices, because they
don't exist for virtio-mem.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/core/machine.c          |  1 +
 hw/virtio/virtio-mem-pci.c | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 8f396ef803..7b74cde10a 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -41,6 +41,7 @@ GlobalProperty hw_compat_9_2[] = {
     { "virtio-balloon-pci", "vectors", "0" },
     { "virtio-balloon-pci-transitional", "vectors", "0" },
     { "virtio-balloon-pci-non-transitional", "vectors", "0" },
+    { "virtio-mem-pci", "vectors", "0" },
 };
 const size_t hw_compat_9_2_len = G_N_ELEMENTS(hw_compat_9_2);
 
diff --git a/hw/virtio/virtio-mem-pci.c b/hw/virtio/virtio-mem-pci.c
index 1b4e9a3284..6cc5f0fd3b 100644
--- a/hw/virtio/virtio-mem-pci.c
+++ b/hw/virtio/virtio-mem-pci.c
@@ -22,6 +22,10 @@ static void virtio_mem_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
     VirtIOMEMPCI *mem_pci = VIRTIO_MEM_PCI(vpci_dev);
     DeviceState *vdev = DEVICE(&mem_pci->vdev);
 
+    if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
+        vpci_dev->nvectors = 2;
+    }
+
     virtio_pci_force_virtio_1(vpci_dev);
     qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
 }
@@ -152,6 +156,13 @@ static void virtio_mem_pci_set_requested_size(Object *obj, Visitor *v,
     object_property_set(OBJECT(&pci_mem->vdev), name, v, errp);
 }
 
+static const Property virtio_mem_pci_class_properties[] = {
+    DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
+    DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
+                       DEV_NVECTORS_UNSPECIFIED),
+};
+
 static void virtio_mem_pci_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
@@ -164,6 +175,7 @@ static void virtio_mem_pci_class_init(ObjectClass *klass, void *data)
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
     pcidev_k->revision = VIRTIO_PCI_ABI_VERSION;
     pcidev_k->class_id = PCI_CLASS_OTHERS;
+    device_class_set_props(dc, virtio_mem_pci_class_properties);
 
     mdc->get_addr = virtio_mem_pci_get_addr;
     mdc->set_addr = virtio_mem_pci_set_addr;
-- 
2.48.1

Re: [PATCH v2 1/2] virtio-mem-pci: Allow setting nvectors, so we can use MSI-X

[Thomas Huth]

On 28/01/2025 19.57, David Hildenbrand wrote:
> Let's do it similar as virtio-balloon-pci. With this change, we can
> use virtio-mem-pci on s390x, although plugging will still fail until
> properly wired up in the machine.
> 
> No need to worry about transitional/non_transitional devices, because they
> don't exist for virtio-mem.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   hw/core/machine.c          |  1 +
>   hw/virtio/virtio-mem-pci.c | 12 ++++++++++++
>   2 files changed, 13 insertions(+)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH v2 2/2] s390x/s390-virtio-ccw: Support plugging PCI-based virtio memory devices

[David Hildenbrand]

Let's just wire it up, unlocking virtio-mem-pci support on s390x.

While at it, drop the "return;" in s390_machine_device_unplug_request(),
to make it look like the other handlers.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/s390x/s390-virtio-ccw.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3af613d4e9..71f3443a53 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -554,8 +554,7 @@ static void s390_machine_device_pre_plug(HotplugHandler *hotplug_dev,
     if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW)) {
         virtio_ccw_md_pre_plug(VIRTIO_MD_CCW(dev), MACHINE(hotplug_dev), errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
-        error_setg(errp,
-                   "PCI-attached virtio based memory devices not supported");
+        virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
     }
 }
 
@@ -566,7 +565,8 @@ static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
 
     if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
         s390_cpu_plug(hotplug_dev, dev, errp);
-    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW)) {
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW) ||
+               object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
         /*
          * At this point, the device is realized and set all memdevs mapped, so
          * qemu_maxrampagesize() will pick up the page sizes of these memdevs
@@ -580,7 +580,11 @@ static void s390_machine_device_plug(HotplugHandler *hotplug_dev,
                        " initial memory");
             return;
         }
-        virtio_ccw_md_plug(VIRTIO_MD_CCW(dev), MACHINE(hotplug_dev), errp);
+        if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW)) {
+            virtio_ccw_md_plug(VIRTIO_MD_CCW(dev), MACHINE(hotplug_dev), errp);
+        } else {
+            virtio_md_pci_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
+        }
     }
 }
 
@@ -589,10 +593,12 @@ static void s390_machine_device_unplug_request(HotplugHandler *hotplug_dev,
 {
     if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
         error_setg(errp, "CPU hot unplug not supported on this machine");
-        return;
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW)) {
         virtio_ccw_md_unplug_request(VIRTIO_MD_CCW(dev), MACHINE(hotplug_dev),
                                      errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
+        virtio_md_pci_unplug_request(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev),
+                                     errp);
     }
 }
 
@@ -601,7 +607,9 @@ static void s390_machine_device_unplug(HotplugHandler *hotplug_dev,
 {
     if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_CCW)) {
         virtio_ccw_md_unplug(VIRTIO_MD_CCW(dev), MACHINE(hotplug_dev), errp);
-     }
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
+        virtio_md_pci_unplug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
+    }
  }
 
 static CpuInstanceProperties s390_cpu_index_to_props(MachineState *ms,
-- 
2.48.1

Re: [PATCH v2 0/2] s390x: support virtio-mem-pci

[Mario Casquero]

This series has been successfully tested in s390x. Here the detailed test steps:

Boot up a VM already containing a memory object
/home/qemu/build/qemu-system-s390x \
...
-m 4G,maxmem=20G \
-object memory-backend-ram,id=mem0,size=16G,reserve=off \
...

Check the memory devices
(qemu) info memory-devices
(qemu)

Check the PCI devices in the VM
[root@localhost ~]# lspci -v
[root@localhost ~]#

Hotplug a virtio-mem-pci device and check the memory devices
(qemu) device_add virtio-mem-pci,id=vmem0,memdev=mem0,requested-size=1G
(qemu) info memory-devices
Memory device [virtio-mem]: "vmem0"
  memaddr: 0x100000000
  node: 0
  requested-size: 1073741824
  size: 1073741824
  max-size: 17179869184
  block-size: 1048576
  memdev: /objects/mem0

Check the PCI device is correct in the guest
[root@localhost ~]# lspci -v
0001:00:00.0 Unclassified device [00ff]: Red Hat, Inc. virtio-mem (rev 01)
Subsystem: Red Hat, Inc. Device 1100
Physical Slot: 00000000
Flags: bus master, fast devsel, latency 0, IOMMU group 0
Memory at 4000000000000000 (32-bit, non-prefetchable) [virtual] [size=4K]
Memory at 4001000000000000 (64-bit, prefetchable) [virtual] [size=16K]
Capabilities: [98] MSI-X: Enable+ Count=2 Masked-
Capabilities: [84] Vendor Specific Information: VirtIO: <unknown>
Capabilities: [70] Vendor Specific Information: VirtIO: Notify
Capabilities: [60] Vendor Specific Information: VirtIO: DeviceCfg
Capabilities: [50] Vendor Specific Information: VirtIO: ISR
Capabilities: [40] Vendor Specific Information: VirtIO: CommonCfg
Kernel driver in use: virtio-pci

Resize the virtio_mem device and verify the new size
(qemu) qom-set vmem0 requested-size 4G
(qemu) info memory-devices
Memory device [virtio-mem]: "vmem0"
  memaddr: 0x100000000
  node: 0
  requested-size: 4294967296
  size: 4294967296
  max-size: 17179869184
  block-size: 1048576
  memdev: /objects/mem0

Finally, try to perform a failed unplug, resize the device to 0, and
unplug it seamlessly:
(qemu) device_del vmem0
Error: virtio-mem device cannot get unplugged while some of its memory
is still plugged
(qemu) qom-set vmem0 requested-size 0
(qemu) device_del vmem0

Tested-by: Mario Casquero <mcasquer@redhat.com>

On Tue, Jan 28, 2025 at 7:57 PM David Hildenbrand <david@redhat.com> wrote:
>
> This is based-on [1], which adds MSI-X support to virtio-balloon-pci,
> but can be applied independently.
>
> Turns out it is fairly easy to get virtio-mem-pci running on s390x. We
> only have to add MSI-X support to virtio-mem-pci, and wire-up the
> (un)plugging in the machine.
>
> Tried some simple stuff (hotplug/hotunplug/resize/reboot), and all seems
> to be working as expected.
>
> The kernel in the VM needs both, CONFIG_VIRTIO_PCI and CONFIG_VIRTIO_MEM
> for it to work.
>
> [1] https://lkml.kernel.org/r/20250115161425.246348-1-arbab@linux.ibm.com
>
> v1 -> v2:
> * There are no transitional/non_transitional devices for virtio-mem
> * Spell out removal of "return;" in second patch
>
> Cc: Eduardo Habkost <eduardo@habkost.net>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Ilya Leoshkevich <iii@linux.ibm.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> Cc: Eric Farman <farman@linux.ibm.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Boris Fiuczynski <fiuczy@linux.ibm.com>
> Cc: Michal Privoznik <mprivozn@redhat.com>
> Cc: Mario Casquero <mcasquer@redhat.com>
>
> David Hildenbrand (2):
>   virtio-mem-pci: Allow setting nvectors, so we can use MSI-X
>   s390x/s390-virtio-ccw: Support plugging PCI-based virtio memory
>     devices
>
>  hw/core/machine.c          |  1 +
>  hw/s390x/s390-virtio-ccw.c | 20 ++++++++++++++------
>  hw/virtio/virtio-mem-pci.c | 12 ++++++++++++
>  3 files changed, 27 insertions(+), 6 deletions(-)
>
> --
> 2.48.1
>

Re: [PATCH v2 0/2] s390x: support virtio-mem-pci

[Michael S. Tsirkin]

On Tue, Jan 28, 2025 at 07:57:03PM +0100, David Hildenbrand wrote:
> This is based-on [1], which adds MSI-X support to virtio-balloon-pci,
> but can be applied independently.
> 
> Turns out it is fairly easy to get virtio-mem-pci running on s390x. We
> only have to add MSI-X support to virtio-mem-pci, and wire-up the
> (un)plugging in the machine.
> 
> Tried some simple stuff (hotplug/hotunplug/resize/reboot), and all seems
> to be working as expected.
> 
> The kernel in the VM needs both, CONFIG_VIRTIO_PCI and CONFIG_VIRTIO_MEM
> for it to work.
> 
> [1] https://lkml.kernel.org/r/20250115161425.246348-1-arbab@linux.ibm.com


Fails CI:

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


> v1 -> v2:
> * There are no transitional/non_transitional devices for virtio-mem
> * Spell out removal of "return;" in second patch
> 
> Cc: Eduardo Habkost <eduardo@habkost.net>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Ilya Leoshkevich <iii@linux.ibm.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> Cc: Eric Farman <farman@linux.ibm.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Cornelia Huck <cohuck@redhat.com>
> Cc: Boris Fiuczynski <fiuczy@linux.ibm.com>
> Cc: Michal Privoznik <mprivozn@redhat.com>
> Cc: Mario Casquero <mcasquer@redhat.com>
> 
> David Hildenbrand (2):
>   virtio-mem-pci: Allow setting nvectors, so we can use MSI-X
>   s390x/s390-virtio-ccw: Support plugging PCI-based virtio memory
>     devices
> 
>  hw/core/machine.c          |  1 +
>  hw/s390x/s390-virtio-ccw.c | 20 ++++++++++++++------
>  hw/virtio/virtio-mem-pci.c | 12 ++++++++++++
>  3 files changed, 27 insertions(+), 6 deletions(-)
> 
> -- 
> 2.48.1

Re: [PATCH v2 0/2] s390x: support virtio-mem-pci

[David Hildenbrand]

On 21.02.25 00:24, Michael S. Tsirkin wrote:
> On Tue, Jan 28, 2025 at 07:57:03PM +0100, David Hildenbrand wrote:
>> This is based-on [1], which adds MSI-X support to virtio-balloon-pci,
>> but can be applied independently.
>>
>> Turns out it is fairly easy to get virtio-mem-pci running on s390x. We
>> only have to add MSI-X support to virtio-mem-pci, and wire-up the
>> (un)plugging in the machine.
>>
>> Tried some simple stuff (hotplug/hotunplug/resize/reboot), and all seems
>> to be working as expected.
>>
>> The kernel in the VM needs both, CONFIG_VIRTIO_PCI and CONFIG_VIRTIO_MEM
>> for it to work.
>>
>> [1] https://lkml.kernel.org/r/20250115161425.246348-1-arbab@linux.ibm.com
> 
> 
> Fails CI:
> 
> https://gitlab.com/mstredhat/qemu/-/jobs/9202574981

Hm, I thought the CI was happy when Thomas sent it upstream.

CRITICAL: log: qemu-system-i386: -device virtio-mem-pci: can't apply 
global virtio-mem-pci.vectors=0: Property 'virtio-mem-pci.vectors' not found

And the same for the virtio-balloon devices with a similar change.

Trying to run one offending cmdline on current upstream:

./qemu-system-i386 -display none -vga none -S -machine 
pc-q35-8.0,accel=tcg -device virtio-mem-pci
qemu-system-i386: -device virtio-mem-pci: the configuration is not 
prepared for memory devices (e.g., for memory hotplug), consider 
specifying the maxmem option


So I am not sure what is happening here? Is some commit in the pipeline 
breaking this?

-- 
Cheers,

David / dhildenb

Re: [PATCH v2 0/2] s390x: support virtio-mem-pci

[David Hildenbrand]

On 21.02.25 09:26, David Hildenbrand wrote:
> On 21.02.25 00:24, Michael S. Tsirkin wrote:
>> On Tue, Jan 28, 2025 at 07:57:03PM +0100, David Hildenbrand wrote:
>>> This is based-on [1], which adds MSI-X support to virtio-balloon-pci,
>>> but can be applied independently.
>>>
>>> Turns out it is fairly easy to get virtio-mem-pci running on s390x. We
>>> only have to add MSI-X support to virtio-mem-pci, and wire-up the
>>> (un)plugging in the machine.
>>>
>>> Tried some simple stuff (hotplug/hotunplug/resize/reboot), and all seems
>>> to be working as expected.
>>>
>>> The kernel in the VM needs both, CONFIG_VIRTIO_PCI and CONFIG_VIRTIO_MEM
>>> for it to work.
>>>
>>> [1] https://lkml.kernel.org/r/20250115161425.246348-1-arbab@linux.ibm.com
>>
>>
>> Fails CI:
>>
>> https://gitlab.com/mstredhat/qemu/-/jobs/9202574981
> 
> Hm, I thought the CI was happy when Thomas sent it upstream.
> 
> CRITICAL: log: qemu-system-i386: -device virtio-mem-pci: can't apply
> global virtio-mem-pci.vectors=0: Property 'virtio-mem-pci.vectors' not found
> 
> And the same for the virtio-balloon devices with a similar change.
> 
> Trying to run one offending cmdline on current upstream:
> 
> ./qemu-system-i386 -display none -vga none -S -machine
> pc-q35-8.0,accel=tcg -device virtio-mem-pci
> qemu-system-i386: -device virtio-mem-pci: the configuration is not
> prepared for memory devices (e.g., for memory hotplug), consider
> specifying the maxmem option
> 
> 
> So I am not sure what is happening here? Is some commit in the pipeline
> breaking this?

Note that these two patches, and the virtio-balloon-pci MSI-X
one are already upstream, Thomas included them in his s390 MR. I see 
that you have them included on your branch:

https://gitlab.com/mstredhat/qemu/-/commits/v03b-20-02-2025?ref_type=heads

The following commit in that tree is messed up:

* virtio-mem-pci: Allow setting nvectors, so we can use MSI-X

Likely, you can just drop these two patches from your tree; they are 
alreayd upstream.

-- 
Cheers,

David / dhildenb