On 6/8/25 22:44, 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>
> <pci-dev>hostdev0</pci-dev>
> <numa-node>1</numa-node>
> </acpi-generic-initiator>
> <acpi-generic-initiator>
> <pci-dev>hostdev0</pci-dev>
> <numa-node>2</numa-node>
> </acpi-generic-initiator>
> <acpi-generic-initiator>
> <pci-dev>hostdev0</pci-dev>
> <numa-node>3</numa-node>
> </acpi-generic-initiator>
> <acpi-generic-initiator>
> <pci-dev>hostdev0</pci-dev>
> <numa-node>4</numa-node>
> </acpi-generic-initiator>
> <acpi-generic-initiator>
> <pci-dev>hostdev0</pci-dev>
> <numa-node>5</numa-node>
> </acpi-generic-initiator>
> <acpi-generic-initiator>
> <pci-dev>hostdev0</pci-dev>
> <numa-node>6</numa-node>
> </acpi-generic-initiator>
> <acpi-generic-initiator>
> <pci-dev>hostdev0</pci-dev>
> <numa-node>7</numa-node>
> </acpi-generic-initiator>
> <acpi-generic-initiator>
> <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=gi0,pci-dev=hostdev0,node=1 \
> -object acpi-generic-initiator,id=gi1,pci-dev=hostdev0,node=2 \
> -object acpi-generic-initiator,id=gi2,pci-dev=hostdev0,node=3 \
> -object acpi-generic-initiator,id=gi3,pci-dev=hostdev0,node=4 \
> -object acpi-generic-initiator,id=gi4,pci-dev=hostdev0,node=5 \
> -object acpi-generic-initiator,id=gi5,pci-dev=hostdev0,node=6 \
> -object acpi-generic-initiator,id=gi6,pci-dev=hostdev0,node=7 \
> -object acpi-generic-initiator,id=gi7,pci-dev=hostdev0,node=8
> ```
>
> = References =
>
> [1] https://lore.kernel.org/all/20231225045603.7654-2-ankita@nvidia.com/
> [2] https://www.nvidia.com/en-in/technologies/multi-instance-gpu/
>
> This patchset is also available in the following git branch:
>
> https://github.com/arighi/libvirt.git acpi-generic-initiator-v4
>
> ChangeLog v3 -> v4:
> - add acpi-generic-initiator documentation
> - refactor virDomainAcpiInitiatorDef to use info->alias and drop the name
> attribute
> - auto-generate alias for the acpi-generic-initiator devices via
> qemuAssignDeviceAliases()
> - use g_autoptr() when possible
> - add a new entry to NEWS.rst
>
> 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 (8):
> 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
> docs: Document acpi-generic-initiator device
> NEWS: Mention new acpi-generic-initiator device
>
> NEWS.rst | 8 ++
> docs/formatdomain.rst | 36 ++++++
> src/ch/ch_domain.c | 1 +
> src/conf/domain_conf.c | 138 +++++++++++++++++++++
> 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 | 14 +++
> src/conf/virconftypes.h | 2 +
> src/hyperv/hyperv_driver.c | 1 +
> src/libxl/libxl_driver.c | 6 +
> src/lxc/lxc_driver.c | 6 +
> src/qemu/qemu_alias.c | 11 ++
> 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 +
> tests/qemucapabilitiesdata/caps_10.0.0_aarch64.xml | 1 +
> .../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 | 94 ++++++++++++++
> tests/qemuxmlconfdata/acpi-generic-initiator.xml | 94 ++++++++++++++
> tests/qemuxmlconftest.c | 1 +
> 37 files changed, 596 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
>
Code-wise this is okay. I've merged some patches together and made small
changes. You can find them here:
https://gitlab.com/MichalPrivoznik/libvirt/-/compare/master...review_acpi_initiator?from_project_id=192693
(I'll be squashing those 'fixup' commits, naturally)
What I am more worried about is the XML representation. But I'm failing
to come up with a better one. Dan, what do you think?
Michal