Ping again On Mon, Sep 20, 2021 at 21:25 Ani Sinha <[email protected]> wrote:
> Ping > > On Mon, Sep 13, 2021 at 1:39 PM Ani Sinha <[email protected]> wrote: > >> >> >> On Sun, 12 Sep 2021, Ani Sinha wrote: >> >> > Hi all: >> > >> > This patchset introduces libvirt xml support for the following two pm >> conf >> > options: >> > >> > <pm> >> > <acpi-hotplug-bridge enabled='no'/> >> > <acpi-root-hotplug enabled='yes'/> >> > </pm> >> > >> >> Another option is to create a new xml tag and add these under that tag. >> Something like: >> >> + <acpi-hotplug> >> + <acpi-hotplug-bridge enabled='no'/> >> + <acpi-root-hotplug enabled='yes'/> >> + </acpi-hotplug> >> >> These are not exactly power management options. So putting them under <pm> >> may not be correct. >> If this is a better approach I will resend the patchset with the changes >> along with addressing other review comments. >> >> >> > The above two options are only available for qemu driver and that too >> for x86 >> > guests only. Both of them are global options. >> > >> > ``acpi-hotplug-bridge`` option enables or disables ACPI hotplug support >> for cold >> > plugged bridges. Examples of cold plugged bridges include PCI-PCI bridge >> > (pci-bridge controller) for pc machines and pcie-root-port controller >> for q35 >> > machines. The corresponding commandline options to qemu for x86 guests >> are: >> > >> > (pc machines): -global >> PIIX4_PM.acpi-pci-hotplug-with-bridge-support=<off/on> >> > (q35 machines): -global >> ICH9-LPC.acpi-pci-hotplug-with-bridge-support=<off/on> >> > >> > Being global options, no other bridge specific options for pci-bridge >> > controller or pcie-root-port controllers 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 changes in qemu.git: >> > >> > 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.git: >> > >> > (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) for bridges are outlined in (b). It is >> possible that >> > some users might still want to use native hotplug on PCIe [1]. >> Therefore, >> > this conf option enables users to choose either ACPI based hotplug or >> native >> > hotplug for cold plugged bridges (for example for pcie root port >> controller >> > in q35 machines). >> > >> > ``acpi-root-hotplug`` option enables or disables ACPI based hotplug for >> PCI root >> > bus (pci-root controller). This option is only available for pc machine >> type. >> > The corresponding commandline option to qemu for x86 guests is: >> > >> > -global PIIX4_PM.acpi-root-pci-hotplug=<off/on> >> > >> > This additional option enables users to disable hotplug for all devices >> in the >> > system without adding an additional PCI-PCI bridge, putting the devices >> behind >> > the bridge and using the existing ``acpi-hotplug-bridge`` option to >> disable >> > hotplug on that bridge. This feature was introduced from qemu version >> 5.2 with >> > the following change in qemu.git: >> > >> > 3d7e78aa7777f ("Introduce a new flag for i440fx to disable PCI hotplug >> on the root bus") >> > >> > The above qemu commit describes some compelling reasons why users might >> to >> > disable hotplug on PCI root buses [2]. >> > >> > A brief summary of the patches: >> > >> > >[PATCH v3 1/5] qemu: capablities: detect presence of >> > >[PATCH v3 2/5] qemu: capablities: detect presence of >> > Patches 1 and 2 implement support for qemu capability checks for the >> above >> > config options. >> > >> > >[PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and >> > Patch 3 actually adds the config option to the schema and adds related >> unit >> > tests. >> > >> > >[PATCH v3 4/5] qemu: command: add support for qemu options that >> > Patch 4 adds the backend qemu commandline support for the options. It >> also adds >> > relevant unit tests for the same. >> > >> > >[PATCH v3 5/5] NEWS: add new acpi pci hotplug options in the release >> > Patch 5 adds the release notes for the next libvirt release. >> > >> > >> > Changelog: >> > v1: initial implementation. Had some bugs and missed some unit tests. >> > v2: fixed bugs and added additional missing unit tests. >> > 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. >> > >> > >> > Notes: >> > >> > [1] 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. >> > >> > [2] The use case scenario described by Laine in >> > >> https://listman.redhat.com/archives/libvir-list/2020-February/msg00110.html >> > intentionally does not discuss i440fx and focusses solely on q35. I do >> realize >> > that redhat has moved on from i440fx and currently efforts for new >> features >> > are concentrated on q35 machines only. We have had some hard debates on >> this >> > on the qemu mailing list before. The fact of the matter is that i440fx >> is >> > not at 1-1 parity with q35. There are many users who are currenly using >> i440fx >> > and are simply not ready to move to q35 without sacrificing some >> > existing features they support today. For example >> > https://wiki.qemu.org/images/4/4e/Q35.pdf lists some of q35 >> limitations. >> > https://www.linux-kvm.org/images/0/06/2012-forum-Q35.pdf provides more >> > information on the differences. Hence we need to solve the issue Laine >> has >> > described in the above email for i440fx without adding additional >> bridges. >> > >> > Further, in Daniel Berrange's words from : >> > https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg03012.html >> > >> > "From the upstream POV, there's been no decision / agreement to phase >> > out PIIX, this is purely a RHEL downstream decision & plan. If other >> > distros / users have a different POV, and find the feature useful, we >> > should accept the patch if it meets the normal QEMU patch requirements. >> > " >> > >> > Also to be noted that I have already experimented this qemu commandline >> option >> > using libvirt passthrough feature as has been documented in >> > >> http://blog.vmsplice.net/2011/04/how-to-pass-qemu-command-line-options.html >> > This was only meant to be a short term solution until libvirt started >> > supporting this natively. Supporting this option through libvirt would >> simplify >> > their use case as well as add capability validations >> > and graceful failure scenarios in case qemu did not support the option. >> > >> > [3] Finally, I implemented support for ``acpi-root-hotplug`` option in >> Qemu. >> > Since adding the support for this option, I have not run away :-) I am >> still >> > around, fixing other issues in the same subsystem in qemu and also now >> I have >> > added myself as a reviewer of patches in this area. I will also be >> trying to >> > support/maintain this new xml conf option in libvirt to the extent I >> can in >> > future with the help of other experienced maintainers. Obviously this >> is all >> > freelance work at this moment and is highly dependent on available free >> time. >> > >> > >> > >> >
