[v2] Enable virtio-mem-ccw

[PATCH v2 0/8] Enable virtio-mem-ccw

Posted by Michal Privoznik 7 months, 1 week ago

v2 of:

https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/KA2DGRIY7DAMNMYM4MBKLOJCB7YYEUKU/

diff to v1:
- Previously, virtio-mem-pci wasn't supported (in fact QEMU errored
  out), but this changed in QEMU upstream, so this limitation is lifted.
- Added a test case for virtio-mem-pci on s390x

Most of the patches is ACKed already (thanks Boris and David!), only a
few remain for review.

Michal Prívozník (8):
  qemu: Do NOT autoadd NUMA node for s390
  qemu_command: Use qemuBuildVirtioDevProps() to build cmd line for
    virtio-mem and virtio-pmem
  qemuxmlconftest: Introduce memory-hotplug-virtio-mem-pci-s390x.xml
  qemu_caps: Introduce QEMU_CAPS_DEVICE_VIRTIO_MEM_CCW
  qemu: Validate virtio-mem-ccw
  qemu: Allow virtio-mem on CCW
  qemuxmlconftest: Introduce memory-hotplug-virtio-mem-ccw-s390x.xml
  NEWS: Document virtio-mem-ccw

 NEWS.rst                                      |  5 ++
 src/qemu/qemu_capabilities.c                  |  2 +
 src/qemu/qemu_capabilities.h                  |  1 +
 src/qemu/qemu_command.c                       | 28 ++++++--
 src/qemu/qemu_domain.c                        |  6 +-
 src/qemu/qemu_postparse.c                     |  1 +
 src/qemu/qemu_validate.c                      | 35 ++++++++-
 .../caps_10.0.0_s390x.xml                     |  1 +
 ...lug-virtio-mem-ccw-s390x.s390x-latest.args | 39 ++++++++++
 ...plug-virtio-mem-ccw-s390x.s390x-latest.xml | 60 ++++++++++++++++
 .../memory-hotplug-virtio-mem-ccw-s390x.xml   | 57 +++++++++++++++
 ...lug-virtio-mem-pci-s390x.s390x-latest.args | 41 +++++++++++
 ...plug-virtio-mem-pci-s390x.s390x-latest.xml | 71 +++++++++++++++++++
 .../memory-hotplug-virtio-mem-pci-s390x.xml   | 59 +++++++++++++++
 tests/qemuxmlconftest.c                       |  2 +
 15 files changed, 399 insertions(+), 9 deletions(-)
 create mode 100644 tests/qemuxmlconfdata/memory-hotplug-virtio-mem-ccw-s390x.s390x-latest.args
 create mode 100644 tests/qemuxmlconfdata/memory-hotplug-virtio-mem-ccw-s390x.s390x-latest.xml
 create mode 100644 tests/qemuxmlconfdata/memory-hotplug-virtio-mem-ccw-s390x.xml
 create mode 100644 tests/qemuxmlconfdata/memory-hotplug-virtio-mem-pci-s390x.s390x-latest.args
 create mode 100644 tests/qemuxmlconfdata/memory-hotplug-virtio-mem-pci-s390x.s390x-latest.xml
 create mode 100644 tests/qemuxmlconfdata/memory-hotplug-virtio-mem-pci-s390x.xml

-- 
2.45.3

Re: [PATCH v2 0/8] Enable virtio-mem-ccw

Posted by David Hildenbrand 7 months ago

On 03.02.25 10:55, Michal Privoznik wrote:
> v2 of:
> 
> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/KA2DGRIY7DAMNMYM4MBKLOJCB7YYEUKU/
> 

I should probably play with this myself, but due to lack of a test 
system where I can mess with systemd:

Assuming we're on a s390x host (using KVM) and we specify:

+    <memory model='virtio-mem'>
+      <target dynamicMemslots='yes'>
+        <size unit='KiB'>2097152</size>
+        <node>0</node>
+        <requested unit='KiB'>1048576</requested>
+      </target>
+    </memory>

In particular, without

+        <block unit='KiB'>2048</block>

What would be the default value on a s390x host?

In QEMU, we'd be using the default of 1 MiB on a s390x host, which 
corresponds to the THP size. Note that on an x86_64 host, the default 
will be 2 MiB.

Ideally, we'd also be using the default of 1 MiB on an s390x host.

How is the default value determined here?

Thanks!

-- 
Cheers,

David / dhildenb

Re: [PATCH v2 0/8] Enable virtio-mem-ccw

Posted by Boris Fiuczynski 7 months ago

