= 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,
 - adding a new qemu capability flag,
 - 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>
...
  <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 \
...
-object '{"qom-type":"acpi-generic-initiator","id":"gi1","pci-dev":"hostdev0","node":1}' \
-object '{"qom-type":"acpi-generic-initiator","id":"gi2","pci-dev":"hostdev0","node":2}' \
-object '{"qom-type":"acpi-generic-initiator","id":"gi3","pci-dev":"hostdev0","node":3}' \
-object '{"qom-type":"acpi-generic-initiator","id":"gi4","pci-dev":"hostdev0","node":4}' \
-object '{"qom-type":"acpi-generic-initiator","id":"gi5","pci-dev":"hostdev0","node":5}' \
-object '{"qom-type":"acpi-generic-initiator","id":"gi6","pci-dev":"hostdev0","node":6}' \
-object '{"qom-type":"acpi-generic-initiator","id":"gi7","pci-dev":"hostdev0","node":7}' \
-object '{"qom-type":"acpi-generic-initiator","id":"gi8","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/

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                             | 153 +++++++++++++++++++++
 src/conf/domain_conf.h                             |  14 ++
 src/conf/domain_postparse.c                        |   1 +
 src/conf/domain_validate.c                         |  40 ++++++
 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                            |  48 +++++--
 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_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 +
 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 +
 29 files changed, 573 insertions(+), 8 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

Gentle ping. Is there any feedback, comment, suggestion about this?

Thanks,
-Andrea

On Fri, Mar 07, 2025 at 08:13:51 +0100, Andrea Righi via Devel wrote:
> Gentle ping. Is there any feedback, comment, suggestion about this?

I have a couple points about coding style and some general stuff. This
feature is out of interest are and thus I will not try to understand
whether the design or implementation makes sense.

I'll summarize my points here as it's not a proper review:


Patch 1/6:

- Format the function header as:

  void
  virDomainAcpiInitiatorDefFree(

- don't mix g_free and VIR_FREE;
- Try to use proper type instead of '<text/>' in XML schema

Patch 2/6:

- virDomainAcpiInitiatorDefParseXML:
    - broken formatting of arguments
    - do not use the for-loop; I refactored most of other code to remove
      this style of parser, use XPaths and virXMLProp* instead
    - the function doesn't report libvirt errors on some code paths
    - virDomainAcpiInitiatorDefNew can't fail thus no point in checking
      return value

    - broken formatting in call of this function from virDomainDeviceDefParse

- virDomainAcpiInitiatorDefCheckABIStability

    - broken formatting of function header

- virDomainAcpiInitiatorDefFormat
    - broken formatting in header
    - attrBuf variable not actually used
    - use virBufferEscapeString for any strings that are parsed for the user
      to ensure XML entity escaping
    - extra space in 'alias' element


- virDomainAcpiInitiatorDefValidate
    - first for loop must have a block  (looks confusing)
        - it's also a rather peculiar way to check that
         'acpiinitiator->numaNode' is < nodeCount;

    - don't use VIR_ERR_INTERNAL_ERROR for configuration errors
    - completely broken formatting of all virReportError calls

patch 3/6:

- patch breaks virReportError formatting without changing the error

Hi Peter,

On Fri, Mar 07, 2025 at 01:45:01PM +0100, Peter Krempa wrote:
> On Fri, Mar 07, 2025 at 08:13:51 +0100, Andrea Righi via Devel wrote:
> > Gentle ping. Is there any feedback, comment, suggestion about this?
> 
> I have a couple points about coding style and some general stuff. This
> feature is out of interest are and thus I will not try to understand
> whether the design or implementation makes sense.
> 
> I'll summarize my points here as it's not a proper review:

Thanks for the review, I'll fix all the coding style issues in the next
version.

Do you think I should include more details in the cover letter to better
explain the use cases of this feature?

-Andrea

> 
> 
> Patch 1/6:
> 
> - Format the function header as:
> 
>   void
>   virDomainAcpiInitiatorDefFree(
> 
> - don't mix g_free and VIR_FREE;
> - Try to use proper type instead of '<text/>' in XML schema
> 
> Patch 2/6:
> 
> - virDomainAcpiInitiatorDefParseXML:
>     - broken formatting of arguments
>     - do not use the for-loop; I refactored most of other code to remove
>       this style of parser, use XPaths and virXMLProp* instead
>     - the function doesn't report libvirt errors on some code paths
>     - virDomainAcpiInitiatorDefNew can't fail thus no point in checking
>       return value
> 
>     - broken formatting in call of this function from virDomainDeviceDefParse
> 
> - virDomainAcpiInitiatorDefCheckABIStability
> 
>     - broken formatting of function header
> 
> - virDomainAcpiInitiatorDefFormat
>     - broken formatting in header
>     - attrBuf variable not actually used
>     - use virBufferEscapeString for any strings that are parsed for the user
>       to ensure XML entity escaping
>     - extra space in 'alias' element
> 
> 
> - virDomainAcpiInitiatorDefValidate
>     - first for loop must have a block  (looks confusing)
>         - it's also a rather peculiar way to check that
>          'acpiinitiator->numaNode' is < nodeCount;
> 
>     - don't use VIR_ERR_INTERNAL_ERROR for configuration errors
>     - completely broken formatting of all virReportError calls
> 
> patch 3/6:
> 
> - patch breaks virReportError formatting without changing the error
>