On Wed, 20 May 2020 12:13:35 -0400 "Michael S. Tsirkin" <m...@redhat.com> wrote:
> 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? What do you mean exactly? > > 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. > > > >
