All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
To: Keith Busch <kbusch@kernel.org>
Cc: Danielle Costantino <dcostantino@meta.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Lukas Wunner <lukas@wunner.de>,
	Mahesh J Salgaonkar <mahesh@linux.ibm.com>,
	Oliver O'Halloran <oohall@gmail.com>,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH 1/2] PCI/DPC: Clear Interrupt Status in dpc_reset_link()
Date: Thu, 12 Feb 2026 14:51:46 -0800	[thread overview]
Message-ID: <8b4800cf-7e8a-42f7-a84a-5081afe00048@linux.intel.com> (raw)
In-Reply-To: <aY5QMlr_7XLIbOsV@kbusch-mbp>



On 2/12/2026 2:12 PM, Keith Busch wrote:
> On Thu, Feb 12, 2026 at 01:49:52PM -0800, Kuppuswamy Sathyanarayanan wrote:
>> Hi Keith,
>>
>> On 2/12/2026 1:23 PM, Keith Busch wrote:
>>> On Thu, Feb 12, 2026 at 11:50:54AM -0800, Kuppuswamy Sathyanarayanan wrote:
>>>> In case of EDR, firmware owns the interrupt handling. It just uses ACPI
>>>> method to request OS help with recovery. Since interrupt handling is
>>>> owned by firmware, I think firmware should clear the interrupt status.
>>>
>>> But the PCI Firmware Specication says the OS owns the status register
>>> while it is handling EDR notification
>>>
>>> "
>>>    the operating system is permitted to write the following:
>>>
>>>      * Device Status Register
>>>      * Uncorrectable Error Status Register
>>>      * Correctable Error Status Register
>>>      * Root Error Status Register
>>>      * RP PIO Status Register
>>>
>>>    in the Port that triggered DPC while processing an Error Disconnect Recover
>>>    notification from firmware
>>> "
>>
>> Please check the _OSC DPC control bit details in PCI firmware spec.
>>
>> It specifically calls out OS is permitted to write to DPC trigger status
>> bit in DPC status register. It does not talk about about DPC interrupt
>> status bit.
>>
>> Copied here for reference:
>>
>> "If control of this feature was requested and denied, or was not requested,
>> then the operating system is permitted to write to the DPC Trigger Status bit
>> in the DPC Status Register in the Port that triggered containment"
>>
>> I think since firmware registers interrupt handler for DPC, after OS helps
>> handle the recovery it should complete the loop and clear it.
> 
> But that just creates a window for when after the OS lets the link
> retrain that the port will be unable to trigger a new containment event.

As per EDR flow, firmware waits for _OST reply from OS to complete the
current interrupt handling. After receiving _OST, firmware decides whether
recovery should continue or if the link should be disabled. When/how firmware
handles subsequent DPC events depends on firmware's implementation.


> 
> Sure, there's no explicit language in any spec I can find that the OS
> must write 1 to bit 3 of the status register, but it doesn't say
> firmware owns that bit either. The firmware handed control of the status
> to the OS in this path. It would not make sense to return to firmware in
> a state that makes it impossible to report new errors during that
> transition window.

The spec explicitly lists what the OS can write during the EDR window. For
few registers it gives full contro; For DPC status, it explicitly states
which bit it can clear (DPC trigger status). 

> 
> Besides, is firmware first even triggering off an interrupt? Pretty sure
> it's triggering off the ERR_COR message, no? Why would it need to own
> the Interrupt Status bit when it's not even relying on it?

If it not triggered by interrupt, then interrupt status bit does not
matter (even for OS handler).

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer


  reply	other threads:[~2026-02-12 22:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-12 19:18 [PATCH 0/2] PCI/DPC: Fix EDR recovery path issues Danielle Costantino
2026-02-12 19:18 ` [PATCH 1/2] PCI/DPC: Clear Interrupt Status in dpc_reset_link() Danielle Costantino
2026-02-12 19:50   ` Kuppuswamy Sathyanarayanan
2026-02-12 21:23     ` Keith Busch
2026-02-12 21:49       ` Kuppuswamy Sathyanarayanan
2026-02-12 22:12         ` Keith Busch
2026-02-12 22:51           ` Kuppuswamy Sathyanarayanan [this message]
2026-02-13  1:22             ` Keith Busch
2026-02-13  4:28               ` Sathyanarayanan Kuppuswamy
2026-02-13 14:01                 ` Keith Busch
2026-02-13 17:08                   ` Kuppuswamy Sathyanarayanan
     [not found]               ` <4c0d0575-0da1-49ff-878e-65622b442e98@linux.intel.com>
2026-02-13  4:29                 ` Sathyanarayanan Kuppuswamy
2026-02-12 21:36   ` Lukas Wunner
2026-02-12 21:50     ` Keith Busch
2026-02-12 19:18 ` [PATCH 2/2] PCI/EDR: Defer AER status clearing until after recovery Danielle Costantino

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8b4800cf-7e8a-42f7-a84a-5081afe00048@linux.intel.com \
    --to=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=bhelgaas@google.com \
    --cc=dcostantino@meta.com \
    --cc=kbusch@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=mahesh@linux.ibm.com \
    --cc=oohall@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.