[PATCH 0/6] MIPS: OCTEON: Remove redundant AER code

[PATCH 0/6] MIPS: OCTEON: Remove redundant AER code

[Naveen Naidu]

e8635b484f64 ("MIPS: Add Cavium OCTEON PCI support.") added MIPS
specific code to enable PCIe and AER error reporting (*irrespective
of CONFIG_PCIEAER value*) because PCI core didn't do that at the time.

But currently, the PCI core clears and enables the AER status registers.
So it's redundant for octeon code to do so. This patch series removes
the redundant code from the pci-octeon.c

Currently, the correctable and uncorrectable AER mask registers are not
set to their default value when AER service driver is loaded. This
defect is also fixed in the "[PATCH 1/6]" in the series.

Please note that "Patch 4/6" is dependent on "Patch 1/6".

Thanks,
Naveen Naidu

Naveen Naidu (6):
 [PATCH 1/6] PCI/AER: Enable COR/UNCOR error reporting in set_device_error_reporting()
 [PATCH 2/6] MIPS: OCTEON: Remove redundant clearing of AER status registers
 [PATCH 3/6] MIPS: OCTEON: Remove redundant enable of PCIe normal error reporting
 [PATCH 4/6] MIPS: OCTEON: Remove redundant enable of COR/UNCOR error
 [PATCH 5/6] MIPS: OCTEON: Remove redundant ECRC Generation Enable
 [PATCH 6/6] MIPS: OCTEON: Remove redundant enable of RP error reporting

 arch/mips/pci/pci-octeon.c | 50 --------------------------------------
 drivers/pci/pcie/aer.c     | 13 +++++++++-
 2 files changed, 12 insertions(+), 51 deletions(-)

-- 
2.25.1

[PATCH 1/6] PCI/AER: Enable COR/UNCOR error reporting in set_device_error_reporting()

[Naveen Naidu]

The (PCIe r5.0, sec 7.6.4.3, Table 7-101) and  (PCIe r5.0, sec 7.8.4.6,
Table 7-104) states that the default values for the Uncorrectable Error
Mask and Correctable Error Mask should be 0b. But the current code does
not set the default value of these registers when the PCIe bus loads the
AER service driver.

Enable reporting of all correctable and uncorrectable errors during
aer_probe()

Signed-off-by: Naveen Naidu <naveennaidu479@gmail.com>
---
 drivers/pci/pcie/aer.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 9784fdcf3006..88c4ca6098fb 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1212,6 +1212,7 @@ static int set_device_error_reporting(struct pci_dev *dev, void *data)
 {
 	bool enable = *((bool *)data);
 	int type = pci_pcie_type(dev);
+	int aer = dev->aer_cap;
 
 	if ((type == PCI_EXP_TYPE_ROOT_PORT) ||
 	    (type == PCI_EXP_TYPE_RC_EC) ||
@@ -1223,8 +1224,18 @@ static int set_device_error_reporting(struct pci_dev *dev, void *data)
 			pci_disable_pcie_error_reporting(dev);
 	}
 
-	if (enable)
+	if (enable) {
+
+		/* Enable reporting of all uncorrectable errors */
+		/* Uncorrectable Error Mask - turned on bits disable errors */
+		pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, 0);
+
+		/* Enable reporting of all correctable errors */
+		/* Correctable Error Mask - turned on bits disable errors */
+		pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, 0);
+
 		pcie_set_ecrc_checking(dev);
+	}
 
 	return 0;
 }
-- 
2.25.1

Re: [PATCH 1/6] PCI/AER: Enable COR/UNCOR error reporting in set_device_error_reporting()

[Bjorn Helgaas]

On Mon, Oct 04, 2021 at 11:29:27PM +0530, Naveen Naidu wrote:
> The (PCIe r5.0, sec 7.6.4.3, Table 7-101) and  (PCIe r5.0, sec 7.8.4.6,
> Table 7-104) 

s/7.6.4.3/7.8.4.3/

Cite it like this:

  Per PCIe r5.0, sec 7.8.4.3 and sec 7.8.4.6, the default values ...

> states that the default values for the Uncorrectable Error
> Mask and Correctable Error Mask should be 0b. But the current code does
> not set the default value of these registers when the PCIe bus loads the
> AER service driver.

The defaults specified here are for hardware designers -- this is what
the registers must contain after power-up or reset.  This section of
the spec isn't telling us what the OS is required to write.

If we want to clear these masks, I think we have to:

  1) Analyze every other place that writes the masks to make sure we
  don't break any of them.  There aren't very many, and most of them
  are in drivers, which would be after the aer_probe() path.  There
  might be a conflict with program_hpx_type2(), though.

  2) Make it dependent on pcie_aer_is_native().  Ownership of the AER
  capability can be retained by the platform, in which case the OS
  shouldn't touch it.

> Enable reporting of all correctable and uncorrectable errors during
> aer_probe()
> 
> Signed-off-by: Naveen Naidu <naveennaidu479@gmail.com>
> ---
>  drivers/pci/pcie/aer.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 9784fdcf3006..88c4ca6098fb 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1212,6 +1212,7 @@ static int set_device_error_reporting(struct pci_dev *dev, void *data)
>  {
>  	bool enable = *((bool *)data);
>  	int type = pci_pcie_type(dev);
> +	int aer = dev->aer_cap;
>  
>  	if ((type == PCI_EXP_TYPE_ROOT_PORT) ||
>  	    (type == PCI_EXP_TYPE_RC_EC) ||
> @@ -1223,8 +1224,18 @@ static int set_device_error_reporting(struct pci_dev *dev, void *data)
>  			pci_disable_pcie_error_reporting(dev);
>  	}
>  
> -	if (enable)
> +	if (enable) {
> +
> +		/* Enable reporting of all uncorrectable errors */
> +		/* Uncorrectable Error Mask - turned on bits disable errors */
> +		pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, 0);
> +
> +		/* Enable reporting of all correctable errors */
> +		/* Correctable Error Mask - turned on bits disable errors */
> +		pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, 0);
> +
>  		pcie_set_ecrc_checking(dev);
> +	}
>  
>  	return 0;
>  }
> -- 
> 2.25.1
> 
> _______________________________________________
> Linux-kernel-mentees mailing list
> Linux-kernel-mentees@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees