On Thu, Oct 03, 2019 at 04:39:01PM -0700, sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx wrote:
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
>
> As per PCI firmware specification r3.2 Downstream Port Containment
> Related Enhancements ECN, sec 4.5.1, table 4-6,
That table adds bit 7, "DPC config control" to the _OSC control field
and talks about modifying registers in the DPC capability.
It doesn't say anything about firmware-first or about the OS modifying
registers in the AER capability. But this patch doesn't do anything
related to DPC or _OSC.
> Error Disconnect
> Recover (EDR) support allows OS to handle error recovery and
> clearing Error Registers even in FF mode. So remove FF mode checks in
> pci_cleanup_aer_uncorrect_error_status(), pci_aer_clear_fatal_status()
> and pci_cleanup_aer_error_status_regs() functions.
>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> Acked-by: Keith Busch <keith.busch@xxxxxxxxx>
> ---
> drivers/pci/pcie/aer.c | 12 ++----------
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index b45bc47d04fe..e1509bb900c5 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -383,9 +383,6 @@ int pci_cleanup_aer_uncorrect_error_status(struct pci_dev *dev)
> if (!pos)
> return -EIO;
>
> - if (pcie_aer_get_firmware_first(dev))
> - return -EIO;
> -
> /* Clear status bits for ERR_NONFATAL errors only */
> pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, &status);
> pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, &sev);
> @@ -406,9 +403,6 @@ void pci_aer_clear_fatal_status(struct pci_dev *dev)
> if (!pos)
> return;
>
> - if (pcie_aer_get_firmware_first(dev))
> - return;
> -
> /* Clear status bits for ERR_FATAL errors only */
> pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, &status);
> pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, &sev);
> @@ -430,9 +424,6 @@ int pci_cleanup_aer_error_status_regs(struct pci_dev *dev)
> if (!pos)
> return -EIO;
>
> - if (pcie_aer_get_firmware_first(dev))
> - return -EIO;
> -
> port_type = pci_pcie_type(dev);
> if (port_type == PCI_EXP_TYPE_ROOT_PORT) {
> pci_read_config_dword(dev, pos + PCI_ERR_ROOT_STATUS, &status);
> @@ -455,7 +446,8 @@ void pci_aer_init(struct pci_dev *dev)
> if (dev->aer_cap)
> dev->aer_stats = kzalloc(sizeof(struct aer_stats), GFP_KERNEL);
>
> - pci_cleanup_aer_error_status_regs(dev);
> + if (!pcie_aer_get_firmware_first(dev))
> + pci_cleanup_aer_error_status_regs(dev);
This effectively moves the "if (pcie_aer_get_firmware_first())" check
from pci_cleanup_aer_error_status_regs() into one of the callers. But
there are two other callers: pci_aer_init() and pci_restore_state().
Do they need the change, or do you want to cleanup the AER error
registers there, but not here?
> }
>
> void pci_aer_exit(struct pci_dev *dev)
> --
> 2.21.0
>
