[PATCH v1] PCI/EDR: Clear PCIe Device Status errors after EDR error recovery

[PATCH v1] PCI/EDR: Clear PCIe Device Status errors after EDR error recovery

[Kuppuswamy Sathyanarayanan]

Commit 068c29a248b6 ("PCI/ERR: Clear PCIe Device Status errors only if
OS owns AER") adds support to clear error status in the Device Status
Register(DEVSTA) only if OS owns the AER support. But this change
breaks the requirement of the EDR feature which requires OS to cleanup
the error registers even if firmware owns the control of AER support.

More details about this requirement can be found in PCIe Firmware
specification v3.3, Table 4-6 Interpretation of the _OSC Control Field.
If the OS supports the Error Disconnect Recover (EDR) feature and
firmware sends the EDR event, then during the EDR recovery window, OS
is responsible for the device error recovery and holds the ownership of
the following error registers.

• Device Status Register
• Uncorrectable Error Status Register
• Correctable Error Status Register
• Root Error Status Register
• RP PIO Status Register

So call pcie_clear_device_status() in edr_handle_event() if the error
recovery is successful.

Reported-by: Tsaur Erwin <erwin.tsaur@intel.com>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/pci/pcie/edr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/pcie/edr.c b/drivers/pci/pcie/edr.c
index a6b9b479b97a..33d7776ea748 100644
--- a/drivers/pci/pcie/edr.c
+++ b/drivers/pci/pcie/edr.c
@@ -193,6 +193,7 @@ static void edr_handle_event(acpi_handle handle, u32 event, void *data)
 	 */
 	if (estate == PCI_ERS_RESULT_RECOVERED) {
 		pci_dbg(edev, "DPC port successfully recovered\n");
+		pcie_clear_device_status(dev);
 		acpi_send_edr_status(pdev, edev, EDR_OST_SUCCESS);
 	} else {
 		pci_dbg(edev, "DPC port recovery failed\n");
-- 
2.25.1

Re: [PATCH v1] PCI/EDR: Clear PCIe Device Status errors after EDR error recovery

[Sathyanarayanan Kuppuswamy]

Hi Bjorn,

Gentle ping!

On 2/15/23 12:05 PM, Kuppuswamy Sathyanarayanan wrote:
> Commit 068c29a248b6 ("PCI/ERR: Clear PCIe Device Status errors only if
> OS owns AER") adds support to clear error status in the Device Status
> Register(DEVSTA) only if OS owns the AER support. But this change
> breaks the requirement of the EDR feature which requires OS to cleanup
> the error registers even if firmware owns the control of AER support.
> 
> More details about this requirement can be found in PCIe Firmware
> specification v3.3, Table 4-6 Interpretation of the _OSC Control Field.
> If the OS supports the Error Disconnect Recover (EDR) feature and
> firmware sends the EDR event, then during the EDR recovery window, OS
> is responsible for the device error recovery and holds the ownership of
> the following error registers.
> 
> • Device Status Register
> • Uncorrectable Error Status Register
> • Correctable Error Status Register
> • Root Error Status Register
> • RP PIO Status Register
> 
> So call pcie_clear_device_status() in edr_handle_event() if the error
> recovery is successful.

Any comments on this patch?

> 
> Reported-by: Tsaur Erwin <erwin.tsaur@intel.com>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> ---
>  drivers/pci/pcie/edr.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/pcie/edr.c b/drivers/pci/pcie/edr.c
> index a6b9b479b97a..33d7776ea748 100644
> --- a/drivers/pci/pcie/edr.c
> +++ b/drivers/pci/pcie/edr.c
> @@ -193,6 +193,7 @@ static void edr_handle_event(acpi_handle handle, u32 event, void *data)
>  	 */
>  	if (estate == PCI_ERS_RESULT_RECOVERED) {
>  		pci_dbg(edev, "DPC port successfully recovered\n");
> +		pcie_clear_device_status(dev);
>  		acpi_send_edr_status(pdev, edev, EDR_OST_SUCCESS);
>  	} else {
>  		pci_dbg(edev, "DPC port recovery failed\n");

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer