Re: [PATCH] PCI: Test for std config alias when testing extended config space

[Bjorn Helgaas <bhelgaas@google.com>]

On Mon, May 05, 2014 at 02:20:51PM -0600, Alex Williamson wrote:
> When a PCI-to-PCIe bridge is stacked on a PCIe-to-PCI bridge, we can
> have PCIe endpoints masked by a conventional PCI bus.  This makes the
> extended config space of the PCIe endpoing inaccessible.  The PCIe-to-
> PCI bridge is supposed to handle any type 1 configuration transactions
> where the extended config offset bits are non-zero as an Unsupported
> Request rather than forward it to the secondary interface.  As noted
> here, there are a couple known offenders to this rule.  These bridges
> drop the extended offset bits, resulting in the conventional config
> space being aliased many times across the extended config space.  For
> Intel NICs, this alias often seems to expose a bogus SR-IOV cap.
> 
> Stacking bridges may seem like an uncommon scenario, but note that the
> any conventional PCI slot in a modern PC is already the secondary
> interface of an onboard PCIe-to-PCI bridge.  The user need only add
> a PCI-to-PCIe adapter and PCIe device to encounter this problem.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

Applied to pci/misc for v3.16, thanks!

> ---
>  drivers/pci/probe.c |   39 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index ef09f5f..fdb0182 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -984,6 +984,43 @@ void set_pcie_hotplug_bridge(struct pci_dev *pdev)
>  
>  
>  /**
> + * pci_ext_cfg_is_aliased - is ext config space just an alias of std config?
> + * @dev: PCI device
> + *
> + * PCI Express to PCI/PCI-X Bridge Specification, rev 1.0, 4.1.4 says that
> + * when forwarding a type1 configuration request the bridge must check that
> + * the extended register address field is zero.  The bridge is not permitted
> + * to forward the transactions and must handle it as an Unsupported Request.
> + * Some bridges do not follow this rule and simply drop the extended register
> + * bits, resulting in the standard config space being aliased, every 256
> + * bytes across the entire configuration space.  Test for this condition by
> + * comparing the first dword of each potential alias to the vendor/device ID.
> + * Known offenders:
> + *   ASM1083/1085 PCIe-to-PCI Reversible Bridge (1b21:1080, rev 01 & 03)
> + *   AMD/ATI SBx00 PCI to PCI Bridge (1002:4384, rev 40)
> + */
> +static bool pci_ext_cfg_is_aliased(struct pci_dev *dev)
> +{
> +#ifdef CONFIG_PCI_QUIRKS
> +	int pos;
> +	u32 header, tmp;
> +
> +	pci_read_config_dword(dev, PCI_VENDOR_ID, &header);
> +
> +	for (pos = PCI_CFG_SPACE_SIZE;
> +	     pos < PCI_CFG_SPACE_EXP_SIZE; pos += PCI_CFG_SPACE_SIZE) {
> +		if (pci_read_config_dword(dev, pos, &tmp) != PCIBIOS_SUCCESSFUL
> +		    || header != tmp)
> +			return false;
> +	}
> +
> +	return true;
> +#else
> +	return false;
> +#endif
> +}
> +
> +/**
>   * pci_cfg_space_size - get the configuration space size of the PCI device.
>   * @dev: PCI device
>   *
> @@ -1001,7 +1038,7 @@ static int pci_cfg_space_size_ext(struct pci_dev *dev)
>  
>  	if (pci_read_config_dword(dev, pos, &status) != PCIBIOS_SUCCESSFUL)
>  		goto fail;
> -	if (status == 0xffffffff)
> +	if (status == 0xffffffff || pci_ext_cfg_is_aliased(dev))
>  		goto fail;
>  
>  	return PCI_CFG_SPACE_EXP_SIZE;
>