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

On Tue, 28 Sep 2021 11:47:26 +0100 Daniel P. Berrangé <berrange(a)redhat.com&gt; 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: [...] > > > > The <target hotplug="on/off"> switch in libvirt for pcie-root-ports > > > > currently does not care whether native or acpi hotplug is used. It simply > > > > turns on the hotplug for that particular port. Whether ACPI or native is > > > > used is controlled by this global flag that Julia has introduced in 6.1. > > > Right so we have > > > *1*) following applies to piix4/q35: * ACPI hotplug when enabled, affects _only_ cold-plugged 'bridges' since it requires 'slots' being described in DSDT table which in current impl. is static table built at reset time. (i.e. built-in or 'bridges' specified on command line, where 'bridges' could be PCI-PCI or PCIe-PCI or root/downstream-ports') for anything else ('bridges' added with device_add) native hotplug is in use (whether it's SHPC or PCI-E native). ACPI hotplug wiring is done by calling qbus_set_hotplug_handler() * for root bus piix4_pm_realize()/ich9_pm_init() * for anything else acpi_pcihp_device_plug_cb() > > > * PIIX4 > > > > > > - acpi-root-pci-hotplug=bool > > > > > > Whether hotplug is enabled for the root bridge or not > > > > > > <target hotplug="on/off"> for pci-root controller > > > > > > > > > - acpi-pci-hotplug-with-bridge-support=bool > > > > > > Toggles support for ACPI based hotplug across all bridges. > > > If disabled will there will be no hotplug at all for PIIX4 ? > > > Or does 'shpc' come into play in that scenario ? 'SHPC' hotplug kicks in if it's enabled. (defaults to 'on' except 2.9 machine type) on q35/APCI side of things we always advertise -all_ hotplug methods available build_q35_osc_method() /* * Always allow native PME, AER (no dependencies) * Allow SHPC (PCI bridges can have SHPC controller) */ aml_append(if_ctx, aml_and(a_ctrl, aml_int(0x1F), a_ctrl)); bits 0, 1 are Native PCI-E hotplug and SHPC respectively for PIIX4 we don't have _OSC so it's up to guest OS to make up supported methods. In order of preference: * Windows supports ACPI hotplug then Native PCI-E (SHPC never worked there)

Stupid question. If both native and acpi is enabled which one does OS chose? Does this choice vary between OSes and different flavours of the same OS like Windows?

* Linux supports ACPI hotplug, SHPC, Native PCI-E (SHPC worked poorly due to need to reserve IO for bridges io reservation hinting (impl. later by Marcel)) > > > PIIX combinations > > > > > > (1) acpi-root-pci-hotplug=yes > > > acpi-pci-hotplug-with-bridge-support=yes > > > > > > - All bridges have hotplug > > > > > > (2) acpi-root-pci-hotplug=yes > > > acpi-pci-hotplug-with-bridge-support=no > > > > > > - No bridges have hotplug > > > > > > (3) acpi-root-pci-hotplug=no > > > acpi-pci-hotplug-with-bridge-support=yes > > > > > > - All bridges except root have hotplug requested by Promox guys, to battle against Windows 'feature' that lets any user to unplug sole NIC using an icon on taskbar.

(Laine mentioned we have similar per port control for PCI-E ('hotplug' property) that was requested by other users probably for the same reason) so acpi-root-pci-hotplug is similar to pcie-root-port.hotplug with a difference that the former applies to whole root bus on PIIX4, where the later could be controlled per root port. > > > (4) acpi-root-pci-hotplug=no > > > acpi-pci-hotplug-with-bridge-support=no > > > > > > - No bridges have hotplug. Essentially identical to (2) > > > > no (4) is not identical to (2). In (4) no hotplug is enabled. In (2) pci > > root bus still has hotplug enabled. > > So you're saying that acpi-root-pci-hotplug=yes overrides the > global request acpi-pci-hotplug-with-bridge-support=no and > turns ACPI hotplug back on for the pcie-root historically ACPI hotplug on root bus was always supported without any option, i.e. acpi-root-pci-hotplug=yes by default. acpi-pci-hotplug-with-bridge-support does what its name claims - i.e. adds hotplug for bridges (at least on PIIX4). > > > * Q35 clarification [*1*] still applies > > > > > > > > > - acpi-pci-hotplug-with-bridge-support=bool > > > > > > Toggles support for ACPI based hotplug. If disabled native > > > PCIe hotplug is activated instead > > > > > > > > > * pcie-root-port > > > > > > - hotplug=bool > > > > > > Toggles whether hotplug is enabled on the port. Affects > > > either native and ACPI based hotplug, depending on what > > > acpi-pci-hotplug-with-bridge-support=bool is set to ? > > > > > > <target hotplug="on/off"> for pcie-root-port controller > > > > > > - native_hotplug=bool > > > > > > Can be used to also enable native hotplug, even when ACPI > > > based hotplug is globally enabled. IOW only really relevant > > > when acpi-pci-hotplug-with-bridge-support=true > > > > > > > > > Q35 combinations > > > > > > (1) acpi-pci-hotplug-with-bridge-support=yes name is a bit confusing depending on the point of view, but if one thinks about root/downstream-ports as bridges it fits just fine. > > > pcie-root-port.hotplug=yes > > > pcie-root-port.native_hotplug=yes it probably needs to say here what defaults are in place currently. maybe say it once above (1) for all Q35 section > > > ports have both ACPI + native hotplug, guest prefers native are you sure about 'prefers'? > > > > > > > > > (2) acpi-pci-hotplug-with-bridge-support=yes > > > pcie-root-port.hotplug=no > > > pcie-root-port.native_hotplug=yes seems as invalid config, we should error out > > > ports have no hotplug, despite native_hotplug=yes IIUC > > > > > > > > > (3) acpi-pci-hotplug-with-bridge-support=yes > > > pcie-root-port.hotplug=yes > > > pcie-root-port.native_hotplug=no > > > > > > ports have ACPI hotplug only > > > > > > > > > (4) acpi-pci-hotplug-with-bridge-support=yes > > > pcie-root-port.hotplug=no > > > pcie-root-port.native_hotplug=no > > > > > > ports have no hotplug > > > > > > > > > (5) acpi-pci-hotplug-with-bridge-support=no > > > pcie-root-port.hotplug=yes > > > pcie-root-port.native_hotplug=yes > > > > > > ports have native hotplug > > > > > > > > > (6) acpi-pci-hotplug-with-bridge-support=no > > > pcie-root-port.hotplug=no > > > pcie-root-port.native_hotplug=yes ditto > > > > > > ports have no hotplug, despite native_hotplug=yes IIUC > > > > > > > > > (7) acpi-pci-hotplug-with-bridge-support=no > > > pcie-root-port.hotplug=yes > > > pcie-root-port.native_hotplug=no > > > > > > ports have no hotplug, despite hotplug=yes IIUC looks wrong too, but harder to check without picking into global acpi-pci-hotplug-with-bridge-support from 'random' 'port' device. not sure what to do here > > > > > > (8) acpi-pci-hotplug-with-bridge-support=no > > > pcie-root-port.hotplug=no > > > pcie-root-port.native_hotplug=no > > > > > > ports have no hotplug Ani, it would be nice to have a similar doc in QEMU, lets say docs/pic_hoptplug.rst and maybe move there relevant sections from pcie.txt/pcie_pci_bridge.txt to have everything PCI hotplug related in one place.