| Message ID | 20181204112048.35378-1-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | PCI: Blacklist power management of Gigabyte X299 DESIGNARE EX PCIe ports | expand |
On Tuesday, December 4, 2018 12:20:48 PM CET Mika Westerberg wrote: > Gigabyte X299 DESIGNARE EX motherboard has one PCIe root port that is > connected to an Alpine Ridge Thunderbolt controller. This port has slot > implemented bit set in the config space but other than that it is not > hotplug capable in the sense we are expecting in Linux (it has > dev->is_hotplug_bridge set to 0): > > 00:1c.4 PCI bridge: Intel Corporation 200 Series PCH PCI Express Root Port #5 > Bus: primary=00, secondary=05, subordinate=46, sec-latency=0 > Memory behind bridge: 78000000-8fffffff [size=384M] > Prefetchable memory behind bridge: 00003800f8000000-00003800ffffffff [size=128M] > ... > Capabilities: [40] Express (v2) Root Port (Slot+), MSI 00 > ... > SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- Surprise- > Slot #8, PowerLimit 25.000W; Interlock- NoCompl+ > SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg- > Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock- > SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet- Interlock- > Changed: MRL- PresDet+ LinkState+ > > This system is using ACPI based hotplug to notify the OS that it needs > to rescan the PCI bus (ACPI hotplug). > > If there is nothing connected in any of the Thunderbolt ports the root > port will not have any runtime PM active children and is thus > automatically runtime suspended pretty soon after boot by PCI PM core. > Now, when a device is connected the BIOS SMI handler responsible for > enumerating newly added devices is not able to find anything because the > port is in D3. > > Prevent this from happening by blacklisting PCI power management of this > particular Gigabyte system. > > Reported-by: Kedar A Dongre <kedar.a.dongre@intel.com> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > I checked booting Windows on the same system and it does not put any of the > PCIe root ports to low power states so there is no issue in Windows. I'm > also quite certain Windows does not have similar blacklist. Well, that only means that Windows uses a different approach to decide whether or not to use PCI PM on PCIe root ports (but we knew that that would be the case upfront, didn't we?). > I wonder if our pci_bridge_d3_possible() heuristics would need to be > refined somehow? At least if this blacklist starts growing. Because Windows uses a different approach here, there will be systems for which Linux will decide to use PCI PM with PCIe root ports and Windows won't (or vice versa). That will cause Linux to use configurations that have not been validated against Windows, so it is likely that some of them will not work. Hence, the need for a blacklist is not a surprise really. So Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> BTW, what version of Windows you have tested?
On Tue, Dec 04, 2018 at 06:55:41PM +0100, Rafael J. Wysocki wrote: > On Tuesday, December 4, 2018 12:20:48 PM CET Mika Westerberg wrote: > > Gigabyte X299 DESIGNARE EX motherboard has one PCIe root port that is > > connected to an Alpine Ridge Thunderbolt controller. This port has slot > > implemented bit set in the config space but other than that it is not > > hotplug capable in the sense we are expecting in Linux (it has > > dev->is_hotplug_bridge set to 0): > > > > 00:1c.4 PCI bridge: Intel Corporation 200 Series PCH PCI Express Root Port #5 > > Bus: primary=00, secondary=05, subordinate=46, sec-latency=0 > > Memory behind bridge: 78000000-8fffffff [size=384M] > > Prefetchable memory behind bridge: 00003800f8000000-00003800ffffffff [size=128M] > > ... > > Capabilities: [40] Express (v2) Root Port (Slot+), MSI 00 > > ... > > SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- Surprise- > > Slot #8, PowerLimit 25.000W; Interlock- NoCompl+ > > SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg- > > Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock- > > SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet- Interlock- > > Changed: MRL- PresDet+ LinkState+ > > > > This system is using ACPI based hotplug to notify the OS that it needs > > to rescan the PCI bus (ACPI hotplug). > > > > If there is nothing connected in any of the Thunderbolt ports the root > > port will not have any runtime PM active children and is thus > > automatically runtime suspended pretty soon after boot by PCI PM core. > > Now, when a device is connected the BIOS SMI handler responsible for > > enumerating newly added devices is not able to find anything because the > > port is in D3. > > > > Prevent this from happening by blacklisting PCI power management of this > > particular Gigabyte system. > > > > Reported-by: Kedar A Dongre <kedar.a.dongre@intel.com> > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > --- > > I checked booting Windows on the same system and it does not put any of the > > PCIe root ports to low power states so there is no issue in Windows. I'm > > also quite certain Windows does not have similar blacklist. > > Well, that only means that Windows uses a different approach to decide whether > or not to use PCI PM on PCIe root ports (but we knew that that would be the > case upfront, didn't we?). Indeed we did. > > I wonder if our pci_bridge_d3_possible() heuristics would need to be > > refined somehow? At least if this blacklist starts growing. > > Because Windows uses a different approach here, there will be systems for > which Linux will decide to use PCI PM with PCIe root ports and Windows > won't (or vice versa). That will cause Linux to use configurations that > have not been validated against Windows, so it is likely that some of them > will not work. Hence, the need for a blacklist is not a surprise really. Fair enough :) > So > > Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Thanks! > BTW, what version of Windows you have tested? I only tested Windows 10 1803.
On Tue, Dec 04, 2018 at 02:20:48PM +0300, Mika Westerberg wrote: > Gigabyte X299 DESIGNARE EX motherboard has one PCIe root port that is > connected to an Alpine Ridge Thunderbolt controller. This port has slot > implemented bit set in the config space but other than that it is not > hotplug capable in the sense we are expecting in Linux (it has > dev->is_hotplug_bridge set to 0): > > 00:1c.4 PCI bridge: Intel Corporation 200 Series PCH PCI Express Root Port #5 > Bus: primary=00, secondary=05, subordinate=46, sec-latency=0 > Memory behind bridge: 78000000-8fffffff [size=384M] > Prefetchable memory behind bridge: 00003800f8000000-00003800ffffffff [size=128M] > ... > Capabilities: [40] Express (v2) Root Port (Slot+), MSI 00 > ... > SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- Surprise- > Slot #8, PowerLimit 25.000W; Interlock- NoCompl+ > SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg- > Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock- > SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet- Interlock- > Changed: MRL- PresDet+ LinkState+ > > This system is using ACPI based hotplug to notify the OS that it needs > to rescan the PCI bus (ACPI hotplug). > > If there is nothing connected in any of the Thunderbolt ports the root > port will not have any runtime PM active children and is thus > automatically runtime suspended pretty soon after boot by PCI PM core. > Now, when a device is connected the BIOS SMI handler responsible for > enumerating newly added devices is not able to find anything because the > port is in D3. > --- > I checked booting Windows on the same system and it does not put any of the > PCIe root ports to low power states so there is no issue in Windows. I'm > also quite certain Windows does not have similar blacklist. > > I wonder if our pci_bridge_d3_possible() heuristics would need to be > refined somehow? At least if this blacklist starts growing. We do blacklist such non-native hotplug ports, but of course only if the Hot-Plug Capable bit is set: /* * Hotplug ports handled by firmware in System Management Mode * may not be put into D3 by the OS (Thunderbolt on non-Macs). */ if (bridge->is_hotplug_bridge && !pciehp_is_native(bridge)) return false; I guess your question boils down to, is there any better way to recognize ports which are handled by the platform firmware? Does acpiphp bind to this port? Thanks, Lukas
On Tue, Dec 04, 2018 at 09:40:49PM +0100, Lukas Wunner wrote: > > I wonder if our pci_bridge_d3_possible() heuristics would need to be > > refined somehow? At least if this blacklist starts growing. > > We do blacklist such non-native hotplug ports, but of course only if > the Hot-Plug Capable bit is set: > > /* > * Hotplug ports handled by firmware in System Management Mode > * may not be put into D3 by the OS (Thunderbolt on non-Macs). > */ > if (bridge->is_hotplug_bridge && !pciehp_is_native(bridge)) > return false; > > I guess your question boils down to, is there any better way to recognize > ports which are handled by the platform firmware? Exactly :) Gigabyte makes lots of motherboards and many of them have USB-C ports so it may be that they use the same BIOS accross them. > Does acpiphp bind to this port? It does, yes but AFAIK it binds to any PCI bus (bridge) if it has an ACPI companion.
On Wed, Dec 05, 2018 at 11:20:34AM +0200, Mika Westerberg wrote: > On Tue, Dec 04, 2018 at 09:40:49PM +0100, Lukas Wunner wrote: > > > I wonder if our pci_bridge_d3_possible() heuristics would need to be > > > refined somehow? At least if this blacklist starts growing. > > > > We do blacklist such non-native hotplug ports, but of course only if > > the Hot-Plug Capable bit is set: > > > > /* > > * Hotplug ports handled by firmware in System Management Mode > > * may not be put into D3 by the OS (Thunderbolt on non-Macs). > > */ > > if (bridge->is_hotplug_bridge && !pciehp_is_native(bridge)) > > return false; > > > > I guess your question boils down to, is there any better way to recognize > > ports which are handled by the platform firmware? > > Does acpiphp bind to this port? > > It does, yes but AFAIK it binds to any PCI bus (bridge) if it has an > ACPI companion. Does the root port have a _RMV and/or _SUN object? We could e.g. disallow runtime PM for any bridge with _RMV present and the HPC bit not set in the Slot Capabilities. This could be checked by extending acpi_pci_bridge_d3() to return either true, false or a "don't know" value. Or by adding another platform_pci_bridge_no_d3() hook. Thanks, Lukas
On Wed, Dec 05, 2018 at 10:48:18AM +0100, Lukas Wunner wrote: > Does the root port have a _RMV and/or _SUN object? We could e.g. > disallow runtime PM for any bridge with _RMV present and the HPC > bit not set in the Slot Capabilities. Unfortunately it does not have either of those methods :/ It pretty much looks like a "normal root port" from OS perspective. Below is the ASL related to the root port (RP05) in question: Device (RP05) { Name (_ADR, 0x001C0004) // _ADR: Address Method (_PRT, 0, NotSerialized) // _PRT: PCI Routing Table { If (PICM) { Return (AG5C) /* \_SB_.AG5C */ } Return (PG5C) /* \_SB_.PG5C */ } } Scope (RP05) { Method (_INI, 0, NotSerialized) // _INI: Initialize { LTRZ = LTR5 /* \LTR5 */ LMSL = PML5 /* \PML5 */ LNSL = PNL5 /* \PNL5 */ OBFZ = OBF5 /* \OBF5 */ } OperationRegion (PXCS, PCI_Config, 0x00, 0x0480) Field (PXCS, AnyAcc, NoLock, Preserve) { VDID, 32, Offset (0x50), L0SE, 1, , 3, LDIS, 1, Offset (0x51), Offset (0x52), , 13, LASX, 1, Offset (0x5A), ABPX, 1, , 2, PDCX, 1, , 2, PDSX, 1, Offset (0x5B), Offset (0x60), Offset (0x62), PSPX, 1, PMEP, 1, Offset (0xA4), D3HT, 2, Offset (0xD8), , 30, HPEX, 1, PMEX, 1, Offset (0xE2), , 2, L23E, 1, L23R, 1, Offset (0x324), , 3, LEDM, 1, Offset (0x420), , 30, DPGE, 1 } Field (PXCS, AnyAcc, NoLock, WriteAsZeros) { Offset (0xDC), , 30, HPSX, 1, PMSX, 1 } Name (LTRV, Package (0x04) { 0x00, 0x00, 0x00, 0x00 }) Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method { If ((Arg0 == ToUUID ("e5c937d0-3553-4d7a-9117-ea4d19c3434d") /* Device Labeling Interface */)) { Switch (ToInteger (Arg2)) { Case (0x00) { Name (OPTS, Buffer (0x02) { 0x00, 0x00 // .. }) CreateBitField (OPTS, 0x00, FUN0) CreateBitField (OPTS, 0x04, FUN4) CreateBitField (OPTS, 0x06, FUN6) CreateBitField (OPTS, 0x08, FUN8) CreateBitField (OPTS, 0x09, FUN9) If ((Arg1 >= 0x02)) { FUN0 = 0x01 If (LTRE) { FUN6 = 0x01 } If (OBFF) { FUN4 = 0x01 } If ((ECR1 == 0x01)) { If ((Arg1 >= 0x03)) { FUN8 = 0x01 FUN9 = 0x01 } } } Return (OPTS) /* \_SB_.PC00.RP05._DSM.OPTS */ } Case (0x04) { If ((Arg1 >= 0x02)) { If (OBFZ) { Return (Buffer (0x10) { /* 0000 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // ........ /* 0008 */ 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00 // ........ }) } Else { Return (Buffer (0x10) { /* 0000 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // ........ /* 0008 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 // ........ }) } } } Case (0x06) { If ((Arg1 >= 0x02)) { If (LTRZ) { If (((LMSL == 0x00) || (LNSL == 0x00))) { If ((PCHS == SPTH)) { LMSL = 0x0846 LNSL = 0x0846 } ElseIf ((PCHS == SPTL)) { LMSL = 0x1003 LNSL = 0x1003 } } LTRV [0x00] = ((LMSL >> 0x0A) & 0x07) LTRV [0x01] = (LMSL & 0x03FF) LTRV [0x02] = ((LNSL >> 0x0A) & 0x07) LTRV [0x03] = (LNSL & 0x03FF) Return (LTRV) /* \_SB_.PC00.RP05.LTRV */ } Else { Return (0x00) } } } Case (0x08) { If ((ECR1 == 0x01)) { If ((Arg1 >= 0x03)) { Return (0x01) } } } Case (0x09) { If ((ECR1 == 0x01)) { If ((Arg1 >= 0x03)) { Return (Package (0x05) { 0xC350, Ones, Ones, 0xC350, Ones }) } } } } } Return (Buffer (0x01) { 0x00 // . }) } Device (PXSX) { Name (_ADR, 0x00) // _ADR: Address Method (_PRW, 0, NotSerialized) // _PRW: Power Resources for Wake { Return (GPRW (0x69, 0x04)) } } Method (HPME, 0, Serialized) { If (((VDID != 0xFFFFFFFF) && (PMSX == 0x01))) { Notify (PXSX, 0x02) // Device Wake PMSX = 0x01 PSPX = 0x01 } } Method (_PRW, 0, NotSerialized) // _PRW: Power Resources for Wake { Return (GPRW (0x69, 0x04)) } }
On Wed, Dec 05, 2018 at 12:40:29PM +0200, Mika Westerberg wrote: > On Wed, Dec 05, 2018 at 10:48:18AM +0100, Lukas Wunner wrote: > > Does the root port have a _RMV and/or _SUN object? We could e.g. > > disallow runtime PM for any bridge with _RMV present and the HPC > > bit not set in the Slot Capabilities. > > Unfortunately it does not have either of those methods :/ It pretty much > looks like a "normal root port" from OS perspective. > > Below is the ASL related to the root port (RP05) in question: The information might be buried in the Device Labeling DSM but I can't make sense of it because I only have access to the PCI Firmware Spec 3.0, which is too old. Thanks, Lukas
On Wed, Dec 05, 2018 at 02:22:14PM +0100, Lukas Wunner wrote: > On Wed, Dec 05, 2018 at 12:40:29PM +0200, Mika Westerberg wrote: > > On Wed, Dec 05, 2018 at 10:48:18AM +0100, Lukas Wunner wrote: > > > Does the root port have a _RMV and/or _SUN object? We could e.g. > > > disallow runtime PM for any bridge with _RMV present and the HPC > > > bit not set in the Slot Capabilities. > > > > Unfortunately it does not have either of those methods :/ It pretty much > > looks like a "normal root port" from OS perspective. > > > > Below is the ASL related to the root port (RP05) in question: > > The information might be buried in the Device Labeling DSM but I can't > make sense of it because I only have access to the PCI Firmware Spec 3.0, > which is too old. Good point. I have 3.2 but only ones in that _DSM are: 4 = PCI Bus Capabilities 6 = LTR Maximum Latency 8 = Avoid power-on Reset Delay Duplication on Sx Resume 9 = Device Readiness Durations Only thing that could be relevant is 4 (PCI Bus Capabilities) but I don't see anything in the spec saying it could be hot plug capable :(
On Tue, Dec 04, 2018 at 02:20:48PM +0300, Mika Westerberg wrote: > Gigabyte X299 DESIGNARE EX motherboard has one PCIe root port that is > connected to an Alpine Ridge Thunderbolt controller. This port has slot > implemented bit set in the config space but other than that it is not > hotplug capable in the sense we are expecting in Linux (it has > dev->is_hotplug_bridge set to 0): I guess we can conclude here that we cannot reliably identify root ports such as this one using ACPI description, which leaves us the blacklist. If there are no objections, can we get this into v4.21? Thanks :)
On Tue, Dec 04, 2018 at 02:20:48PM +0300, Mika Westerberg wrote: > Gigabyte X299 DESIGNARE EX motherboard has one PCIe root port that is > connected to an Alpine Ridge Thunderbolt controller. This port has slot > implemented bit set in the config space but other than that it is not > hotplug capable in the sense we are expecting in Linux (it has > dev->is_hotplug_bridge set to 0): > > 00:1c.4 PCI bridge: Intel Corporation 200 Series PCH PCI Express Root Port #5 > Bus: primary=00, secondary=05, subordinate=46, sec-latency=0 > Memory behind bridge: 78000000-8fffffff [size=384M] > Prefetchable memory behind bridge: 00003800f8000000-00003800ffffffff [size=128M] > ... > Capabilities: [40] Express (v2) Root Port (Slot+), MSI 00 > ... > SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- Surprise- > Slot #8, PowerLimit 25.000W; Interlock- NoCompl+ > SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg- > Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock- > SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet- Interlock- > Changed: MRL- PresDet+ LinkState+ > > This system is using ACPI based hotplug to notify the OS that it needs > to rescan the PCI bus (ACPI hotplug). > > If there is nothing connected in any of the Thunderbolt ports the root > port will not have any runtime PM active children and is thus > automatically runtime suspended pretty soon after boot by PCI PM core. > Now, when a device is connected the BIOS SMI handler responsible for > enumerating newly added devices is not able to find anything because the > port is in D3. Ugh. I don't see how this is a maintainable solution. Are we going to have to just update this blacklist empirically as we get reports of systems that are "broken"? I say "broken" because I don't think we can point to anything here that doesn't conform to the specs, so maybe we tripped over something that *should* be covered in the spec, or maybe we're just not interpreting something correctly. For example, it looks like PCI_EXP_FLAGS_SLOT is set, but Linux basically ignores it. Maybe if PCI_EXP_FLAGS_SLOT is set but we aren't using pciehp, we should assume any hotplug would be handled via acpiphp? And in that case, we should avoid doing anything that would prevent platform firmware from enumerating things below the bridge? > Prevent this from happening by blacklisting PCI power management of this > particular Gigabyte system. > > Reported-by: Kedar A Dongre <kedar.a.dongre@intel.com> Is there a bugzilla or any other URL we could include here to help with future changes in this area? > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > I checked booting Windows on the same system and it does not put any of the > PCIe root ports to low power states so there is no issue in Windows. I'm > also quite certain Windows does not have similar blacklist. > > I wonder if our pci_bridge_d3_possible() heuristics would need to be > refined somehow? At least if this blacklist starts growing. > > drivers/pci/pci.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index c9d8e3c837de..1c6e47522c84 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -2501,6 +2501,23 @@ void pci_config_pm_runtime_put(struct pci_dev *pdev) > pm_runtime_put_sync(parent); > } > > +static const struct dmi_system_id bridge_d3_blacklist[] = { > + { > + /* > + * Gigabyte X299 root port is not marked as hotplug > + * capable which allows Linux to power manage it. > + * However, this confuses the BIOS SMI handler so don't > + * power manage root ports on that system. > + */ > + .ident = "X299 DESIGNARE EX-CF", > + .matches = { > + DMI_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), > + DMI_MATCH(DMI_BOARD_NAME, "X299 DESIGNARE EX-CF"), > + }, > + }, > + { } > +}; > + > /** > * pci_bridge_d3_possible - Is it possible to put the bridge into D3 > * @bridge: Bridge to check > @@ -2546,6 +2563,9 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge) > if (bridge->is_hotplug_bridge) > return false; > > + if (dmi_check_system(bridge_d3_blacklist)) > + return false; > + > /* > * It should be safe to put PCIe ports from 2015 or newer > * to D3. > -- > 2.19.2 >
On Mon, Dec 17, 2018 at 02:28:27PM -0600, Bjorn Helgaas wrote: > On Tue, Dec 04, 2018 at 02:20:48PM +0300, Mika Westerberg wrote: > > Gigabyte X299 DESIGNARE EX motherboard has one PCIe root port that is > > connected to an Alpine Ridge Thunderbolt controller. This port has slot > > implemented bit set in the config space but other than that it is not > > hotplug capable in the sense we are expecting in Linux (it has > > dev->is_hotplug_bridge set to 0): > > > > 00:1c.4 PCI bridge: Intel Corporation 200 Series PCH PCI Express Root Port #5 > > Bus: primary=00, secondary=05, subordinate=46, sec-latency=0 > > Memory behind bridge: 78000000-8fffffff [size=384M] > > Prefetchable memory behind bridge: 00003800f8000000-00003800ffffffff [size=128M] > > ... > > Capabilities: [40] Express (v2) Root Port (Slot+), MSI 00 > > ... > > SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- Surprise- > > Slot #8, PowerLimit 25.000W; Interlock- NoCompl+ > > SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg- > > Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock- > > SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet- Interlock- > > Changed: MRL- PresDet+ LinkState+ > > > > This system is using ACPI based hotplug to notify the OS that it needs > > to rescan the PCI bus (ACPI hotplug). > > > > If there is nothing connected in any of the Thunderbolt ports the root > > port will not have any runtime PM active children and is thus > > automatically runtime suspended pretty soon after boot by PCI PM core. > > Now, when a device is connected the BIOS SMI handler responsible for > > enumerating newly added devices is not able to find anything because the > > port is in D3. > > Ugh. I don't see how this is a maintainable solution. Are we going > to have to just update this blacklist empirically as we get reports of > systems that are "broken"? I was hoping not but for that we would need to have some means to identify these. What you suggest below might be one way to avoid adding the blacklist. > I say "broken" because I don't think we can point to anything here > that doesn't conform to the specs, so maybe we tripped over something > that *should* be covered in the spec, or maybe we're just not > interpreting something correctly. That is indeed possible. > For example, it looks like PCI_EXP_FLAGS_SLOT is set, but Linux > basically ignores it. Maybe if PCI_EXP_FLAGS_SLOT is set but we > aren't using pciehp, we should assume any hotplug would be handled via > acpiphp? And in that case, we should avoid doing anything that would > prevent platform firmware from enumerating things below the bridge? I don't see why that would not work. This could cause "power regression" on some systems but I think that's better than systems that do not work at all. > > Prevent this from happening by blacklisting PCI power management of this > > particular Gigabyte system. > > > > Reported-by: Kedar A Dongre <kedar.a.dongre@intel.com> > > Is there a bugzilla or any other URL we could include here to help with > future changes in this area? No, this was reported internally. I can file one if you think it is helpful.
On Tue, Dec 18, 2018 at 10:55:18AM +0200, Mika Westerberg wrote: > On Mon, Dec 17, 2018 at 02:28:27PM -0600, Bjorn Helgaas wrote: > > On Tue, Dec 04, 2018 at 02:20:48PM +0300, Mika Westerberg wrote: > > > Gigabyte X299 DESIGNARE EX motherboard has one PCIe root port that is > > > connected to an Alpine Ridge Thunderbolt controller. This port has slot > > > implemented bit set in the config space but other than that it is not > > > hotplug capable in the sense we are expecting in Linux (it has > > > dev->is_hotplug_bridge set to 0): > > > > > > 00:1c.4 PCI bridge: Intel Corporation 200 Series PCH PCI Express Root Port #5 > > > Bus: primary=00, secondary=05, subordinate=46, sec-latency=0 > > > Memory behind bridge: 78000000-8fffffff [size=384M] > > > Prefetchable memory behind bridge: 00003800f8000000-00003800ffffffff [size=128M] > > > ... > > > Capabilities: [40] Express (v2) Root Port (Slot+), MSI 00 > > > ... > > > SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- Surprise- > > > Slot #8, PowerLimit 25.000W; Interlock- NoCompl+ > > > SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg- > > > Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock- > > > SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet- Interlock- > > > Changed: MRL- PresDet+ LinkState+ > > > > > > This system is using ACPI based hotplug to notify the OS that it needs > > > to rescan the PCI bus (ACPI hotplug). > > > > > > If there is nothing connected in any of the Thunderbolt ports the root > > > port will not have any runtime PM active children and is thus > > > automatically runtime suspended pretty soon after boot by PCI PM core. > > > Now, when a device is connected the BIOS SMI handler responsible for > > > enumerating newly added devices is not able to find anything because the > > > port is in D3. > > > > Ugh. I don't see how this is a maintainable solution. Are we going > > to have to just update this blacklist empirically as we get reports of > > systems that are "broken"? > > I was hoping not but for that we would need to have some means to > identify these. What you suggest below might be one way to avoid adding > the blacklist. > > > I say "broken" because I don't think we can point to anything here > > that doesn't conform to the specs, so maybe we tripped over something > > that *should* be covered in the spec, or maybe we're just not > > interpreting something correctly. > > That is indeed possible. > > > For example, it looks like PCI_EXP_FLAGS_SLOT is set, but Linux > > basically ignores it. Maybe if PCI_EXP_FLAGS_SLOT is set but we > > aren't using pciehp, we should assume any hotplug would be handled via > > acpiphp? And in that case, we should avoid doing anything that would > > prevent platform firmware from enumerating things below the bridge? > > I don't see why that would not work. This could cause "power regression" > on some systems but I think that's better than systems that do not work > at all. Yeah, I think that would be better, assuming it wouldn't cause a flood of power regressions. I'd even rather have a whitelist of systems where we use acpiphp and it's safe to do power management. > > Is there a bugzilla or any other URL we could include here to help with > > future changes in this area? > > No, this was reported internally. > > I can file one if you think it is helpful. I think a kernel.org bugzilla that archived the "lspci -vv", a dmesg log, and an acpidump might be helpful. Bjorn
On Tue, Dec 18, 2018 at 02:58:50PM -0600, Bjorn Helgaas wrote: > > > For example, it looks like PCI_EXP_FLAGS_SLOT is set, but Linux > > > basically ignores it. Maybe if PCI_EXP_FLAGS_SLOT is set but we > > > aren't using pciehp, we should assume any hotplug would be handled via > > > acpiphp? And in that case, we should avoid doing anything that would > > > prevent platform firmware from enumerating things below the bridge? > > > > I don't see why that would not work. This could cause "power regression" > > on some systems but I think that's better than systems that do not work > > at all. > > Yeah, I think that would be better, assuming it wouldn't cause a flood > of power regressions. I'd even rather have a whitelist of systems > where we use acpiphp and it's safe to do power management. Actually it looks like it would break power management of other components such as xHCI and Thunderbolt controller which are connected to a downstream port that has "Slot implemented" set as well. I have another idea, though. Windows says the Gigabyte system platform role is "Desktop" whereas on another system where Windows does power manage the ports the role is "Mobile". I think this maps directly to ACPI FADT table Preferred_PM_Profile field (there is sysfs attribute /sys/firmware/acpi/pm_profile exposing this as well). I wonder if we could use this information in pci_bridge_d3_possible() so that anything with "Desktop" profile returns false when native PCIe hotplug is not used? > > > Is there a bugzilla or any other URL we could include here to help with > > > future changes in this area? > > > > No, this was reported internally. > > > > I can file one if you think it is helpful. > > I think a kernel.org bugzilla that archived the "lspci -vv", a dmesg > log, and an acpidump might be helpful. Here it is: https://bugzilla.kernel.org/show_bug.cgi?id=202031
On Wed, Dec 19, 2018 at 03:23:24PM +0200, Mika Westerberg wrote: > On Tue, Dec 18, 2018 at 02:58:50PM -0600, Bjorn Helgaas wrote: > > > > For example, it looks like PCI_EXP_FLAGS_SLOT is set, but Linux > > > > basically ignores it. Maybe if PCI_EXP_FLAGS_SLOT is set but we > > > > aren't using pciehp, we should assume any hotplug would be handled via > > > > acpiphp? And in that case, we should avoid doing anything that would > > > > prevent platform firmware from enumerating things below the bridge? > > > > > > I don't see why that would not work. This could cause "power regression" > > > on some systems but I think that's better than systems that do not work > > > at all. > > > > Yeah, I think that would be better, assuming it wouldn't cause a flood > > of power regressions. I'd even rather have a whitelist of systems > > where we use acpiphp and it's safe to do power management. > > Actually it looks like it would break power management of other > components such as xHCI and Thunderbolt controller which are connected > to a downstream port that has "Slot implemented" set as well. To be precise, I think you mean that if we avoided power management on ports with "Slot Implemented", ports leading to xHCI and Thunderbolt would consume more power but would work correctly, right? And the theory is that those ports work even if the OS puts them into D3 because the firmware is smart enough to wake them up before poking things below them? Doesn't that make the port's power state out of sync with what the OS thinks it is? > I have another idea, though. Windows says the Gigabyte system platform > role is "Desktop" whereas on another system where Windows does power > manage the ports the role is "Mobile". I think this maps directly to > ACPI FADT table Preferred_PM_Profile field (there is sysfs attribute > /sys/firmware/acpi/pm_profile exposing this as well). > > I wonder if we could use this information in pci_bridge_d3_possible() so > that anything with "Desktop" profile returns false when native PCIe > hotplug is not used? Hmmmm. I guess it's plausible that Windows might be more aggressive about power management for "Mobile" roles as opposed to "Desktop". But there's not really a logical connection to this situation (PCI hotplug is a rare, non-latency sensitive event, so why wouldn't we save power on the Desktop as well?), so it feels like a heuristic that might coincidentally work sometimes but is liable to break at others. Popping back up to the top of the stack, what's the situation on other systems? On this system, PCI_EXP_SLTCAP_HPC is not set. Do other systems have that set but clear OSC_PCI_EXPRESS_NATIVE_HP_CONTROL so we don't use pciehp? Should this be some sort of quirk? I guess that's morally equivalent to the blacklist. But maybe it would be a more direct hint to BIOS writers that this is a defect? > > I think a kernel.org bugzilla that archived the "lspci -vv", a dmesg > > log, and an acpidump might be helpful. > > Here it is: > > https://bugzilla.kernel.org/show_bug.cgi?id=202031 Thanks! Bjorn
On Wed, Dec 19, 2018 at 08:45:19AM -0600, Bjorn Helgaas wrote: > On Wed, Dec 19, 2018 at 03:23:24PM +0200, Mika Westerberg wrote: > > On Tue, Dec 18, 2018 at 02:58:50PM -0600, Bjorn Helgaas wrote: > > > > > For example, it looks like PCI_EXP_FLAGS_SLOT is set, but Linux > > > > > basically ignores it. Maybe if PCI_EXP_FLAGS_SLOT is set but we > > > > > aren't using pciehp, we should assume any hotplug would be handled via > > > > > acpiphp? And in that case, we should avoid doing anything that would > > > > > prevent platform firmware from enumerating things below the bridge? > > > > > > > > I don't see why that would not work. This could cause "power regression" > > > > on some systems but I think that's better than systems that do not work > > > > at all. > > > > > > Yeah, I think that would be better, assuming it wouldn't cause a flood > > > of power regressions. I'd even rather have a whitelist of systems > > > where we use acpiphp and it's safe to do power management. > > > > Actually it looks like it would break power management of other > > components such as xHCI and Thunderbolt controller which are connected > > to a downstream port that has "Slot implemented" set as well. > > To be precise, I think you mean that if we avoided power management on > ports with "Slot Implemented", ports leading to xHCI and Thunderbolt > would consume more power but would work correctly, right? And the > theory is that those ports work even if the OS puts them into D3 > because the firmware is smart enough to wake them up before poking > things below them? Doesn't that make the port's power state out of > sync with what the OS thinks it is? I think better example where this fails is normal Thunderbolt device (not host) which includes PCIe switch and there is an PCIe endpoint, say network interface connected to one of the downstream ports. That downstream port has "Slot implemented" set but is not hotplug capable. So the device would work correctly but if you take the recent "Runtime D3, RTD3" system such as Lenovo Carbon X1 6th gen it keeps the whole PCIe hierarchy from entering D3cold. I would rather not to break that ;-) > > I have another idea, though. Windows says the Gigabyte system platform > > role is "Desktop" whereas on another system where Windows does power > > manage the ports the role is "Mobile". I think this maps directly to > > ACPI FADT table Preferred_PM_Profile field (there is sysfs attribute > > /sys/firmware/acpi/pm_profile exposing this as well). > > > > I wonder if we could use this information in pci_bridge_d3_possible() so > > that anything with "Desktop" profile returns false when native PCIe > > hotplug is not used? > > Hmmmm. I guess it's plausible that Windows might be more aggressive > about power management for "Mobile" roles as opposed to "Desktop". > But there's not really a logical connection to this situation (PCI > hotplug is a rare, non-latency sensitive event, so why wouldn't we > save power on the Desktop as well?), so it feels like a heuristic that > might coincidentally work sometimes but is liable to break at others. OK. > Popping back up to the top of the stack, what's the situation on other > systems? On this system, PCI_EXP_SLTCAP_HPC is not set. Do other systems > have that set but clear OSC_PCI_EXPRESS_NATIVE_HP_CONTROL so we don't use > pciehp? Should this be some sort of quirk? I guess that's morally > equivalent to the blacklist. But maybe it would be a more direct hint to > BIOS writers that this is a defect? Yes, typically these systems have PCI_EXP_SLTCAP_HPC set but BIOS does not allow "PCIeHotplug" in _OSC which prevents pciehp from controlling them. I'm not sure if this is actually a defect in BIOS because again this works fine in Windows and I don't see in any specs (ACPI/PCIe) saying you cannot do this. By a quirk you mean add DMI based quirk somewhere in drivers/pci/quirks.c which makes this root port to have ->is_hotplug_bridge set?
On Wed, Dec 19, 2018 at 05:15:58PM +0200, Mika Westerberg wrote: > On Wed, Dec 19, 2018 at 08:45:19AM -0600, Bjorn Helgaas wrote: > > On Wed, Dec 19, 2018 at 03:23:24PM +0200, Mika Westerberg wrote: > > > On Tue, Dec 18, 2018 at 02:58:50PM -0600, Bjorn Helgaas wrote: > > > > > > For example, it looks like PCI_EXP_FLAGS_SLOT is set, but Linux > > > > > > basically ignores it. Maybe if PCI_EXP_FLAGS_SLOT is set but we > > > > > > aren't using pciehp, we should assume any hotplug would be handled via > > > > > > acpiphp? And in that case, we should avoid doing anything that would > > > > > > prevent platform firmware from enumerating things below the bridge? > > > > > > Actually it looks like it would break power management of other > > > components such as xHCI and Thunderbolt controller which are connected > > > to a downstream port that has "Slot implemented" set as well. > > > > To be precise, I think you mean that if we avoided power management on > > ports with "Slot Implemented", ports leading to xHCI and Thunderbolt > > would consume more power but would work correctly, right? And the > > theory is that those ports work even if the OS puts them into D3 > > because the firmware is smart enough to wake them up before poking > > things below them? Doesn't that make the port's power state out of > > sync with what the OS thinks it is? > > I think better example where this fails is normal Thunderbolt device > (not host) which includes PCIe switch and there is an PCIe endpoint, say > network interface connected to one of the downstream ports. That > downstream port has "Slot implemented" set but is not hotplug capable. > > So the device would work correctly but if you take the recent "Runtime > D3, RTD3" system such as Lenovo Carbon X1 6th gen it keeps the whole > PCIe hierarchy from entering D3cold. I would rather not to break that ;-) Yeah but as you say, those are Downstream Ports. What if you constrain it to Root Ports? Thanks, Lukas
On Wed, Dec 19, 2018 at 06:09:15PM +0100, Lukas Wunner wrote: > > I think better example where this fails is normal Thunderbolt device > > (not host) which includes PCIe switch and there is an PCIe endpoint, say > > network interface connected to one of the downstream ports. That > > downstream port has "Slot implemented" set but is not hotplug capable. > > > > So the device would work correctly but if you take the recent "Runtime > > D3, RTD3" system such as Lenovo Carbon X1 6th gen it keeps the whole > > PCIe hierarchy from entering D3cold. I would rather not to break that ;-) > > Yeah but as you say, those are Downstream Ports. What if you constrain > it to Root Ports? Yes, that could work. I did not test it yet, though. Thinking this bit further maybe we can contstrain it to ports that have slot implemented set and have an ACPI companion instead of just root ports? Point being that ports without ACPI companion could not possibly get ACPI Notify() either.
On Thu, Dec 20, 2018 at 11:06 AM Mika Westerberg <mika.westerberg@linux.intel.com> wrote: > > On Wed, Dec 19, 2018 at 06:09:15PM +0100, Lukas Wunner wrote: > > > I think better example where this fails is normal Thunderbolt device > > > (not host) which includes PCIe switch and there is an PCIe endpoint, say > > > network interface connected to one of the downstream ports. That > > > downstream port has "Slot implemented" set but is not hotplug capable. > > > > > > So the device would work correctly but if you take the recent "Runtime > > > D3, RTD3" system such as Lenovo Carbon X1 6th gen it keeps the whole > > > PCIe hierarchy from entering D3cold. I would rather not to break that ;-) > > > > Yeah but as you say, those are Downstream Ports. What if you constrain > > it to Root Ports? > > Yes, that could work. I did not test it yet, though. Thinking this bit > further maybe we can contstrain it to ports that have slot implemented > set and have an ACPI companion instead of just root ports? Point being > that ports without ACPI companion could not possibly get ACPI Notify() > either. I like this idea. We can basically assume, at least to begin with, that ACPI companions of PCIe ports are there for a reason. That reason very well may be to provide a way to handle notifications.
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index c9d8e3c837de..1c6e47522c84 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -2501,6 +2501,23 @@ void pci_config_pm_runtime_put(struct pci_dev *pdev) pm_runtime_put_sync(parent); } +static const struct dmi_system_id bridge_d3_blacklist[] = { + { + /* + * Gigabyte X299 root port is not marked as hotplug + * capable which allows Linux to power manage it. + * However, this confuses the BIOS SMI handler so don't + * power manage root ports on that system. + */ + .ident = "X299 DESIGNARE EX-CF", + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, "Gigabyte Technology Co., Ltd."), + DMI_MATCH(DMI_BOARD_NAME, "X299 DESIGNARE EX-CF"), + }, + }, + { } +}; + /** * pci_bridge_d3_possible - Is it possible to put the bridge into D3 * @bridge: Bridge to check @@ -2546,6 +2563,9 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge) if (bridge->is_hotplug_bridge) return false; + if (dmi_check_system(bridge_d3_blacklist)) + return false; + /* * It should be safe to put PCIe ports from 2015 or newer * to D3.
Gigabyte X299 DESIGNARE EX motherboard has one PCIe root port that is connected to an Alpine Ridge Thunderbolt controller. This port has slot implemented bit set in the config space but other than that it is not hotplug capable in the sense we are expecting in Linux (it has dev->is_hotplug_bridge set to 0): 00:1c.4 PCI bridge: Intel Corporation 200 Series PCH PCI Express Root Port #5 Bus: primary=00, secondary=05, subordinate=46, sec-latency=0 Memory behind bridge: 78000000-8fffffff [size=384M] Prefetchable memory behind bridge: 00003800f8000000-00003800ffffffff [size=128M] ... Capabilities: [40] Express (v2) Root Port (Slot+), MSI 00 ... SltCap: AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- Surprise- Slot #8, PowerLimit 25.000W; Interlock- NoCompl+ SltCtl: Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg- Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock- SltSta: Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet- Interlock- Changed: MRL- PresDet+ LinkState+ This system is using ACPI based hotplug to notify the OS that it needs to rescan the PCI bus (ACPI hotplug). If there is nothing connected in any of the Thunderbolt ports the root port will not have any runtime PM active children and is thus automatically runtime suspended pretty soon after boot by PCI PM core. Now, when a device is connected the BIOS SMI handler responsible for enumerating newly added devices is not able to find anything because the port is in D3. Prevent this from happening by blacklisting PCI power management of this particular Gigabyte system. Reported-by: Kedar A Dongre <kedar.a.dongre@intel.com> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- I checked booting Windows on the same system and it does not put any of the PCIe root ports to low power states so there is no issue in Windows. I'm also quite certain Windows does not have similar blacklist. I wonder if our pci_bridge_d3_possible() heuristics would need to be refined somehow? At least if this blacklist starts growing. drivers/pci/pci.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)