Hi Michal,
On Fri, May 09, 2025 at 03:30:21PM +0200, Michal Prívozník wrote:
> On 4/2/25 10:25, Andrea Righi via Devel wrote:
> > = Overview =
> >
> > This patch set introduces support for acpi-generic-initiator devices,
> > supported by QEMU [1].
> >
> > The acpi-generic-initiator object is required to support Multi-Instance GPU
> > (MIG) configurations on NVIDIA GPUs [2]. MIG enables partitioning of GPU
> > resources into multiple isolated instances, each requiring a dedicated NUMA
> > node definition.
> >
> > = Implementation =
> >
> > This patch set implements the libvirt counterpart to the QEMU feature,
> > enabling users to configure acpi-generic-initiator objects within libvirt
> > domain XML.
> >
> > This includes:
> > - adding XML syntax to define acpi-generic-initiator objects,
> > - resolving the acpi-generic-initiator definitions into the proper QEMU
> > command-line arguments,
> > - ensuring compatibility with existing NUMA configuration.
> >
> > = Example =
> >
> > - Domain XML:
> > ```
> > ...
> > <cpu mode='host-passthrough' check='none'>
> > <numa>
> > <cell id='0' cpus='0-15' memory='8388608' unit='KiB'/>
> > <cell id='1' memory='0' unit='KiB'/>
> > <cell id='2' memory='0' unit='KiB'/>
> > <cell id='3' memory='0' unit='KiB'/>
> > <cell id='4' memory='0' unit='KiB'/>
> > <cell id='5' memory='0' unit='KiB'/>
> > <cell id='6' memory='0' unit='KiB'/>
> > <cell id='7' memory='0' unit='KiB'/>
> > <cell id='8' memory='0' unit='KiB'/>
> > </numa>
> > </cpu>
> > ...
> > <devices>
> > ...
> > <hostdev mode='subsystem' type='pci' managed='no'>
> > <source>
> > <address domain='0x0009' bus='0x01' slot='0x00' function='0x0'/>
> > </source>
> > <address type='pci' domain='0x0000' bus='0x03' slot='0x00'
> > function='0x0'/>
> > </hostdev>
> > <acpi-generic-initiator>
> > <alias name="gi1"/>
> > <pci-dev>hostdev0</pci-dev>
> > <numa-node>1</numa-node>
> > </acpi-generic-initiator>
> > <acpi-generic-initiator>
> > <alias name="gi2"/>
> > <pci-dev>hostdev0</pci-dev>
> > <numa-node>2</numa-node>
> > </acpi-generic-initiator>
> > <acpi-generic-initiator>
> > <alias name="gi3"/>
> > <pci-dev>hostdev0</pci-dev>
> > <numa-node>3</numa-node>
> > </acpi-generic-initiator>
> > <acpi-generic-initiator>
> > <alias name="gi4"/>
> > <pci-dev>hostdev0</pci-dev>
> > <numa-node>4</numa-node>
> > </acpi-generic-initiator>
> > <acpi-generic-initiator>
> > <alias name="gi5"/>
> > <pci-dev>hostdev0</pci-dev>
> > <numa-node>5</numa-node>
> > </acpi-generic-initiator>
> > <acpi-generic-initiator>
> > <alias name="gi6"/>
> > <pci-dev>hostdev0</pci-dev>
> > <numa-node>6</numa-node>
> > </acpi-generic-initiator>
> > <acpi-generic-initiator>
> > <alias name="gi7"/>
> > <pci-dev>hostdev0</pci-dev>
> > <numa-node>7</numa-node>
> > </acpi-generic-initiator>
> > <acpi-generic-initiator>
> > <alias name="gi8"/>
> > <pci-dev>hostdev0</pci-dev>
> > <numa-node>8</numa-node>
> > </acpi-generic-initiator>
> > </devices>
> > ```
> >
> > - Generated QEMU command line options:
> > ```
> > ... /usr/bin/qemu-system-aarch64 \
> > ...
> > -object
> > '{"qom-type":"memory-backend-ram","id":"ram-node0","size":8589934592}' \
> > -numa node,nodeid=0,cpus=0-15,memdev=ram-node0 \
> > -numa node,nodeid=1 \
> > -numa node,nodeid=2 \
> > -numa node,nodeid=3 \
> > -numa node,nodeid=4 \
> > -numa node,nodeid=5 \
> > -numa node,nodeid=6 \
> > -numa node,nodeid=7 \
> > -numa node,nodeid=8 \
> > ...
> > -device
> > '{"driver":"vfio-pci","host":"0009:01:00.0","id":"hostdev0","bus":"pci.3","addr":"0x0"}'
> > ...
> > -object acpi-generic-initiator,id=gi1,pci-dev=hostdev0,node=1 \
> > -object acpi-generic-initiator,id=gi2,pci-dev=hostdev0,node=2 \
> > -object acpi-generic-initiator,id=gi3,pci-dev=hostdev0,node=3 \
> > -object acpi-generic-initiator,id=gi4,pci-dev=hostdev0,node=4 \
> > -object acpi-generic-initiator,id=gi5,pci-dev=hostdev0,node=5 \
> > -object acpi-generic-initiator,id=gi6,pci-dev=hostdev0,node=6 \
> > -object acpi-generic-initiator,id=gi7,pci-dev=hostdev0,node=7 \
> > -object acpi-generic-initiator,id=gi8,pci-dev=hostdev0,node=8
> > ```
> >
> > = References =
> >
> > [1] https://lore.kernel.org/all/20231225045603.7654-2-ank...@nvidia.com/
> > [2] https://www.nvidia.com/en-in/technologies/multi-instance-gpu/
> >
> > ChangeLog v2 -> v3:
> > - replaced <text/> with proper types in the XML schema
> > - avoid mixing g_free() and VIR_FREE()
> > - use virXMLPropString() instead of looping all XML nodes
> > - report proper errors with virReportError()
> > - use virBufferEscapeString() to process strings passed by the user
> > - fix broken formatting of function headers
> > - misc coding style fixes
> >
> > ChangeLog v1 -> v2:
> > - split parser and driver changes in separate patches
> > - introduce a new qemu capability flag
> > - introduce test in qemuxmlconftest
> >
> > Andrea Righi (6):
> > schema: Introduce acpi-generic-initiator definition
> > conf: Introduce acpi-generic-initiator device
> > qemu: Allow to define NUMA nodes without memory or CPUs assigned
> > qemu: capabilies: Introduce QEMU_CAPS_ACPI_GENERIC_INITIATOR
> > qemu: support acpi-generic-initiator
> > qemu: Add test case for acpi-generic-initiator
> >
> > src/ch/ch_domain.c | 1 +
> > src/conf/domain_conf.c | 159
> > +++++++++++++++++++++
> > src/conf/domain_conf.h | 14 ++
> > src/conf/domain_postparse.c | 1 +
> > src/conf/domain_validate.c | 37 +++++
> > src/conf/numa_conf.c | 3 +
> > src/conf/schemas/domaincommon.rng | 19 +++
> > src/conf/virconftypes.h | 2 +
> > src/libxl/libxl_driver.c | 6 +
> > src/lxc/lxc_driver.c | 6 +
> > src/qemu/qemu_capabilities.c | 2 +
> > src/qemu/qemu_capabilities.h | 1 +
> > src/qemu/qemu_command.c | 49 ++++++-
> > src/qemu/qemu_domain.c | 2 +
> > src/qemu/qemu_domain_address.c | 4 +
> > src/qemu/qemu_driver.c | 3 +
> > src/qemu/qemu_hotplug.c | 5 +
> > src/qemu/qemu_postparse.c | 1 +
> > src/qemu/qemu_validate.c | 1 +
> > src/test/test_driver.c | 4 +
> > .../caps_10.0.0_x86_64+amdsev.xml | 1 +
> > tests/qemucapabilitiesdata/caps_10.0.0_x86_64.xml | 1 +
> > tests/qemucapabilitiesdata/caps_9.0.0_x86_64.xml | 1 +
> > tests/qemucapabilitiesdata/caps_9.1.0_riscv64.xml | 1 +
> > tests/qemucapabilitiesdata/caps_9.1.0_x86_64.xml | 1 +
> > .../caps_9.2.0_aarch64+hvf.xml | 1 +
> > .../caps_9.2.0_x86_64+amdsev.xml | 1 +
> > tests/qemucapabilitiesdata/caps_9.2.0_x86_64.xml | 1 +
> > .../acpi-generic-initiator.x86_64-latest.args | 55 +++++++
> > .../acpi-generic-initiator.x86_64-latest.xml | 102 +++++++++++++
> > tests/qemuxmlconfdata/acpi-generic-initiator.xml | 102 +++++++++++++
> > tests/qemuxmlconftest.c | 1 +
> > 32 files changed, 581 insertions(+), 7 deletions(-)
> > create mode 100644
> > tests/qemuxmlconfdata/acpi-generic-initiator.x86_64-latest.args
> > create mode 100644
> > tests/qemuxmlconfdata/acpi-generic-initiator.x86_64-latest.xml
> > create mode 100644 tests/qemuxmlconfdata/acpi-generic-initiator.xml
> >
>
> Sorry for late review. Normally, to make it up to you I'd fix all the
> small nits and merge, but this is introducing new feature and yet not
> updating documentation and that's a show stopper. Also, don't forget to
> add a NEWS.rst entry ;-)
Thanks for taking a look at this. Do you think docs/formatdomain.rst is the
appropriate place to document the new acpi-generic-initiator?
I'll also make sure to add a proper entry in NEWS.rst.
-Andrea
