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-ank...@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

Reply via email to