Re: [PATCH v6 0/4] introduce support for acpi-bridge-hotplug feature

Other than laine doesn't anyone have any feedback on this patch set? Timely reviews help contributions. It's difficult to sit on a patch set and keep rebasing for ever. On Tue, Oct 5, 2021 at 11:21 AM Ani Sinha <ani(a)anisinha.ca&gt; wrote: > changelog: > > 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 | 7 ++ > docs/formatdomain.rst | 11 +++ > 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 | 14 +++ > 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, 503 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 > >