Gentle reminder to kindly review this patch :-) @Julia Suvorova @Michael S. Tsirkin
On Mon, Sep 7, 2020 at 6:47 PM Ani Sinha <a...@anisinha.ca> wrote: > > Julia Michael, > > Can you please provide your inputs on this one and the corresponding unit > tests? > > On Fri, Sep 4, 2020 at 9:40 PM Ani Sinha <a...@anisinha.ca> wrote: >> >> Cold plugged bridges are not hot unpluggable, even when their hotplug >> >> property (acpi-pci-hotplug-with-bridge-support) is turned off. Please see >> >> the function acpi_pcihp_pc_no_hotplug() (thanks Julia). However, with >> >> the current implementaton, windows would try to hot-unplug a pci bridge when >> >> it's hotplug switch is off. This is regardless of whether there are devices >> >> attached to the bridge. This is because we add amls like _EJ0 etc for the >> >> pci slot where the bridge is cold plugged. We have a demo video here: >> >> https://youtu.be/pME2sjyQweo >> >> >> >> In this fix, we identify a cold plugged bridge and for cold plugged bridges, >> >> we do not add the appropriate amls and acpi methods that are used by the OS >> >> to identify a hot-pluggable/unpluggable pci device. After this change, >> Windows >> >> does not show an option to eject the PCI bridge. A demo video is here: >> >> https://youtu.be/kbgej5B9Hgs >> >> >> >> While at it, I have also updated a stale comment. >> >> >> >> This change is tested with a Windows 2012R2 guest image and Windows 2019 >> server >> >> guest image running on Ubuntu 18.04 host. This change is based off of >> upstream >> >> qemu master branch tag v5.1.0. >> >> >> >> Signed-off-by: Ani Sinha <a...@anisinha.ca> >> >> --- >> >> hw/i386/acpi-build.c | 12 ++++++------ >> >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> >> >> changelog: >> >> v3: commit log updates providing more accurate information as received from >> Julia. >> >> v2: cosmetic commit log updates with patch testing information. >> >> v1: initial patch. >> >> >> >> >> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >> >> index b7bcbbbb2a..90b863f4ec 100644 >> >> --- a/hw/i386/acpi-build.c >> >> +++ b/hw/i386/acpi-build.c >> >> @@ -359,6 +359,7 @@ static void build_append_pci_bus_devices(Aml >> *parent_scope, PCIBus *bus, >> >> int slot = PCI_SLOT(i); >> >> bool hotplug_enabled_dev; >> >> bool bridge_in_acpi; >> >> + bool cold_plugged_bridge; >> >> >> >> if (!pdev) { >> >> if (bsel) { /* add hotplug slots for non present devices */ >> >> @@ -380,15 +381,14 @@ static void build_append_pci_bus_devices(Aml >> *parent_scope, PCIBus *bus, >> >> pc = PCI_DEVICE_GET_CLASS(pdev); >> >> dc = DEVICE_GET_CLASS(pdev); >> >> >> >> - /* 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. >> >> + /* >> >> + * Cold plugged bridges aren't themselves hot-pluggable. >> >> * Hotplugged bridges *are* hot-pluggable. >> >> */ >> >> - bridge_in_acpi = pc->is_bridge && pcihp_bridge_en && >> >> - !DEVICE(pdev)->hotplugged; >> >> + cold_plugged_bridge = pc->is_bridge && !DEVICE(pdev)->hotplugged; >> >> + bridge_in_acpi = cold_plugged_bridge && pcihp_bridge_en; >> >> >> >> - hotplug_enabled_dev = bsel && dc->hotpluggable && !bridge_in_acpi; >> >> + hotplug_enabled_dev = bsel && dc->hotpluggable && >> !cold_plugged_bridge; >> >> >> >> if (pc->class_id == PCI_CLASS_BRIDGE_ISA) { >> >> continue; >> >> -- >> >> 2.17.1 >> >> >>