Re: [PATCH v3 3/5] conf: introduce acpi-hotplug-bridge and acpi-root-hotplug pm options

On Tue, Sep 28, 2021 at 11:47:26AM +0100, Daniel P. Berrangé wrote: > On Tue, Sep 28, 2021 at 03:28:04PM +0530, Ani Sinha wrote: > > > > > > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote: > > > > > On Tue, Sep 28, 2021 at 02:35:47PM +0530, Ani Sinha wrote: > > > > > > > > > > > > On Tue, 28 Sep 2021, Daniel P. Berrangé wrote: > > > > > > > > > On Sun, Sep 12, 2021 at 08:56:29AM +0530, Ani Sinha wrote: > > > > > > This change introduces libvirt xml support for the following two pm options: > > > > > > > > > > > > <pm> > > > > > > <acpi-hotplug-bridge enabled='no'/> > > > > > > <acpi-root-hotplug enabled='yes'/> > > > > > > </pm> > > > > > > > > > > > > > > > > +``acpi-hotplug-bridge`` > > > > > > + :since:`Since 7.8.0` This option enables or disables BIOS ACPI based hotplug support > > > > > > + for cold plugged bridges. It is available only for x86 guests, both for q35 and pc > > > > > > + machine types. For pc machines, the support is available from `QEMU 2.12`. For q35 > > > > > > + machines, the support is available from `QEMU 6.1`. Examples of cold plugged bridges > > > > > > + include PCI-PCI bridges for pc machine types (pci-bridge controller). For q35 machines, > > > > > > + it includes PCIE root ports (pcie-root-port controller). This is a global option that > > > > > > + affects all bridges. No other bridge specific option is required to be specified. > > > > > > > > > > Can you confirm my understanding of the situation.. > > > > > > > > > > - i440fx / PCI topology - hotplug always uses ACPI > > > > > > > > > > > > > ACPI is the primary means of enabling hotplug. shpc might also have a role > > > > here but I think it is disabled. Igor (cc'd) might throw some lights on > > > > how shpc comes to play. > > > > > > Yes, I think it will be important to understand if 'shpc' becomes relevant > > > when ACPI hotplug is disabled for PCI > > > > > > > > > > > > - q35 / PCIe topology - hotplug historically used native PCIe hotplug, > > > > > but in 6.1 switched to ACPI > > > > > > > > > > > > > Correct. > > > > > > > > > Given, the name "acpi-hotplug-bridge", am I right that this option > > > > > has *no* effect, if the q35 machine is using native PCIe hotplug > > > > > approach ? > > > > > > > > Its complicated. > > > > With "acpi-hotplug-bridge" ON, native hotplug is disabled in qemu. > > > > With "acpi-hotplug-bridge" OFF, native hotplug is enabled in qemu. > > > > > > Oh, I mis-read and didn't realize this was controlling the QEMU > > > "acpi-pci-hotplug-with-bridge-support" configuration. > > > > > > With this in mind I think the naming is somewhat misleading. Setting it > > > to off would give users the impression that hotplug is disabled, which > > > is not the case for Q35 at least. It is just switching to a different > > > hotplug implementation. > > > > > > At least from Q35 pov, I think it would be better to call it > > > > > > hotplug-mode="acpi|pcie" > > > > > > so it is clear that no matter what value it is set to, hotplug > > > is still available. > > > > > > If we also consider PIIX, then depending on the answer wrt shpc > > > above, we might want one of > > > > > > hotplug-mode="acpi|pcie|none" > > > hotplug-mode="acpi|pcie|shpc" > > > > > > > If libvirt does not deal with shpc today I think we should not bother with > > shpc at all. We should simply have a boolean mode appropriately named that > > choses between acpi hotplug vs native. > > I want to understand what's possible at the qemu hardware level, > so we don't paint ourselves into a corner. > > IIUC, with shpc we only have a toggle on "pci-bridge" devices, > and those currently have shpc=true by default. There's no shpc > setting on the pci-root, and theres no global setting. Opps, I was mislead. They have shpc=false by default due to machine types >= 2.9 overriding it to false

> Seems to imply that if we have acpi-hotplug disabled for PIIX, > then there would be no hotplug on the pci-root, but shpc hotplug > would still be available on any pci-bridge devices ? Regards, Daniel