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

Ani Sinha posted 4 patches 2 years, 5 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20211008064215.50480-1-ani@anisinha.ca
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
[PATCH v7 0/4] introduce support for acpi-bridge-hotplug feature
Posted by Ani Sinha 2 years, 5 months ago
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

-- 
2.25.1

Re: [PATCH v7 0/4] introduce support for acpi-bridge-hotplug feature
Posted by Laine Stump 2 years, 5 months ago
Reviewed-by: Laine Stump <lain@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
>