[PATCH v7 0/4] introduce support for acpi-bridge-hotplug feature

Fri Oct 8 15:29:35 UTC 2021

Reviewed-by: Laine Stump <lain at redhat.com>

I'm going to push this later today after I put it through CI (and get 
back from a mandatory 4 hour drive).

On 10/8/21 2:42 AM, Ani Sinha wrote:
> changelog:
> 
> v7: Laine's suggestions from v6 incorporated. rebased the patchset to latest
>      master.
> v6: rebased to latest. capabilities have been renamed as per suggestions that
>      were made here:
>      https://listman.redhat.com/archives/libvir-list/2021-October/msg00061.html
> v5: rebased the patchset with the latest master.
> v4: split the original series into two - pci-root controller specific one
>      (https://www.mail-archive.com/libvir-list@redhat.com/msg221645.html)
>      and this one specific to pci bridges.
>      The conf xml has been introduced as per suggestion by Berrange here:
>      https://patchew.org/Libvirt/20210912032631.2853520-1-ani@anisinha.ca
>      Changes has been introduced to parse and validate the xml accordingly
>      as well as to add backend qemu commandline option.
> v3: reorganized the patches as per Laine's suggestion. Added more
>      details in commit messages. Added conf description in formatdomain.rst.
>      Added changelog for next release.
> v2: fixed bugs and added additional missing unit tests.
> v1: initial implementation. Had some bugs and missed some unit tests
> 
> This change introduces a new libvirt sub-element <pci> under <features> that
> can be used to configure all pci related features.
> Currently the only sub-sub element supported by this sub-element is
> 'acpi-bridge-hotplug' as shown below:
> 
> <features>
>    <pci>
>      <acpi-bridge-hotplug state='on|off'/>
>    </pci>
> </features>
> 
> The above option is only available for qemu driver and that too for x86 guests
> only. It is a global option.
> 
> 'acpi-bridge-hotplug' option enables or disables ACPI hotplug support for
> cold-plugged pci bridges. Examples of bridges include PCI-PCI bridge
> (pci-bridge controller) or PCIe-PCI bridges for pc machines and
> pcie-root-port controller for q35 machines. Being global option, no other
> bridge specific option are required. For pc machine type in x86, this option
> is available in qemu for a long time, from version 2.1.
> Please see the following changes in qemu repo:
> 
> 9e047b982452c6 ("piix4: add acpi pci hotplug support")
> 133a2da488062e ("pc: acpi: generate AML only for PCI0 devices if PCI bridge hotplug is disabled")
> 
> For q35 machine type, this was introduced in qemu 6.1 with the following
> changes in qemu repo:
> 
> (a) c0e427d6eb5fef ("hw/acpi/ich9: Enable ACPI PCI hot-plug")
> (b) 17858a16950860 ("hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35")
> 
> The reasons for enabling ACPI based hotplug for PCIe (q35) based machines (as
> opposed to native hotplug) are outlined in (b). There are use cases where users
> would still want to use native hotplug (see notes). Therefore, this config option
> enables users to choose either ACPI based hotplug or native hotplug for bridges
> (for example for pcie root port controller in q35 machines).
> 
> Notes:
> One concrete example of why one might still want to use native hotplug with
> pcie-root-port controller is the fact that we are still discovering issues with
> acpi hotplug on PCIE. One such issue is:
> https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg02146.html
> Another reason is that users have been using native hotplug on pcie root ports
> up until now. They have built and tested their systems based on native hotplug.
> They may not want to suddenly move to acpi based hotplug just because it is now
> the default in qemu. Supporting the option to chose one or the other through
> libvirt makes things simpler for end users.
> 
> Ani Sinha (4):
>    qemu: capablities: detect presence of
>      acpi-pci-hotplug-with-bridge-support
>    conf: introduce support for acpi-bridge-hotplug feature
>    qemu: command: add support for acpi-bridge-hotplug feature
>    NEWS: add new acpi pci hotplug config option in the release note for
>      next release
> 
>   NEWS.rst                                      |  8 ++
>   docs/formatdomain.rst                         | 16 ++++
>   docs/schemas/domaincommon.rng                 | 15 ++++
>   src/conf/domain_conf.c                        | 89 ++++++++++++++++++-
>   src/conf/domain_conf.h                        |  9 ++
>   src/qemu/qemu_capabilities.c                  |  4 +
>   src/qemu/qemu_capabilities.h                  |  2 +
>   src/qemu/qemu_command.c                       | 20 +++++
>   src/qemu/qemu_validate.c                      | 46 ++++++++++
>   .../caps_2.11.0.x86_64.xml                    |  1 +
>   .../caps_2.12.0.x86_64.xml                    |  1 +
>   .../caps_3.0.0.x86_64.xml                     |  1 +
>   .../caps_3.1.0.x86_64.xml                     |  1 +
>   .../caps_4.0.0.x86_64.xml                     |  1 +
>   .../caps_4.1.0.x86_64.xml                     |  1 +
>   .../caps_4.2.0.x86_64.xml                     |  1 +
>   .../caps_5.0.0.x86_64.xml                     |  1 +
>   .../caps_5.1.0.x86_64.xml                     |  1 +
>   .../caps_5.2.0.x86_64.xml                     |  1 +
>   .../caps_6.0.0.x86_64.xml                     |  1 +
>   .../caps_6.1.0.x86_64.xml                     |  2 +
>   .../aarch64-acpi-hotplug-bridge-disable.err   |  1 +
>   .../aarch64-acpi-hotplug-bridge-disable.xml   | 33 +++++++
>   ...pc-i440fx-acpi-hotplug-bridge-disable.args | 31 +++++++
>   .../pc-i440fx-acpi-hotplug-bridge-disable.err |  1 +
>   .../pc-i440fx-acpi-hotplug-bridge-disable.xml | 33 +++++++
>   .../pc-i440fx-acpi-hotplug-bridge-enable.xml  | 33 +++++++
>   .../q35-acpi-hotplug-bridge-disable.args      | 33 +++++++
>   .../q35-acpi-hotplug-bridge-disable.err       |  1 +
>   .../q35-acpi-hotplug-bridge-disable.xml       | 47 ++++++++++
>   .../q35-acpi-hotplug-bridge-enable.xml        | 47 ++++++++++
>   tests/qemuxml2argvtest.c                      | 16 ++++
>   .../pc-i440fx-acpi-hotplug-bridge-disable.xml |  1 +
>   .../pc-i440fx-acpi-hotplug-bridge-enable.xml  |  1 +
>   .../q35-acpi-hotplug-bridge-disable.xml       |  1 +
>   .../q35-acpi-hotplug-bridge-enable.xml        |  1 +
>   tests/qemuxml2xmltest.c                       | 14 +++
>   37 files changed, 515 insertions(+), 1 deletion(-)
>   create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.err
>   create mode 100644 tests/qemuxml2argvdata/aarch64-acpi-hotplug-bridge-disable.xml
>   create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.args
>   create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.err
>   create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
>   create mode 100644 tests/qemuxml2argvdata/pc-i440fx-acpi-hotplug-bridge-enable.xml
>   create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.args
>   create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.err
>   create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-disable.xml
>   create mode 100644 tests/qemuxml2argvdata/q35-acpi-hotplug-bridge-enable.xml
>   create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-disable.xml
>   create mode 120000 tests/qemuxml2xmloutdata/pc-i440fx-acpi-hotplug-bridge-enable.xml
>   create mode 120000 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-disable.xml
>   create mode 120000 tests/qemuxml2xmloutdata/q35-acpi-hotplug-bridge-enable.xml
> 