diff mbox series

[PULL,46/56] pcihp: acpi: ignore coldplugged bridges when composing hotpluggable slots

Message ID 20230130201810.11518-47-mst@redhat.com (mailing list archive)
State New, archived
Headers show
Series [PULL,01/56] shpc: disallow unplug when power indicator is blinking | expand

Commit Message

Michael S. Tsirkin Jan. 30, 2023, 8:21 p.m. UTC
From: Igor Mammedov <imammedo@redhat.com>

coldplugged bridges are not unpluggable, so there is no need
to describe slots where they are plugged as hotpluggable. To
that effect we have a condition that marks slot as non-hotpluggable
if it's populated by coldplugged bridge and prevents generation
_SUN/_EJ0 objects for it. That leaves dynamic _DSM method on
such slot (which also depends on BSEL and pcihp hardware).
This _DSM method provides only dynamic acpi-index support so far,
which is not actually used/supported by linux kernel for bridges
and it's doubtful there will be need for it at all.

So it's rather pointless to generate acpi-index related AML
for bridges and we can simplify hotplug slots generator a bit
more by completely ignoring coldplugged bridges on hotplug path.

Another point in favor of dropping dynamic _DSM support, is
that we can replace it with static _DSM if necessary since
a slot with bridge can't change during VM runtime and without
any dependency on ACPI PCI hotplug at that.
Later I plan to implement bridge specific static _DSM
   PCI Firmware Specification 3.2
   4.6.5.  _DSM for Ignoring PCI Boot Configurations
part of spec, to fix longstanding issue with fixed IO/MEM
resource assignment that often leads to hotplugged device
being in-operational within the guest due limited IO/MEM
windows programmed on bridge at boot time.

Expected change when coldplugged bridge is ignored by hotplug
code, should look like:
-            Scope (S18)
-            {
-                Name (ASUN, 0x03)
-                Method (_DSM, 4, Serialized)  // _DSM: Device-Specific Method
-                {
-                    Local0 = Package (0x02)
-                        {
-                            BSEL,
-                            ASUN
-                        }
-                    Return (PDSM (Arg0, Arg1, Arg2, Arg3, Local0))
-                }
-            }

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20230112140312.3096331-37-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/acpi-build.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
diff mbox series

Patch

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 2077efbee4..a02608c215 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -409,8 +409,11 @@  static bool is_devfn_ignored_generic(const int devfn, const PCIBus *bus)
 
 static bool is_devfn_ignored_hotplug(const int devfn, const PCIBus *bus)
 {
-    if (bus->devices[devfn]) {
-        return is_devfn_ignored_generic(devfn, bus);
+    PCIDevice *pdev = bus->devices[devfn];
+    if (pdev) {
+        return is_devfn_ignored_generic(devfn, bus) ||
+               /* Cold plugged bridges aren't themselves hot-pluggable */
+               (IS_PCI_BRIDGE(pdev) && !DEVICE(pdev)->hotplugged);
     } else { /* non populated slots */
          /*
          * hotplug is supported only for non-multifunction device
@@ -445,14 +448,7 @@  static void build_append_pcihp_slots(Aml *parent_scope, PCIBus *bus,
         }
 
         if (pdev) {
-            /*
-             * Cold plugged bridges aren't themselves hot-pluggable.
-             * Hotplugged bridges *are* hot-pluggable.
-             */
-            bool cold_plugged_bridge = IS_PCI_BRIDGE(pdev) &&
-                                  !DEVICE(pdev)->hotplugged;
-            hotpluggbale_slot = DEVICE_GET_CLASS(pdev)->hotpluggable &&
-                                !cold_plugged_bridge;
+            hotpluggbale_slot = DEVICE_GET_CLASS(pdev)->hotpluggable;
             dev = aml_scope("S%.02X", devfn);
         } else {
             dev = aml_device("S%.02X", devfn);