lkml.org 
[lkml]   [2020]   [Nov]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] AER: aer_root_reset() non-native handling
On Sat, Oct 31, 2020 at 6:36 AM Sean V Kelley <sean.v.kelley@intel.com> wrote:
>
> If an OS has not been granted AER control via _OSC, then
> the OS should not make changes to PCI_ERR_ROOT_COMMAND and
> PCI_ERR_ROOT_STATUS related registers. Per section 4.5.1 of
> the System Firmware Intermediary (SFI) _OSC and DPC Updates
> ECN [1], this bit also covers these aspects of the PCI
> Express Advanced Error Reporting. Further, the handling of
> clear/enable of PCI_ERROR_ROOT_COMMAND when wrapped around
> PCI_ERR_ROOT_STATUS should have no effect and be removed.
> Based on the above and earlier discussion [2], make the
> following changes:
>
> Add a check for the native case (i.e., AER control via _OSC)
> Re-order and remove some of the handling:
> - clear PCI_ERR_ROOT_COMMAND ROOT_PORT_INTR_ON_MESG_MASK
> - do reset
> - clear PCI_ERR_ROOT_STATUS
> - enable PCI_ERR_ROOT_COMMAND ROOT_PORT_INTR_ON_MESG_MASK
>
> to this:
>
> - clear PCI_ERR_ROOT_STATUS
> - do reset
>
> The current "clear, reset, enable" order suggests that the reset
> might cause errors that we should ignore. But I am unable to find a
> reference and the clearing of PCI_ERR_ROOT_STATUS does not require them.
>
> [1] System Firmware Intermediary (SFI) _OSC and DPC Updates ECN, Feb 24,
> 2020, affecting PCI Firmware Specification, Rev. 3.2
> https://members.pcisig.com/wg/PCI-SIG/document/14076
> [2] https://lore.kernel.org/linux-pci/20201020162820.GA370938@bjorn-Precision-5520/
>
> Signed-off-by: Sean V Kelley <sean.v.kelley@intel.com>
> ---
> drivers/pci/pcie/aer.c | 21 ++++++---------------
> 1 file changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 65dff5f3457a..bbfb07842d89 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1361,23 +1361,14 @@ static pci_ers_result_t aer_root_reset(struct pci_dev *dev)
> u32 reg32;
> int rc;
>
> -
> - /* Disable Root's interrupt in response to error messages */
> - pci_read_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
> - reg32 &= ~ROOT_PORT_INTR_ON_MESG_MASK;
> - pci_write_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, reg32);
There may be some reasons to disable interrupt first and then do resetting,
clear status and re-enable interrupt.
Perhaps to avoid error noise, what would happen if the resetting
causes errors itself ?

Thanks,
Ethan

> + if (pcie_aer_is_native(dev)) {
> + /* Clear Root Error Status */
> + pci_read_config_dword(dev, aer + PCI_ERR_ROOT_STATUS, &reg32);
> + pci_write_config_dword(dev, aer + PCI_ERR_ROOT_STATUS, reg32);
> + }
>
> rc = pci_bus_error_reset(dev);
> - pci_info(dev, "Root Port link has been reset\n");
> -
> - /* Clear Root Error Status */
> - pci_read_config_dword(dev, aer + PCI_ERR_ROOT_STATUS, &reg32);
> - pci_write_config_dword(dev, aer + PCI_ERR_ROOT_STATUS, reg32);
> -
> - /* Enable Root Port's interrupt in response to error messages */
> - pci_read_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, &reg32);
> - reg32 |= ROOT_PORT_INTR_ON_MESG_MASK;
> - pci_write_config_dword(dev, aer + PCI_ERR_ROOT_COMMAND, reg32);
> + pci_info(dev, "Root Port link has been reset (%d)\n", rc);
>
> return rc ? PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_RECOVERED;
> }
> --
> 2.29.2
>

\
 
 \ /
  Last update: 2020-11-02 07:28    [W:0.082 / U:1.912 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site