Re: [PATCH v3] PCI: pciehp: Fix hotplug on Catlow Lake with unreliable PME status

[Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>]

Hi Bjorn/Mika,

On 9/11/2026 10:01 AM, Kuppuswamy Sathyanarayanan wrote:
> Hi Mika, Bjorn, Lukas, Rafael,
> 

Gentle ping on my reply below.

Bjorn, could you let me know which of the three directions you would like
me to take for v5?

Mika, could you take a look at the acpi_pci_bridge_d3() findings and let
me know if they match your understanding?

> Apologies for the very long silence on this.  I was out on an extended
> vacation and then got pulled into other work when I got back.  I want to
> spin v5 now, and before that I want to respond to some of the opens.  v4
> was posted about an hour before Bjorn's v3 review landed, so none of that
> discussion was reflected in it.
> 
> Quick recap for anyone paging this back in:
> 
>   On Intel Catlow Lake PCH Root Ports, the Root Status register (PME
>   Status) is not reliably updated when the PME arrives.  The interrupt
>   itself is delivered; IOSF-SB backdoor reads confirm the status bits stay
>   clear, and nothing in BIOS or the kernel is clearing them.  So
>   pcie_pme_irq() sees no PME Status, returns IRQ_NONE, and a hot-add that
>   happens while the port is runtime suspended in D3hot is lost, because
>   pciehp_suspend() has cleared HPIE and is relying on PME to bring the
>   port back.
> 
>   v1 hacked pciehp_isr().  v2 called pm_runtime_disable() in the quirk,
>   which costs ~10% PC6 residency here and, more importantly, doesn't help
>   once the system suspends.  v3/v4 instead set PCI_DEV_FLAGS_PME_UNRELIABLE
>   via a quirk and skip the HPIE clear in pciehp_disable_interrupt(), so the
>   port still goes to D3hot but hotplug events arrive as ordinary hotplug
>   interrupts.  Bjorn's objection to that is that eb34da60edee cleared HPIE
>   for a reason, and we had nothing solid to say why Catlow won't hit the
>   spurious wakeup that motivated it.  Mika's last question was whether the
>   port enters D3cold at all.
> 
> On 4/7/2026 12:08 AM, Mika Westerberg wrote:
>> Hi,
>>
>> On Fri, Apr 03, 2026 at 12:37:39PM -0700, Kuppuswamy Sathyanarayanan wrote:
>>> Hi Mika,
>>>
>>> On 3/27/2026 4:16 AM, Mika Westerberg wrote:
>>>> Hey,
>>>>
>>>> On Thu, Mar 26, 2026 at 02:23:50PM -0700, Kuppuswamy Sathyanarayanan wrote:
>>>>> Hi Mika,
>>>>>
>>>>> On 3/25/2026 11:12 PM, Mika Westerberg wrote:
>>>>>> On Wed, Mar 25, 2026 at 02:12:48PM -0700, Kuppuswamy Sathyanarayanan wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 3/24/2026 11:11 PM, Mika Westerberg wrote:
>>>>>>>> On Tue, Mar 24, 2026 at 02:45:25PM -0700, Kuppuswamy Sathyanarayanan wrote:
>>>>>>>>>> eb34da60edee ("PCI: pciehp: Disable hotplug interrupt during suspend")
>>>>>>>>>> cleared PCI_EXP_SLTCTL_HPIE so that when the link goes down, we
>>>>>>>>>> wouldn't get a PCI_EXP_SLTSTA_DLLSC interrupt and wake the system.
>>>>>>>>>>
>>>>>>>>>> I don't know the details of why the PCI_EXP_SLTSTA_DLLSC would cause
>>>>>>>>>> that wakeup.  I would think pciehp should field that, and it should be
>>>>>>>>>> able to figure out whether to bring the port out of D3hot.
>>>>>>>>>>
>>>>>>>>>> Anyway, with this patch it looks like we'll leave PCI_EXP_SLTCTL_HPIE
>>>>>>>>>> set, and potentially get that PCI_EXP_SLTSTA_DLLSC interrupt again?
>>>>>>>>>
>>>>>>>>> I have tested this patch on Catlow Lake. Enabling HPIE does not result in
>>>>>>>>> spurious wakeups as mentioned in Mika's patch.
>>>>>>>>>
>>>>>>>>> Mika, any comments?
>>>>>>>>
>>>>>>>> What do you have connected to the slot?
>>>>>>>
>>>>>>> A network card.
>>>>>>
>>>>>> Okay.
>>>>>>
>>>>>> Out of interest how do you hotplug it? :)
>>>>>
>>>>> We physically remove and insert the card.
>>>>
>>>> Got it.
>>>>
>>>>>>>> IIRC the interrupt triggers when presence change toggles (due to the link
>>>>>>>> going down).
>>>>>>>>
>>>>>>>
>>>>>>> I have tested the s3 mode. I was able to see message related to system entering
>>>>>>> suspend and then coming back again after (after user intervention). I also noted
>>>>>>> pcie_disable_interrupt() called before suspend and pcie_enable_interrupt() called
>>>>>>> after resume.
>>>>>>
>>>>>> In case of S3 the BIOS also configures the hardware before entering
>>>>>> suspend. On client at least it's suspend-to-idle and any interrupt will
>>>>>> bring the CPU and the system out of it. It could be that that's the reason
>>>>>> you don't see any issue if this is server system and it goes into full S3?
>>>>>>
>>>>>
>>>>> Looking at the kernel logs, the system is actually using suspend-to-idle 
>>>>> (s2idle), not full S3:
>>>>>
>>>>>   PM: suspend entry (s2idle)
>>>>>
>>>>> So this is the same suspend mode where you observed the spurious wakeup issue.
>>>>> Interestingly, we're not seeing the problem on Catlow Lake with HPIE enabled.
>>>>>
>>>>> I am trying to understand the wakeup sequence in your case. IIUC, before the
>>>>> system enters suspend, it will put the device and port in D3hot, right? So link
>>>>> down should happen before the system goes to sleep or idle. At what point does
>>>>> the spurious DLLSC interrupt occur that causes the unwanted wakeup?
>>>>
>>>> I think in case of tunneled PCIe it is presence detect that toggles and
>>>> triggers the interrupt if left enabled.
>>>>
>>>> The flow is something like this (from my memory):
>>>>
>>>> 1. User enters s2idle.
>>>> 2. PM core suspends devices.
>>>> 3. PCI core suspends the devices behind the root port and then the root
>>>>    port itself. This makes the root port be in D3hot and the link below it
>>>>    is still in L1.
>>>> 4. PCI/ACPI turns of the power resource attached to the root port. This
>>>>    puts the link into L2/3 ready and then PERST# is asserted in which case
>>>>    the tunnels are gone and presence detect changes and the link enters L2
>>>>    and the root port enters D3cold.
>>>
>>> Thanks for the detailed explanation. So the spurious wakeup happens when the
>>> power resource is turned off during suspend, which triggers the presence detect
>>> change. Is there a way to detect if a port has this power resource configuration
>>> via ACPI methods? I'm wondering if we could make HPIE disabling conditional on
>>> the presence of this power management setup.
>>
>> Yes the Root Port has _PR3() method but see below.
>>
>>>> In your case does the root port enter D3cold? Does it have power resource?
>>>> Or it stays in D3hot? We should not put any hotplug ports into low power
>>>> states if they don't have HotPlugSupportInD3 property as described here:
>>>
>>> I think it stays in D3hot. I am not very clear about the power resource. is
>>> there a way to check for it? Should I look for power_resources_* in sysfs or
>>> check the ACPI tables directly?
>>>
>>> Regarding your suggestion about HotPlugSupportInD3: would it make sense to modify
>>> the HPIE disable logic to be conditional on the presence of this _DSD property? 
>>
>> We already have the check in acpi_pci_bridge_d3(). In your case that should
>> return false and the port should never enter D3.
> 
> It would return false - but we never get that far.  The affected Port is
> 0000:00:1a.0 (\_SB.PC00.RP25), and it has _PS0/_PS3 and _PRW, but no
> _PR0/_PR3, no _S0W and no HotPlugSupportInD3.  With that, the first block
> wins:
> 
> 	if (adev) {
> 		if (acpi_dev_power_state_for_wake(adev) <= ACPI_STATE_D2)
> 			return false;	<-- no _S0W, so 0xFF; not taken
> 		if (acpi_device_power_manageable(adev))
> 			return true;	<-- taken; _PS0 alone is enough
> 	}
> 
> and the HotPlugSupportInD3 check that would have said no is unreachable.
> So the shortcut added by c6e331312ebf ("PCI/ACPI: Whitelist hotplug ports
> for D3 if power managed by ACPI", 2020) returns true before the
> requirements added by dff6139015dc ("PCI/ACPI: Allow D3 only if Root Port
> can signal and wake from D3", 2022) are ever consulted. 
> 
> Bjorn, I think the same data answers your concern.  In Mika's flow the
> presence-detect toggle comes from PCI/ACPI turning off the Port's power
> resource and asserting PERST#.  With no _PR3, D3cold is never valid, the
> Port only ever reaches D3hot, PERST# is never asserted, and presence detect
> never toggles.  That is a structural difference from the case eb34da60edee
> was written for.
> 
> You also suggested keeping the workaround in pme.c, for instance calling
> pcie_pme_walk_bus() regardless of PCI_EXP_RTSTA_PME on quirked ports.  I
> don't think that works, for the reason Lukas raised.  The Port's interrupt
> is shared (hotplug, bandwidth notification, AER and DPC).  Without PME Status
> as the filter, all of those would walk the bus and resume every device below
> the Port to read its PMCSR, on every interrupt rather than only around
> suspend.
> 
> That leaves the question of which direction to take for v5.  I see three,
> and I would prefer the first.
> 
> 1. Keep the v3/v4 mechanism (quirk plus skipping the HPIE clear) and
>    justify it with the no-Power-Resource, no-D3cold reasoning above, in the
>    commit log and in a comment next to the pciehp change.  
> 
> 2. Go back to the v2 shape and keep the affected Port out of low-power
>    states from the quirk, so PME is never depended on.  Bjorn, I think this
>    is where your simplicity argument actually lands, more so than the pme.c
>    variant above, since it touches no shared code at all.  What stopped me
>    in v2 was not only the power cost.  pciehp_suspend() clears HPIE
>    regardless of runtime PM state, so keeping the Port resumed still loses
>    hot-add across s2idle, whereas option 1 covers both cases.
> 
> 3. Make the HotPlugSupportInD3, _PRW and _S0W requirements apply to
>    power-manageable ports too instead of being short-circuited.  That fixes
>    this Port with no quirk at all, which is what dff6139015dc looks like it
>    was meant to do, but it also takes D3 away from any port with _PS0/_PS3
>    and no HotPlugSupportInD3, so I think it belongs in a separate
>    discussion rather than gating this series.  I do think it is worth doing
>    on its own.  Lukas, Rafael, let me know if you agree it is worth a
>    separate patch.
> 
> Could you let me know which one you would like me to take?  If there are no
> objections I will send v5 with option 1.
> 
> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer