[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 5/9] i440fx/acpi: do not add hotplug related amls for cold pl
From: |
Igor Mammedov |
Subject: |
Re: [PATCH 5/9] i440fx/acpi: do not add hotplug related amls for cold plugged bridges |
Date: |
Mon, 14 Sep 2020 14:36:46 +0200 |
On Fri, 11 Sep 2020 23:37:51 +0530
Ani Sinha <ani@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
>
> As a result of the patch, the following are the changes to the DSDT ACPI
> table:
>
> @@ -858,38 +858,33 @@
> Return (Zero)
> }
>
> Method (_S2D, 0, NotSerialized) // _S2D: S2 Device State
> {
> Return (Zero)
> }
>
> Method (_S3D, 0, NotSerialized) // _S3D: S3 Device State
> {
> Return (Zero)
> }
> }
>
> Device (S18)
> {
> - Name (_SUN, 0x03) // _SUN: Slot User Number
> Name (_ADR, 0x00030000) // _ADR: Address
> - Method (_EJ0, 1, NotSerialized) // _EJx: Eject Device
> - {
> - PCEJ (BSEL, _SUN)
> - }
> }
>
> Device (S20)
> {
> Name (_SUN, 0x04) // _SUN: Slot User Number
> Name (_ADR, 0x00040000) // _ADR: Address
> Method (_EJ0, 1, NotSerialized) // _EJx: Eject Device
> {
> PCEJ (BSEL, _SUN)
> }
> }
>
> Device (S28)
> {
> Name (_SUN, 0x05) // _SUN: Slot User Number
> Name (_ADR, 0x00050000) // _ADR: Address
> @@ -1148,37 +1143,32 @@
> PCEJ (BSEL, _SUN)
> }
> }
>
> Device (SF8)
> {
> Name (_SUN, 0x1F) // _SUN: Slot User Number
> Name (_ADR, 0x001F0000) // _ADR: Address
> Method (_EJ0, 1, NotSerialized) // _EJx: Eject Device
> {
> PCEJ (BSEL, _SUN)
> }
> }
>
> Method (DVNT, 2, NotSerialized)
> {
> - If ((Arg0 & 0x08))
> - {
> - Notify (S18, Arg1)
> - }
> -
> If ((Arg0 & 0x10))
> {
> Notify (S20, Arg1)
> }
>
> If ((Arg0 & 0x20))
> {
> Notify (S28, Arg1)
> }
>
> If ((Arg0 & 0x40))
> {
> Notify (S30, Arg1)
> }
>
> If ((Arg0 & 0x80))
>
> 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 <ani@anisinha.ca>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/acpi-build.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 7a5a8b3521..e079b686f5 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;
- [PATCH 1/9] tests/acpi: document addition of table DSDT.roothp for unit testing root pci hotplug on/off, Ani Sinha, 2020/09/11
- [PATCH 2/9] tests/acpi: add a new unit test to test hotplug off/on feature on the root pci bus, Ani Sinha, 2020/09/11
- [PATCH 3/9] tests/acpi: add a new ACPI table in order to test root pci hotplug on/off, Ani Sinha, 2020/09/11
- [PATCH 4/9] Fix a gap where acpi_pcihp_find_hotplug_bus() returns a non-hotpluggable bus, Ani Sinha, 2020/09/11
- [PATCH 5/9] i440fx/acpi: do not add hotplug related amls for cold plugged bridges, Ani Sinha, 2020/09/11
- Re: [PATCH 5/9] i440fx/acpi: do not add hotplug related amls for cold plugged bridges,
Igor Mammedov <=
- [PATCH 6/9] tests/acpi: list added acpi table binary file for pci bridge hotplug test, Ani Sinha, 2020/09/11
- [PATCH 7/9] tests/acpi: unit test for 'acpi-pci-hotplug-with-bridge-support' bridge flag, Ani Sinha, 2020/09/11
- [PATCH 8/9] tests/acpi: add newly added acpi DSDT table blob for pci bridge hotplug flag, Ani Sinha, 2020/09/11
- [PATCH 9/9] piix4: don't reserve hw resources when hotplug is off globally, Ani Sinha, 2020/09/11