Re: [PATCH v3] PCI/AER: Fix aerdrv loading with "pcie_ports=native" parameter

[Bjorn Helgaas]

On Mon, Jul 02, 2018 at 11:16:01AM -0500, Alexandru Gagniuc wrote:
> According to the documentation, "pcie_ports=native", linux should use
> native AER and DPC services. While that is true for the _OSC method
> parsing, this is not the only place that is checked. Should the HEST
> table list PCIe ports as firmware-first, linux will not use native
> services.
> 
> This happens because aer_acpi_firmware_first() doesn't take
> 'pcie_ports' into account. This is wrong. DPC uses the same logic when
> it decides whether to load or not, so fixing this also fixes DPC not
> loading.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@xxxxxxxxx>
> ---
>  drivers/pci/pcie/aer.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index a2e88386af28..db2c01056dc7 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -283,13 +283,14 @@ static int aer_hest_parse(struct acpi_hest_header *hest_hdr, void *data)
>  
>  static void aer_set_firmware_first(struct pci_dev *pci_dev)
>  {
> -	int rc;
> +	int rc = 0;
>  	struct aer_hest_parse_info info = {
>  		.pci_dev	= pci_dev,
>  		.firmware_first	= 0,
>  	};
>  
> -	rc = apei_hest_parse(aer_hest_parse, &info);
> +	if (!pcie_ports_native)
> +		rc = apei_hest_parse(aer_hest_parse, &info);
>  
>  	if (rc)
>  		pci_dev->__aer_firmware_first = 0;
> @@ -324,7 +325,9 @@ bool aer_acpi_firmware_first(void)
>  	};
>  
>  	if (!parsed) {
> -		apei_hest_parse(aer_hest_parse, &info);
> +		if (!pcie_ports_native)
> +			apei_hest_parse(aer_hest_parse, &info);
> +
>  		aer_firmware_first = info.firmware_first;
>  		parsed = true;
>  	}

I was thinking something along the lines of the patch below, so we
don't have to work through the settings of "rc" and "info".  But maybe
I'm missing something subtle?

One subtle thing that I didn't look into is the
pcie_aer_get_firmware_first() stub for the non-CONFIG_ACPI_APEI case.


diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index b24f2d252180..5ccbd7635f33 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -342,6 +342,9 @@ int pcie_aer_get_firmware_first(struct pci_dev *dev)
 	if (!pci_is_pcie(dev))
 		return 0;
 
+	if (pcie_ports_native)
+		return 0;
+
 	if (!dev->__aer_firmware_first_valid)
 		aer_set_firmware_first(dev);
 	return dev->__aer_firmware_first;
@@ -362,6 +365,9 @@ bool aer_acpi_firmware_first(void)
 		.firmware_first	= 0,
 	};
 
+	if (pcie_ports_native)
+		return false;
+
 	if (!parsed) {
 		apei_hest_parse(aer_hest_parse, &info);
 		aer_firmware_first = info.firmware_first;