On Wed, May 20, 2020 at 02:20:12PM +0200, Igor Mammedow wrote: > On Wed, 20 May 2020 07:23:21 -0400 > "Michael S. Tsirkin" <m...@redhat.com> wrote: > > > On Wed, May 20, 2020 at 01:05:47PM +0200, Igor Mammedow wrote: > > > On Wed, 20 May 2020 06:28:37 -0400 > > > "Michael S. Tsirkin" <m...@redhat.com> wrote: > > > > > > > On Wed, May 20, 2020 at 11:56:26AM +0200, Igor Mammedow wrote: > > > > > On Wed, 20 May 2020 05:47:53 -0400 > > > > > "Michael S. Tsirkin" <m...@redhat.com> wrote: > > > > > > > > > > > On Wed, May 20, 2020 at 11:43:54AM +0200, Igor Mammedow > > > > > > wrote: > > > > > > > On Fri, 15 May 2020 12:13:53 +0000 > > > > > > > Ani Sinha <ani.si...@nutanix.com> wrote: > > > > > > > > > > > > > > > > On May 14, 2020, at 1:13 AM, Igor Mammedov > > > > > > > > > <imamm...@redhat.com> wrote: > > > > > > > > >> > > > > > > > > >> > > > > > > > > >>> Will following hack work for you? > > > > > > > > >>> possible permutations > > > > > > > > >>> 1) ACPI hotplug everywhere > > > > > > > > >>> -global PIIX4_PM.acpi-pci-hotplug=on -global > > > > > > > > >>> PIIX4_PM.acpi-pci-hotplug-with-bridge-support=on > > > > > > > > >>> -device pci-bridge,chassis_nr=1,shpc=doesnt_matter > > > > > > > > >>> -device e1000,bus=pci.1,addr=01,id=netdev1 > > > > > > > > >>> > > > > > > > > >>> 2) No hotplug at all > > > > > > > > >>> -global PIIX4_PM.acpi-pci-hotplug=off -global > > > > > > > > >>> PIIX4_PM.acpi-pci-hotplug-with-bridge-support=on > > > > > > > > >>> -device pci-bridge,chassis_nr=1,shpc=off -device > > > > > > > > >>> e1000,bus=pci.1,addr=01,id=netdev1 > > > > > > > > >>> > > > > > > > > >>> -global PIIX4_PM.acpi-pci-hotplug=off -global > > > > > > > > >>> PIIX4_PM.acpi-pci-hotplug-with-bridge-support=off > > > > > > > > >>> -device pci-bridge,chassis_nr=1,shpc=doesnt_matter > > > > > > > > >>> -device e1000,bus=pci.1,addr=01,id=netdev1 > > > > > > > > >> > > > > > > > > >> Given that my patch is not acceptable, I’d prefer the > > > > > > > > >> following in the order of preference: > > > > > > > > >> > > > > > > > > >> (a) Have an option to disable hot ejection of PCI-PCI > > > > > > > > >> bridge so that Windows does not even show this HW in > > > > > > > > >> the “safely remove HW” option. If we can do this then > > > > > > > > >> from OS perspective the GUI options will be same as > > > > > > > > >> what is available with PCIE/q35 - none of the devices > > > > > > > > >> will be hot ejectable if the hot plug option is turned > > > > > > > > >> off from the PCIE slots where devices are plugged > > > > > > > > >> into. I looked at the code. It seems to manipulate > > > > > > > > >> ACPI tables of the empty slots of the root bus where > > > > > > > > >> no devices are attached (see comment "/* add hotplug > > > > > > > > >> slots for non present devices */ “). For cold plugged > > > > > > > > >> bridges, it recurses down to scan the slots of the > > > > > > > > >> bridge. Is it possible to disable hot plug for the > > > > > > > > >> slot to which the bridge is attached? > > > > > > > > > > > > > > > > > > I don't think it's possible to have per slot hotplug on > > > > > > > > > conventional PCI hardware. it's per bridge property. > > > > > > > > > > > > > > > > > > > > > > > > > We add the AMLs per empty slot though. When the pic > > > > > > > > bridge is attached, we do nothing, just recurse into the > > > > > > > > bridge slots. That is what I was asking, if it was > > > > > > > > possible to just disable the AMLs or use some tricks to > > > > > > > > say that this particular slot is not hotpluggable. I am > > > > > > > > not sure why Windows is trying to eject the PCI bridge > > > > > > > > and failing. Maybe something related to this comment? > > > > > > > > > > > > > > > > > > > > > > > > /* When hotplug for bridges is enabled, bridges are > > > > > > > > > > > > > > > > * described in ACPI separately (see > > > > > > > > build_pci_bus_end). > > > > > > > > * In this case they aren't themselves > > > > > > > > hot-pluggable. > > > > > > > > * Hotplugged bridges *are* hot-pluggable. > > > > > > > > */ > > > > > > > > > > > > > > thinking some more on this topic, it seems that with ACPI > > > > > > > hotplug we already have implicit non-hotpluggble slot (slot > > > > > > > with bridge) while the rest are staying hotpluggable. > > > > > > > > > > > > > > So my question is: if it's acceptable to add > > > > > > > 'PCIDevice::hotpluggable" property to all PCI devices so > > > > > > > that user / libvirt could set it to false in case they do > > > > > > > not want coldplugged device be considered as hotpluggable? > > > > > > > (this way other devices could be treated the same way as > > > > > > > bridges) > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > > > > > I think Julia already posted a patch adding this to downstream > > > > > > pcie bridges. Adding this to pci slots sounds like a > > > > > > reasonable thing. > > > > > Question was more about external interface, were we do not have > > > > > ports as separate devices with conventional PCI. The only knob > > > > > we have is a a PCI device, where we have a property to turn > > > > > on/off hotplug. ex: -device e1000,hotpluggable=off > > > > > and if libvirt would be able to use it > > > > > > > > It would make sense but is it practical to add the capability is > > > > added in a generic way to all bridges and hosts? > > > > If not how do users probe for presence of the capability? > > > it probably won't work with native SHPC hotplug (which looks to be > > > incomplete in QEMU anyway), but it should work with ACPI and per > > > port PCIE hotplugs. > > > In case of SHPC, we probably should be able to cleanly error out > > > with 'unsupported' reason if "hotpluggable" conflicts with bridge > > > policy. > > > > "Try it and see if it works" is somewhat problematic from management > > POV since there's a never ending stream of new things they would have > > to try. If this approach is taken, we'd have to try to loop in some > > people from libvirt and see what's their take. > to clarify, we are talking here about bridges to conventional > PCI with native SHPC hotplug semantics wrt mgmt and > potential pcidevice.hotpluggable property. > (the later should work fine in ACPI and PCIE hoptlug cases). > > currently by default pci bridges have property shpc=off, so mgmt > should know that deals with PCI bridge and has to enable SHPC > on bridge explicitly,
Wait a second does that actually affect hotplug with ACPI too? > in which case it could probably be taught that > using conflicting hotpluggable for device attached to bridge and shpc > values is wrong thing. > If that's not it, then I'm not sure what kind of discovery you are > talking about. -- MST