Re: [PATCH] PCI/DPC: Quirk PIO log size for certain Intel PCIe root ports

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, Aug 16, 2022 at 01:20:42PM +0300, Mika Westerberg wrote:
> There is a BIOS bug on Intel Tiger Lake and Alder Lake systems that
> accidentally clears the root port PIO log size even though it should be 4.
> Fix the affected root ports by forcing the log size to be 4 if it is set
> to 0. The BIOS for the next generation CPUs should have this fixed.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=209943
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
>  drivers/pci/pcie/dpc.c | 13 ++++++++-----
>  drivers/pci/quirks.c   | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 3e9afee02e8d..ab06c801a2c1 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -335,11 +335,14 @@ void pci_dpc_init(struct pci_dev *pdev)
>  		return;
>  
>  	pdev->dpc_rp_extensions = true;
> -	pdev->dpc_rp_log_size = (cap & PCI_EXP_DPC_RP_PIO_LOG_SIZE) >> 8;
> -	if (pdev->dpc_rp_log_size < 4 || pdev->dpc_rp_log_size > 9) {
> -		pci_err(pdev, "RP PIO log size %u is invalid\n",
> -			pdev->dpc_rp_log_size);
> -		pdev->dpc_rp_log_size = 0;
> +	/* If not already set by the quirk in quirks.c */
> +	if (!pdev->dpc_rp_log_size) {
> +		pdev->dpc_rp_log_size = (cap & PCI_EXP_DPC_RP_PIO_LOG_SIZE) >> 8;
> +		if (pdev->dpc_rp_log_size < 4 || pdev->dpc_rp_log_size > 9) {
> +			pci_err(pdev, "RP PIO log size %u is invalid\n",
> +				pdev->dpc_rp_log_size);
> +			pdev->dpc_rp_log_size = 0;
> +		}
>  	}
>  }
>  
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 4944798e75b5..260d8b50f68d 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5956,3 +5956,40 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56b1, aspm_l1_acceptable_latency
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c0, aspm_l1_acceptable_latency);
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x56c1, aspm_l1_acceptable_latency);
>  #endif
> +
> +#ifdef CONFIG_PCIE_DPC
> +/*
> + * Intel Tiger Lake and Alder Lake BIOS has a bug that clears the DPC
> + * log size of the integrated Thunderbolt PCIe root ports so we quirk
> + * them here.
> + */
> +static void dpc_log_size(struct pci_dev *dev)
> +{
> +	u16 dpc_cap, val;
> +
> +	dpc_cap = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_DPC);
> +	if (!dpc_cap)
> +		return;
> +
> +	pci_read_config_word(dev, dpc_cap + PCI_EXP_DPC_CAP, &val);
> +	if (!(val & PCI_EXP_DPC_CAP_RP_EXT))
> +		return;
> +
> +	if (!((val & PCI_EXP_DPC_RP_PIO_LOG_SIZE) >> 8)) {
> +		pci_info(dev, "quirking RP PIO log size\n");
> +		dev->dpc_rp_log_size = 4;

Just to clarify here, I originally thought this was only a cosmetic
fix, and the "DPC: RP PIO log size 0 is invalid" could be ignored.

I guess it's true that it can be ignored, but it *also* means that
dpc_process_rp_pio_error() will not dump the RP PIO Header Log even
though these devices support the RP Extensions and implement the RP
PIO Log registers, right?

> +	}
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x461f, dpc_log_size);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x462f, dpc_log_size);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x463f, dpc_log_size);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x466e, dpc_log_size);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x9a23, dpc_log_size);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x9a25, dpc_log_size);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x9a27, dpc_log_size);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x9a29, dpc_log_size);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x9a2b, dpc_log_size);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x9a2d, dpc_log_size);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x9a2f, dpc_log_size);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x9a31, dpc_log_size);
> +#endif
> -- 
> 2.35.1
>