Re: [PATCH v2 2/2] PCI: Handle Broadcom Vulcan DMA alias calculation quirk

[Bjorn Helgaas <helgaas@kernel.org>]

On Sun, May 08, 2016 at 03:03:21PM +0530, Jayachandran C wrote:
> The Broadcom Vulcan PCI topology is slightly unusual, for a multi-node
> system, it looks like:
> 
>  [bus 0]
>      |
>      +--[node 0 PCI bridge 0.0.0]
>      |           |
>      |        [bus 1]
>      |           +---[SoC PCI devices 1.4.x, 1.5.x] ........
>      |           +---[Glue bridges    1.a.x, 1.b.x]         \
>      |                    |                        .....{node 0 GIC ITS}
>      |                    |                       /
>      |                    +----[SoC PCIe controller root ports]
>      |                                |         \...... {SMMUv3 0..3}
>      |                                |
>      |                                +---- [external PCI devices]
>      +- [node 1 PCI bridge 0.0.1]
>      |           |
>      |        [bus n - similar to bus 1 above]
>      ...
> 
> The for_each_dma_alias call for external PCI devices should not go
> beyond the PCIe controllers where the SMMU and ITS is associated, going
> further above to the glue bridges or the node bridges will find aliases
> which are not valid. For internal devices, the association is at the
> node level bridge and the alias search should not go further.

I like the line of reasoning that we should not iterate higher in the
hierarchy than where the translation hardware is attached.  That seems
like a reasonable thing in general, not just for this hardware.

Is there some generic way to learn where the translation hardware is
attached?  If there is, we could make pci_for_each_dma_alias() use
that information, and maybe we could solve the problem for other
systems as well as for Vulcan.

> To handle this quirk, we mark the SoC PCIe bridges and node PCI bridge
> with flag PCI_DEV_FLAGS_DMA_ALIAS_ROOT so that the alias and RID
> calculations are correct.
> 
> Signed-off-by: Jayachandran C <jchandra@broadcom.com>
> ---
>  drivers/pci/quirks.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 8e67802..62664b5 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3734,6 +3734,20 @@ DECLARE_PCI_FIXUP_HEADER(0x1283, 0x8892, quirk_use_pcie_bridge_dma_alias);
>  DECLARE_PCI_FIXUP_HEADER(0x8086, 0x244e, quirk_use_pcie_bridge_dma_alias);
>  
>  /*
> + * The IOMMU and interrupt controller of Broadcom vulcan are associated
> + * not at the root bus, but at a bridge below. The DMA alias search should
> + * not go above the bridge at which the association exists.
> + */
> +static void quirk_bridge_brcm_vulcan_pcie_root(struct pci_dev *pdev)
> +{
> +	pdev->dev_flags |= PCI_DEV_FLAGS_DMA_ALIAS_ROOT;
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_BROADCOM, 0x9000,
> +				quirk_bridge_brcm_vulcan_pcie_root);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_BROADCOM, 0x9084,
> +				quirk_bridge_brcm_vulcan_pcie_root);
> +
> +/*
>   * Intersil/Techwell TW686[4589]-based video capture cards have an empty (zero)
>   * class code.  Fix it.
>   */
> -- 
> 1.9.1
>