Re: [PATCH V2] Add a new PIIX option to control PCI hot unplugging of devices on non-root buses

From: Ani Sinha <ani.sinha@nutanix.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: "Eduardo Habkost" <ehabkost@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Julia Suvorova" <jusual@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"Aleksandar Markovic" <aleksandar.qemu.devel@gmail.com>,
	"Ani Sinha" <ani@anisinha.ca>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Aurelien Jarno" <aurelien@aurel32.net>,
	"Richard Henderson" <rth@twiddle.net>
Subject: Re: [PATCH V2] Add a new PIIX option to control PCI hot unplugging of devices on non-root buses
Date: Fri, 15 May 2020 12:13:53 +0000	[thread overview]
Message-ID: <7FF83CE8-F25A-4458-80A7-EAA6296EF175@nutanix.com> (raw)
In-Reply-To: <20200513214312.0dfa4752@redhat.com>

> On May 14, 2020, at 1:13 AM, Igor Mammedov <imammedo@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.                                                          
         */

> 
> 
>> (b) Failing above, having a global option to disable all hot plug, including the 32 slots of the root bus would be good. However, this does not give us the flexibility we have with PCIE (that is, to hot plug a  device, we can always plug it to a slot with hot plug enabled).
> 
> sounds fine to me, at least it will address problem.
> Can you post a patch to that effect please?
> /It should disable all AML related to hotplug and related hadware code/

I have tried a patch and sent it.