Re: [PATCH v7 0/4] introduce support for acpi-bridge-hotplug feature
Reviewed-by: Laine Stump
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-...@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 under 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:
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 |
[PATCH v7 0/4] introduce support for acpi-bridge-hotplug feature
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-...@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 under 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:
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 +++