On 2/4/25 10:06, David Hildenbrand wrote:
> On 03.02.25 10:55, Michal Privoznik wrote:
>> v2 of:
>>
>> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/ 
>> thread/KA2DGRIY7DAMNMYM4MBKLOJCB7YYEUKU/
>>
> 
> I should probably play with this myself, but due to lack of a test 
> system where I can mess with systemd:
> 
> Assuming we're on a s390x host (using KVM) and we specify:
> 
> +    <memory model='virtio-mem'>
> +      <target dynamicMemslots='yes'>
> +        <size unit='KiB'>2097152</size>
> +        <node>0</node>
> +        <requested unit='KiB'>1048576</requested>
> +      </target>
> +    </memory>
> 
> In particular, without
> 
> +        <block unit='KiB'>2048</block>
> 
> What would be the default value on a s390x host?
> 
> In QEMU, we'd be using the default of 1 MiB on a s390x host, which 
> corresponds to the THP size. Note that on an x86_64 host, the default 
> will be 2 MiB.
> 
> Ideally, we'd also be using the default of 1 MiB on an s390x host.
> 
> How is the default value determined here?
> 
> Thanks!
> 

Current libvirt implementation does not generate a default.
When not providing the element block an error occurs:
  error: block size must be a power of two
When providing the element block the required minimum value is 1024KiB.


-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH v2 0/8] Enable virtio-mem-ccw

Posted by David Hildenbrand 7 months ago

On 04.02.25 10:57, Boris Fiuczynski wrote:
> On 2/4/25 10:06, David Hildenbrand wrote:
>> On 03.02.25 10:55, Michal Privoznik wrote:
>>> v2 of:
>>>
>>> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/
>>> thread/KA2DGRIY7DAMNMYM4MBKLOJCB7YYEUKU/
>>>
>>
>> I should probably play with this myself, but due to lack of a test
>> system where I can mess with systemd:
>>
>> Assuming we're on a s390x host (using KVM) and we specify:
>>
>> +    <memory model='virtio-mem'>
>> +      <target dynamicMemslots='yes'>
>> +        <size unit='KiB'>2097152</size>
>> +        <node>0</node>
>> +        <requested unit='KiB'>1048576</requested>
>> +      </target>
>> +    </memory>
>>
>> In particular, without
>>
>> +        <block unit='KiB'>2048</block>
>>
>> What would be the default value on a s390x host?
>>
>> In QEMU, we'd be using the default of 1 MiB on a s390x host, which
>> corresponds to the THP size. Note that on an x86_64 host, the default
>> will be 2 MiB.
>>
>> Ideally, we'd also be using the default of 1 MiB on an s390x host.
>>
>> How is the default value determined here?
>>
>> Thanks!
>>
> 
> Current libvirt implementation does not generate a default.
> When not providing the element block an error occurs:
>    error: block size must be a power of two
> When providing the element block the required minimum value is 1024KiB.

Right, the 1 MiB minimum value independent of the memory backed is 
currently enforced by QEMU.

Interesting that no defaults are provided. I assume it might be an 
interesting idea to query the default from QEMU, instead of 
re-implementing that logic in libvirt.

-- 
Cheers,

David / dhildenb

Re: [PATCH v2 0/8] Enable virtio-mem-ccw

Posted by Boris Fiuczynski 7 months ago

On 2/4/25 11:04, David Hildenbrand wrote:
> On 04.02.25 10:57, Boris Fiuczynski wrote:
>> On 2/4/25 10:06, David Hildenbrand wrote:
>>> On 03.02.25 10:55, Michal Privoznik wrote:
>>>> v2 of:
>>>>
>>>> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/
>>>> thread/KA2DGRIY7DAMNMYM4MBKLOJCB7YYEUKU/
>>>>
>>>
>>> I should probably play with this myself, but due to lack of a test
>>> system where I can mess with systemd:
>>>
>>> Assuming we're on a s390x host (using KVM) and we specify:
>>>
>>> +    <memory model='virtio-mem'>
>>> +      <target dynamicMemslots='yes'>
>>> +        <size unit='KiB'>2097152</size>
>>> +        <node>0</node>
>>> +        <requested unit='KiB'>1048576</requested>
>>> +      </target>
>>> +    </memory>
>>>
>>> In particular, without
>>>
>>> +        <block unit='KiB'>2048</block>
>>>
>>> What would be the default value on a s390x host?
>>>
>>> In QEMU, we'd be using the default of 1 MiB on a s390x host, which
>>> corresponds to the THP size. Note that on an x86_64 host, the default
>>> will be 2 MiB.
>>>
>>> Ideally, we'd also be using the default of 1 MiB on an s390x host.
>>>
>>> How is the default value determined here?
>>>
>>> Thanks!
>>>
>>
>> Current libvirt implementation does not generate a default.
>> When not providing the element block an error occurs:
>>    error: block size must be a power of two
>> When providing the element block the required minimum value is 1024KiB.
> 
> Right, the 1 MiB minimum value independent of the memory backed is 
> currently enforced by QEMU.

The validation is done by libvirt in the domain validation in which QEMU 
is not involved.

> 
> Interesting that no defaults are provided. I assume it might be an 
> interesting idea to query the default from QEMU, instead of re- 
> implementing that logic in libvirt.
> 

Usually the later is done when a default is set by libvirt and is not 
required to be provided by the user.


-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294