[PATCH v5 0/2] Handle Cavium ThunderX2 PCI topology quirk

[PATCH v5 0/2] Handle Cavium ThunderX2 PCI topology quirk

[Jayachandran C]

Hi Bjorn,

Here's v5 of the patchset with an updated commit message based on your
suggestion. I have added a paragraph at the end about MSI-X as well.

This is the newest in the series based on the discussion here:
https://www.spinics.net/lists/arm-kernel/msg575581.html

Previous discussions and patches:
http://www.spinics.net/lists/linux-pci/msg51001.html
https://patchwork.ozlabs.org/patch/582633/ and
https://lists.linuxfoundation.org/pipermail/iommu/2016-June/017681.html

Let me know if I missed anything, Thanks,
JC.

v4->v5:
 - updated commit message for patch 2/2

v3->v4:
 - new address of author

v2>v3:
 - changed device flag name from PCI_DEV_FLAGS_DMA_ALIAS_ROOT to
   PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT
 - updated commit message to make the quirk clearer.
 
Jayachandran C (2):
  PCI: Add device flag PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT
  PCI: quirks: Fix ThunderX2 dma alias handling

 drivers/pci/quirks.c | 14 ++++++++++++++
 drivers/pci/search.c |  4 ++++
 include/linux/pci.h  |  2 ++
 3 files changed, 20 insertions(+)

-- 
2.7.4

[PATCH v5 1/2] PCI: Add device flag PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT

[Jayachandran C]

Add a new quirk flag PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT to limit the DMA
alias search to go no further than the bridge where the IOMMU unit is
attached.

The flag will be used to indicate a bridge device which forwards the
address translation requests to the IOMMU, i.e where the interrupt and
DMA requests leave the PCIe hierarchy and go into the system blocks.

Usually this happens at the PCI RC, so this flag is not needed.  But
on systems where there are bridges that introduce aliases above the
"real" root bridge, this flag is needed to ensure that the function
pci_for_each_dma_alias() works correctly.

The function pci_for_each_dma_alias() is updated to stop when it see a
bridge with this flag set.

Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: David Daney <david.daney@cavium.com>
---
 drivers/pci/search.c | 4 ++++
 include/linux/pci.h  | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 33e0f03..4c6044a 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -60,6 +60,10 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 
 		tmp = bus->self;
 
+		/* stop at bridge where translation unit is associated */
+		if (tmp->dev_flags & PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT)
+			return ret;
+
 		/*
 		 * PCIe-to-PCI/X bridges alias transactions from downstream
 		 * devices using the subordinate bus number (PCI Express to
diff --git a/include/linux/pci.h b/include/linux/pci.h
index eb3da1a..3f596ac 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -178,6 +178,8 @@ enum pci_dev_flags {
 	PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 7),
 	/* Get VPD from function 0 VPD */
 	PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 8),
+	/* a non-root bridge where translation occurs, stop alias search here */
+	PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT = (__force pci_dev_flags_t) (1 << 9),
 };
 
 enum pci_irq_reroute_variant {
-- 
2.7.4

[PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling

[Jayachandran C]

On Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the
PCI topology is slightly unusual.  For a multi-node system, it looks
like:

    00:00.0 [PCI] bridge to [bus 01-1e]
    01:0a.0 [PCI-PCIe bridge, type 8] bridge to [bus 02-04]
    02:00.0 [PCIe root port, type 4] bridge to [bus 03-04] (XLATE_ROOT)
    03:00.0 PCIe Endpoint

pci_for_each_dma_alias() assumes IOMMU translation is done at the root
of the PCI hierarchy.  It generates 03:00.0, 01:0a.0, and 00:00.0 as
DMA aliases for 03:00.0 because buses 01 and 00 are non-PCIe buses that
doesn't carry the Requester ID.

Because the ThunderX2 IOMMU is at 02:00.0, the Requester IDs 01:0a.0
or 00:00.0 are never valid for the endpoint.  This quirk stops alias
generation at the XLATE_ROOT bridge so we won't generate 01:0a.0 or
00:00.0

The current IOMMU code only maps the last alias (this is a separate bug
in itself).  Prior to this quirk, we only created IOMMU mappings for the
invalid Requester ID 00:00:0, which never matched any DMA transactions.

With this quirk, we create IOMMU mappings for a valid Requester ID,
which fixes devices with no aliases but leaves devices with aliases
still broken.

The last alias for the endpoint is also used by the ARM GICv3 MSI-X
code.  Without this quirk, the GIC Interrupt Translation Tables are
setup with the invalid Requester ID, and the MSI-X generated by the
device fails to be translated and routed.

Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: David Daney <david.daney@cavium.com>
---
 drivers/pci/quirks.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 6736836..564a84a 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3958,6 +3958,20 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2260, quirk_mic_x200_dma_alias);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2264, quirk_mic_x200_dma_alias);
 
 /*
+ * The IOMMU and interrupt controller on Broadcom Vulcan/Cavium ThunderX2 are
+ * associated not at the root bus, but at a bridge below. This quirk flag
+ * will ensure that the aliases are identified correctly.
+ */
+static void quirk_bridge_cavm_thrx2_pcie_root(struct pci_dev *pdev)
+{
+	pdev->dev_flags |= PCI_DEV_FLAGS_BRIDGE_XLATE_ROOT;
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_BROADCOM, 0x9000,
+				quirk_bridge_cavm_thrx2_pcie_root);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_BROADCOM, 0x9084,
+				quirk_bridge_cavm_thrx2_pcie_root);
+
+/*
  * Intersil/Techwell TW686[4589]-based video capture cards have an empty (zero)
  * class code.  Fix it.
  */
-- 
2.7.4

[PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling

[Bjorn Helgaas]

I tentatively applied both patches to pci/host-thunder for v4.12.

However, I am concerned about the topology here:

On Thu, Apr 13, 2017 at 08:30:45PM +0000, Jayachandran C wrote:
> On Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the
> PCI topology is slightly unusual.  For a multi-node system, it looks
> like:
> 
>     00:00.0 [PCI] bridge to [bus 01-1e]
>     01:0a.0 [PCI-PCIe bridge, type 8] bridge to [bus 02-04]
>     02:00.0 [PCIe root port, type 4] bridge to [bus 03-04] (XLATE_ROOT)
>     03:00.0 PCIe Endpoint

A root port normally has a single PCIe link leading downstream.
According to this, 02:00.0 is a root port that has the usual
downstream link leading to 03:00.0, but it also has an upstream link
to 01:0a.0.

Maybe this example is omitting details that are not relevant to DMA
aliases?  The PCIe capability only contains one set of link-related
registers, so I don't know how we could manage a single device that
has two links.

A device with two links would break things like ASPM.  In
set_pcie_port_type(), for example, we have this comment:

   * A Root Port or a PCI-to-PCIe bridge is always the upstream end
   * of a Link.  No PCIe component has two Links.  Two Links are
   * connected by a Switch that has a Port on each Link and internal
   * logic to connect the two Ports.

The topology above breaks these assumptions, which will make
pdev->has_secondary_link incorrect, which means ASPM won't work
correctly.

What am I missing?

Bjorn

[PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling

[Jayachandran C]

On Thu, Apr 13, 2017 at 07:19:11PM -0500, Bjorn Helgaas wrote:
> I tentatively applied both patches to pci/host-thunder for v4.12.
> 
> However, I am concerned about the topology here:
> 
> On Thu, Apr 13, 2017 at 08:30:45PM +0000, Jayachandran C wrote:
> > On Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the
> > PCI topology is slightly unusual.  For a multi-node system, it looks
> > like:
> > 
> >     00:00.0 [PCI] bridge to [bus 01-1e]
> >     01:0a.0 [PCI-PCIe bridge, type 8] bridge to [bus 02-04]
> >     02:00.0 [PCIe root port, type 4] bridge to [bus 03-04] (XLATE_ROOT)
> >     03:00.0 PCIe Endpoint
> 
> A root port normally has a single PCIe link leading downstream.
> According to this, 02:00.0 is a root port that has the usual
> downstream link leading to 03:00.0, but it also has an upstream link
> to 01:0a.0.

The PCI topology is a bit broken due to the way that the PCIe IP block
was integrated into SoC PCI bridges and devices. The current mechanism
of adding a PCI-PCIe bridge to glue these together is not ideal.

> Maybe this example is omitting details that are not relevant to DMA
> aliases?  The PCIe capability only contains one set of link-related
> registers, so I don't know how we could manage a single device that
> has two links.
 
The root port is standard and has just one link to the EP (or whatever
is on the external PCIe slot). The fallout of the current hw design is
that the PCI-PCIe bridge has a link that does not follow standard and
does not have a counterpart (as you noted).

> A device with two links would break things like ASPM.  In
> set_pcie_port_type(), for example, we have this comment:
> 
>    * A Root Port or a PCI-to-PCIe bridge is always the upstream end
>    * of a Link.  No PCIe component has two Links.  Two Links are
>    * connected by a Switch that has a Port on each Link and internal
>    * logic to connect the two Ports.
> 
> The topology above breaks these assumptions, which will make
> pdev->has_secondary_link incorrect, which means ASPM won't work
> correctly.

Given the current hardware, the pcieport driver seems to work reasonably
for the root port at 02:00.0, with AER support. I will take a look at the
ASPM part.

Thanks,
JC.

[PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling

[Bjorn Helgaas]

On Fri, Apr 14, 2017 at 4:06 PM, Jayachandran C
<jnair@caviumnetworks.com> wrote:
> On Thu, Apr 13, 2017 at 07:19:11PM -0500, Bjorn Helgaas wrote:
>> I tentatively applied both patches to pci/host-thunder for v4.12.
>>
>> However, I am concerned about the topology here:
>>
>> On Thu, Apr 13, 2017 at 08:30:45PM +0000, Jayachandran C wrote:
>> > On Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the
>> > PCI topology is slightly unusual.  For a multi-node system, it looks
>> > like:
>> >
>> >     00:00.0 [PCI] bridge to [bus 01-1e]
>> >     01:0a.0 [PCI-PCIe bridge, type 8] bridge to [bus 02-04]
>> >     02:00.0 [PCIe root port, type 4] bridge to [bus 03-04] (XLATE_ROOT)
>> >     03:00.0 PCIe Endpoint
>>
>> A root port normally has a single PCIe link leading downstream.
>> According to this, 02:00.0 is a root port that has the usual
>> downstream link leading to 03:00.0, but it also has an upstream link
>> to 01:0a.0.
>
> The PCI topology is a bit broken due to the way that the PCIe IP block
> was integrated into SoC PCI bridges and devices. The current mechanism
> of adding a PCI-PCIe bridge to glue these together is not ideal.

Yeah, that's definitely broken.

>> Maybe this example is omitting details that are not relevant to DMA
>> aliases?  The PCIe capability only contains one set of link-related
>> registers, so I don't know how we could manage a single device that
>> has two links.
>
> The root port is standard and has just one link to the EP (or whatever
> is on the external PCIe slot). The fallout of the current hw design is
> that the PCI-PCIe bridge has a link that does not follow standard and
> does not have a counterpart (as you noted).
>
>> A device with two links would break things like ASPM.  In
>> set_pcie_port_type(), for example, we have this comment:
>>
>>    * A Root Port or a PCI-to-PCIe bridge is always the upstream end
>>    * of a Link.  No PCIe component has two Links.  Two Links are
>>    * connected by a Switch that has a Port on each Link and internal
>>    * logic to connect the two Ports.
>>
>> The topology above breaks these assumptions, which will make
>> pdev->has_secondary_link incorrect, which means ASPM won't work
>> correctly.
>
> Given the current hardware, the pcieport driver seems to work reasonably
> for the root port at 02:00.0, with AER support. I will take a look at the
> ASPM part.

I don't think pcieport itself cares much about links.  ASPM does, but
it looks like set_pcie_port_type() actually is smart enough to know
that PCI-to-PCIe bridges and Root Ports always have links on their
secondary sides.  So has_secondary_link probably does get set
correctly.

But I think you'll hit the VIA "strange chipset" thing in
pcie_aspm_init_link_state(), which will probably prevent us from doing
ASPM on the link from 02:00.0 to 03:00.0.

Could you collect "lspci -vv" output from this system?  I'd like to
archive that as background for this IOMMU issue and the ASPM tweaks I
suspect we'll have to do.  I *wish* we had more information about that
VIA thing, because I suspect we could get rid of it if we had more
details.

Bjorn

[PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling

[Jayachandran C]

On Fri, Apr 14, 2017 at 09:00:06PM -0500, Bjorn Helgaas wrote:
> On Fri, Apr 14, 2017 at 4:06 PM, Jayachandran C
> <jnair@caviumnetworks.com> wrote:
> > On Thu, Apr 13, 2017 at 07:19:11PM -0500, Bjorn Helgaas wrote:
> >> I tentatively applied both patches to pci/host-thunder for v4.12.
> >>
> >> However, I am concerned about the topology here:
> >>
> >> On Thu, Apr 13, 2017 at 08:30:45PM +0000, Jayachandran C wrote:
> >> > On Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the
> >> > PCI topology is slightly unusual.  For a multi-node system, it looks
> >> > like:
> >> >
> >> >     00:00.0 [PCI] bridge to [bus 01-1e]
> >> >     01:0a.0 [PCI-PCIe bridge, type 8] bridge to [bus 02-04]
> >> >     02:00.0 [PCIe root port, type 4] bridge to [bus 03-04] (XLATE_ROOT)
> >> >     03:00.0 PCIe Endpoint
> >>
> >> A root port normally has a single PCIe link leading downstream.
> >> According to this, 02:00.0 is a root port that has the usual
> >> downstream link leading to 03:00.0, but it also has an upstream link
> >> to 01:0a.0.
> >
> > The PCI topology is a bit broken due to the way that the PCIe IP block
> > was integrated into SoC PCI bridges and devices. The current mechanism
> > of adding a PCI-PCIe bridge to glue these together is not ideal.
> 
> Yeah, that's definitely broken.
> 
> >> Maybe this example is omitting details that are not relevant to DMA
> >> aliases?  The PCIe capability only contains one set of link-related
> >> registers, so I don't know how we could manage a single device that
> >> has two links.
> >
> > The root port is standard and has just one link to the EP (or whatever
> > is on the external PCIe slot). The fallout of the current hw design is
> > that the PCI-PCIe bridge has a link that does not follow standard and
> > does not have a counterpart (as you noted).
> >
> >> A device with two links would break things like ASPM.  In
> >> set_pcie_port_type(), for example, we have this comment:
> >>
> >>    * A Root Port or a PCI-to-PCIe bridge is always the upstream end
> >>    * of a Link.  No PCIe component has two Links.  Two Links are
> >>    * connected by a Switch that has a Port on each Link and internal
> >>    * logic to connect the two Ports.
> >>
> >> The topology above breaks these assumptions, which will make
> >> pdev->has_secondary_link incorrect, which means ASPM won't work
> >> correctly.
> >
> > Given the current hardware, the pcieport driver seems to work reasonably
> > for the root port at 02:00.0, with AER support. I will take a look at the
> > ASPM part.
> 
> I don't think pcieport itself cares much about links.  ASPM does, but
> it looks like set_pcie_port_type() actually is smart enough to know
> that PCI-to-PCIe bridges and Root Ports always have links on their
> secondary sides.  So has_secondary_link probably does get set
> correctly.
> 
> But I think you'll hit the VIA "strange chipset" thing in
> pcie_aspm_init_link_state(), which will probably prevent us from doing
> ASPM on the link from 02:00.0 to 03:00.0.
> 
> Could you collect "lspci -vv" output from this system?  I'd like to
> archive that as background for this IOMMU issue and the ASPM tweaks I
> suspect we'll have to do.  I *wish* we had more information about that
> VIA thing, because I suspect we could get rid of it if we had more
> details.

The full logs are slightly large, so I have kept them at:
https://github.com/jchandra-cavm/thunderx2/blob/master/logs/
The lspci -vv output is lspci-vv.txt and lspci -tvn output is lspci-tvn.txt

The output is from 2 socket system, the cards are not on the first slot
like the example above, so the bus and device numbers are different.

Looks like I have to spend some time on ASPM next.

Thanks,
JC.

[PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling

[Bjorn Helgaas]

On Mon, Apr 17, 2017 at 12:47 PM, Jayachandran C
<jnair@caviumnetworks.com> wrote:
> On Fri, Apr 14, 2017 at 09:00:06PM -0500, Bjorn Helgaas wrote:
>> On Fri, Apr 14, 2017 at 4:06 PM, Jayachandran C
>> <jnair@caviumnetworks.com> wrote:
>> > On Thu, Apr 13, 2017 at 07:19:11PM -0500, Bjorn Helgaas wrote:
>> >> I tentatively applied both patches to pci/host-thunder for v4.12.
>> >>
>> >> However, I am concerned about the topology here:
>> >>
>> >> On Thu, Apr 13, 2017 at 08:30:45PM +0000, Jayachandran C wrote:
>> >> > On Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the
>> >> > PCI topology is slightly unusual.  For a multi-node system, it looks
>> >> > like:
>> >> >
>> >> >     00:00.0 [PCI] bridge to [bus 01-1e]
>> >> >     01:0a.0 [PCI-PCIe bridge, type 8] bridge to [bus 02-04]
>> >> >     02:00.0 [PCIe root port, type 4] bridge to [bus 03-04] (XLATE_ROOT)
>> >> >     03:00.0 PCIe Endpoint
>> >>
>> >> A root port normally has a single PCIe link leading downstream.
>> >> According to this, 02:00.0 is a root port that has the usual
>> >> downstream link leading to 03:00.0, but it also has an upstream link
>> >> to 01:0a.0.
>> >
>> > The PCI topology is a bit broken due to the way that the PCIe IP block
>> > was integrated into SoC PCI bridges and devices. The current mechanism
>> > of adding a PCI-PCIe bridge to glue these together is not ideal.
>>
>> Yeah, that's definitely broken.
>>
>> >> Maybe this example is omitting details that are not relevant to DMA
>> >> aliases?  The PCIe capability only contains one set of link-related
>> >> registers, so I don't know how we could manage a single device that
>> >> has two links.
>> >
>> > The root port is standard and has just one link to the EP (or whatever
>> > is on the external PCIe slot). The fallout of the current hw design is
>> > that the PCI-PCIe bridge has a link that does not follow standard and
>> > does not have a counterpart (as you noted).
>> >
>> >> A device with two links would break things like ASPM.  In
>> >> set_pcie_port_type(), for example, we have this comment:
>> >>
>> >>    * A Root Port or a PCI-to-PCIe bridge is always the upstream end
>> >>    * of a Link.  No PCIe component has two Links.  Two Links are
>> >>    * connected by a Switch that has a Port on each Link and internal
>> >>    * logic to connect the two Ports.
>> >>
>> >> The topology above breaks these assumptions, which will make
>> >> pdev->has_secondary_link incorrect, which means ASPM won't work
>> >> correctly.
>> >
>> > Given the current hardware, the pcieport driver seems to work reasonably
>> > for the root port at 02:00.0, with AER support. I will take a look at the
>> > ASPM part.
>>
>> I don't think pcieport itself cares much about links.  ASPM does, but
>> it looks like set_pcie_port_type() actually is smart enough to know
>> that PCI-to-PCIe bridges and Root Ports always have links on their
>> secondary sides.  So has_secondary_link probably does get set
>> correctly.
>>
>> But I think you'll hit the VIA "strange chipset" thing in
>> pcie_aspm_init_link_state(), which will probably prevent us from doing
>> ASPM on the link from 02:00.0 to 03:00.0.
>>
>> Could you collect "lspci -vv" output from this system?  I'd like to
>> archive that as background for this IOMMU issue and the ASPM tweaks I
>> suspect we'll have to do.  I *wish* we had more information about that
>> VIA thing, because I suspect we could get rid of it if we had more
>> details.
>
> The full logs are slightly large, so I have kept them at:
> https://github.com/jchandra-cavm/thunderx2/blob/master/logs/
> The lspci -vv output is lspci-vv.txt and lspci -tvn output is lspci-tvn.txt
>
> The output is from 2 socket system, the cards are not on the first slot
> like the example above, so the bus and device numbers are different.
>
> Looks like I have to spend some time on ASPM next.

Thanks, I attached these to
https://bugzilla.kernel.org/show_bug.cgi?id=195447 and added that link
to the changelogs.

  01:0a.0 PCI-to-PCIe bridge to [bus 02-03]
    Capabilities: [40] Express (v2) PCI/PCI-X to PCI-Express Bridge

lspci doesn't decode the "Slot Implemented" bit here.  The spec (PCIe
r3.1, sec 7.8.2) isn't explicit about whether that bit is defined for
this kind of bridge, but it seems to me like this bridge contains a
Downstream Port that could lead to a slot, so we *should* decode "Slot
Implemented", and if it does indicate a slot, we should decode the
Slot Capabilities, Control, and Status registers as well.

Linux also doesn't currently believe this bridge can have a slot below
it (see pcie_cap_has_sltctl() and pcie_downstream_port()).  I don't
know if your topology has actual slots there, but I think the spec
does allow it, so Linux probably should handle that.

For this port:

  02:00.0 Root Port to [bus 03]
    Capabilities: [ac] Express (v2) Root Port (Slot-)

I'm pretty sure there *is* a slot (currently empty), and your lspci
output shows "Slot-", which seems wrong to me.  It should show "Slot+"
with Presence Detect State showing "Slot Empty", shouldn't it?

Bjorn

[PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling

[Jon Masters]

Hi Bjorn, JC,

On 04/13/2017 08:19 PM, Bjorn Helgaas wrote:

> I tentatively applied both patches to pci/host-thunder for v4.12.

Thanks for that :)

> However, I am concerned about the topology here:

Various feedback has been provided on this one over the past $time. In
addition, I have requested that this serve as an example of why we need
a more complete PCIe integration guide for ARMv8. It's on the list of
things for my intended opus magnum on the topic ;)

> On Thu, Apr 13, 2017 at 08:30:45PM +0000, Jayachandran C wrote:
>> On Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the
>> PCI topology is slightly unusual.  For a multi-node system, it looks
>> like:
>>
>>     00:00.0 [PCI] bridge to [bus 01-1e]
>>     01:0a.0 [PCI-PCIe bridge, type 8] bridge to [bus 02-04]
>>     02:00.0 [PCIe root port, type 4] bridge to [bus 03-04] (XLATE_ROOT)
>>     03:00.0 PCIe Endpoint
> 
> A root port normally has a single PCIe link leading downstream.

In integration terms, there's always a bus on the other side of the RC.
It's just usually a processor local bus of some kind on a proprietary
interconnect, but there's always something there. The issue is when you
can see it as PCIe and it's not through a transparent glue bridge.

I had originally hoped that the ECAM could be hacked up so that we would
first walk the topology at the 02:00.0 as a root and not see what's
above it BUT there are other device attachments up there for the on-SoC
devices and I think we really intend to use those.

Bottom line in my opinion is document this case, use it as a learning
example, and move forward. This has been useful in justifying why we
need better integration documentation from the server community. And in
particular from the OS vendors, of which I guess we can allude to their
being more than Linux interested in ARM server these days ;)

Jon.

-- 
Computer Architect | Sent from my Fedora powered Ryzen

[PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling

[Jon Masters]

One additional footnote. I spent a bunch of time recently on my personal
Xeon systems walking through the PCIe topology and aligning on how to
advise the ARM server community proceed going forward. If you look at
how Intel vs AMD handle their host bridges for example, you'll see two
very different approaches to the case of cross-socket PCIe. But my
operating assumption is that anything longer term which looks boring and
x86 enough is probably fine from an ARM server point of view.

On 04/19/2017 07:38 PM, Jon Masters wrote:
> Hi Bjorn, JC,
> 
> On 04/13/2017 08:19 PM, Bjorn Helgaas wrote:
> 
>> I tentatively applied both patches to pci/host-thunder for v4.12.
> 
> Thanks for that :)
> 
>> However, I am concerned about the topology here:
> 
> Various feedback has been provided on this one over the past $time. In
> addition, I have requested that this serve as an example of why we need
> a more complete PCIe integration guide for ARMv8. It's on the list of
> things for my intended opus magnum on the topic ;)
> 
>> On Thu, Apr 13, 2017 at 08:30:45PM +0000, Jayachandran C wrote:
>>> On Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the
>>> PCI topology is slightly unusual.  For a multi-node system, it looks
>>> like:
>>>
>>>     00:00.0 [PCI] bridge to [bus 01-1e]
>>>     01:0a.0 [PCI-PCIe bridge, type 8] bridge to [bus 02-04]
>>>     02:00.0 [PCIe root port, type 4] bridge to [bus 03-04] (XLATE_ROOT)
>>>     03:00.0 PCIe Endpoint
>>
>> A root port normally has a single PCIe link leading downstream.
> 
> In integration terms, there's always a bus on the other side of the RC.
> It's just usually a processor local bus of some kind on a proprietary
> interconnect, but there's always something there. The issue is when you
> can see it as PCIe and it's not through a transparent glue bridge.
> 
> I had originally hoped that the ECAM could be hacked up so that we would
> first walk the topology at the 02:00.0 as a root and not see what's
> above it BUT there are other device attachments up there for the on-SoC
> devices and I think we really intend to use those.
> 
> Bottom line in my opinion is document this case, use it as a learning
> example, and move forward. This has been useful in justifying why we
> need better integration documentation from the server community. And in
> particular from the OS vendors, of which I guess we can allude to their
> being more than Linux interested in ARM server these days ;)
> 
> Jon.
> 

-- 
Computer Architect | Sent from my Fedora powered Ryzen

[PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling

[Bjorn Helgaas]

On Wed, Apr 19, 2017 at 7:25 PM, Jon Masters <jcm@redhat.com> wrote:
> One additional footnote. I spent a bunch of time recently on my personal
> Xeon systems walking through the PCIe topology and aligning on how to
> advise the ARM server community proceed going forward. If you look at
> how Intel vs AMD handle their host bridges for example, you'll see two
> very different approaches to the case of cross-socket PCIe.

As a learning opportunity for me, can you share "lspci -vv" examples
that show this Intel vs AMD difference?  Maybe the ACPI host bridge
descriptions from dmesg are relevant too?

> But my
> operating assumption is that anything longer term which looks boring and
> x86 enough is probably fine from an ARM server point of view.

That sounds pretty safe to me.

Bjorn

[PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling

[Bjorn Helgaas]

On Mon, Apr 17, 2017 at 12:47 PM, Jayachandran C
<jnair@caviumnetworks.com> wrote:
> On Fri, Apr 14, 2017 at 09:00:06PM -0500, Bjorn Helgaas wrote:
>> On Fri, Apr 14, 2017 at 4:06 PM, Jayachandran C
>> <jnair@caviumnetworks.com> wrote:
>> > On Thu, Apr 13, 2017 at 07:19:11PM -0500, Bjorn Helgaas wrote:
>> >> I tentatively applied both patches to pci/host-thunder for v4.12.
>> >>
>> >> However, I am concerned about the topology here:
>> >>
>> >> On Thu, Apr 13, 2017 at 08:30:45PM +0000, Jayachandran C wrote:
>> >> > On Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the
>> >> > PCI topology is slightly unusual.  For a multi-node system, it looks
>> >> > like:
>> >> >
>> >> >     00:00.0 [PCI] bridge to [bus 01-1e]
>> >> >     01:0a.0 [PCI-PCIe bridge, type 8] bridge to [bus 02-04]
>> >> >     02:00.0 [PCIe root port, type 4] bridge to [bus 03-04] (XLATE_ROOT)
>> >> >     03:00.0 PCIe Endpoint
>> >>
>> >> A root port normally has a single PCIe link leading downstream.
>> >> According to this, 02:00.0 is a root port that has the usual
>> >> downstream link leading to 03:00.0, but it also has an upstream link
>> >> to 01:0a.0.
>> >
>> > The PCI topology is a bit broken due to the way that the PCIe IP block
>> > was integrated into SoC PCI bridges and devices. The current mechanism
>> > of adding a PCI-PCIe bridge to glue these together is not ideal.
>>
>> Yeah, that's definitely broken.
>>
>> >> Maybe this example is omitting details that are not relevant to DMA
>> >> aliases?  The PCIe capability only contains one set of link-related
>> >> registers, so I don't know how we could manage a single device that
>> >> has two links.
>> >
>> > The root port is standard and has just one link to the EP (or whatever
>> > is on the external PCIe slot). The fallout of the current hw design is
>> > that the PCI-PCIe bridge has a link that does not follow standard and
>> > does not have a counterpart (as you noted).
>> >
>> >> A device with two links would break things like ASPM.  In
>> >> set_pcie_port_type(), for example, we have this comment:
>> >>
>> >>    * A Root Port or a PCI-to-PCIe bridge is always the upstream end
>> >>    * of a Link.  No PCIe component has two Links.  Two Links are
>> >>    * connected by a Switch that has a Port on each Link and internal
>> >>    * logic to connect the two Ports.
>> >>
>> >> The topology above breaks these assumptions, which will make
>> >> pdev->has_secondary_link incorrect, which means ASPM won't work
>> >> correctly.
>> >
>> > Given the current hardware, the pcieport driver seems to work reasonably
>> > for the root port at 02:00.0, with AER support. I will take a look at the
>> > ASPM part.
>>
>> I don't think pcieport itself cares much about links.  ASPM does, but
>> it looks like set_pcie_port_type() actually is smart enough to know
>> that PCI-to-PCIe bridges and Root Ports always have links on their
>> secondary sides.  So has_secondary_link probably does get set
>> correctly.
>>
>> But I think you'll hit the VIA "strange chipset" thing in
>> pcie_aspm_init_link_state(), which will probably prevent us from doing
>> ASPM on the link from 02:00.0 to 03:00.0.
>>
>> Could you collect "lspci -vv" output from this system?  I'd like to
>> archive that as background for this IOMMU issue and the ASPM tweaks I
>> suspect we'll have to do.  I *wish* we had more information about that
>> VIA thing, because I suspect we could get rid of it if we had more
>> details.
>
> The full logs are slightly large, so I have kept them at:
> https://github.com/jchandra-cavm/thunderx2/blob/master/logs/
> The lspci -vv output is lspci-vv.txt and lspci -tvn output is lspci-tvn.txt
>
> The output is from 2 socket system, the cards are not on the first slot
> like the example above, so the bus and device numbers are different.

Can somebody with this system collect the "lspci -xxxx" output as well?

I'm making some lspci changes to handle the PCI-to-PCIe bridge
correctly, and I can use the "lspci -xxxx" output to create an lspci
test case.

[PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling

[Jayachandran C]

On Fri, Apr 21, 2017 at 10:48:15AM -0500, Bjorn Helgaas wrote:
> On Mon, Apr 17, 2017 at 12:47 PM, Jayachandran C
> <jnair@caviumnetworks.com> wrote:
> > On Fri, Apr 14, 2017 at 09:00:06PM -0500, Bjorn Helgaas wrote:
> >> On Fri, Apr 14, 2017 at 4:06 PM, Jayachandran C
> >> <jnair@caviumnetworks.com> wrote:
> >> > On Thu, Apr 13, 2017 at 07:19:11PM -0500, Bjorn Helgaas wrote:
> >> >> I tentatively applied both patches to pci/host-thunder for v4.12.
> >> >>
> >> >> However, I am concerned about the topology here:
> >> >>
> >> >> On Thu, Apr 13, 2017 at 08:30:45PM +0000, Jayachandran C wrote:
> >> >> > On Cavium ThunderX2 arm64 SoCs (called Broadcom Vulcan earlier), the
> >> >> > PCI topology is slightly unusual.  For a multi-node system, it looks
> >> >> > like:
> >> >> >
> >> >> >     00:00.0 [PCI] bridge to [bus 01-1e]
> >> >> >     01:0a.0 [PCI-PCIe bridge, type 8] bridge to [bus 02-04]
> >> >> >     02:00.0 [PCIe root port, type 4] bridge to [bus 03-04] (XLATE_ROOT)
> >> >> >     03:00.0 PCIe Endpoint
> >> >>
> >> >> A root port normally has a single PCIe link leading downstream.
> >> >> According to this, 02:00.0 is a root port that has the usual
> >> >> downstream link leading to 03:00.0, but it also has an upstream link
> >> >> to 01:0a.0.
> >> >
> >> > The PCI topology is a bit broken due to the way that the PCIe IP block
> >> > was integrated into SoC PCI bridges and devices. The current mechanism
> >> > of adding a PCI-PCIe bridge to glue these together is not ideal.
> >>
> >> Yeah, that's definitely broken.
> >>
> >> >> Maybe this example is omitting details that are not relevant to DMA
> >> >> aliases?  The PCIe capability only contains one set of link-related
> >> >> registers, so I don't know how we could manage a single device that
> >> >> has two links.
> >> >
> >> > The root port is standard and has just one link to the EP (or whatever
> >> > is on the external PCIe slot). The fallout of the current hw design is
> >> > that the PCI-PCIe bridge has a link that does not follow standard and
> >> > does not have a counterpart (as you noted).
> >> >
> >> >> A device with two links would break things like ASPM.  In
> >> >> set_pcie_port_type(), for example, we have this comment:
> >> >>
> >> >>    * A Root Port or a PCI-to-PCIe bridge is always the upstream end
> >> >>    * of a Link.  No PCIe component has two Links.  Two Links are
> >> >>    * connected by a Switch that has a Port on each Link and internal
> >> >>    * logic to connect the two Ports.
> >> >>
> >> >> The topology above breaks these assumptions, which will make
> >> >> pdev->has_secondary_link incorrect, which means ASPM won't work
> >> >> correctly.
> >> >
> >> > Given the current hardware, the pcieport driver seems to work reasonably
> >> > for the root port at 02:00.0, with AER support. I will take a look at the
> >> > ASPM part.
> >>
> >> I don't think pcieport itself cares much about links.  ASPM does, but
> >> it looks like set_pcie_port_type() actually is smart enough to know
> >> that PCI-to-PCIe bridges and Root Ports always have links on their
> >> secondary sides.  So has_secondary_link probably does get set
> >> correctly.
> >>
> >> But I think you'll hit the VIA "strange chipset" thing in
> >> pcie_aspm_init_link_state(), which will probably prevent us from doing
> >> ASPM on the link from 02:00.0 to 03:00.0.
> >>
> >> Could you collect "lspci -vv" output from this system?  I'd like to
> >> archive that as background for this IOMMU issue and the ASPM tweaks I
> >> suspect we'll have to do.  I *wish* we had more information about that
> >> VIA thing, because I suspect we could get rid of it if we had more
> >> details.
> >
> > The full logs are slightly large, so I have kept them at:
> > https://github.com/jchandra-cavm/thunderx2/blob/master/logs/
> > The lspci -vv output is lspci-vv.txt and lspci -tvn output is lspci-tvn.txt
> >
> > The output is from 2 socket system, the cards are not on the first slot
> > like the example above, so the bus and device numbers are different.
> 
> Can somebody with this system collect the "lspci -xxxx" output as well?
> 
> I'm making some lspci changes to handle the PCI-to-PCIe bridge
> correctly, and I can use the "lspci -xxxx" output to create an lspci
> test case.

[Sorry was AFK for a few days]

I have updated the above directory with the log. Also tested your next branch
and it works fine on ThunderX2.

JC.

[PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling

[Bjorn Helgaas]

On Fri, Apr 21, 2017 at 05:05:41PM +0000, Jayachandran C wrote:
> On Fri, Apr 21, 2017 at 10:48:15AM -0500, Bjorn Helgaas wrote:
> > On Mon, Apr 17, 2017 at 12:47 PM, Jayachandran C
> > <jnair@caviumnetworks.com> wrote:
> > > On Fri, Apr 14, 2017 at 09:00:06PM -0500, Bjorn Helgaas wrote:

> > >> Could you collect "lspci -vv" output from this system?  I'd like to
> > >> archive that as background for this IOMMU issue and the ASPM tweaks I
> > >> suspect we'll have to do.  I *wish* we had more information about that
> > >> VIA thing, because I suspect we could get rid of it if we had more
> > >> details.
> > >
> > > The full logs are slightly large, so I have kept them at:
> > > https://github.com/jchandra-cavm/thunderx2/blob/master/logs/
> > > The lspci -vv output is lspci-vv.txt and lspci -tvn output is lspci-tvn.txt
> > >
> > > The output is from 2 socket system, the cards are not on the first slot
> > > like the example above, so the bus and device numbers are different.
> > 
> > Can somebody with this system collect the "lspci -xxxx" output as well?
> > 
> > I'm making some lspci changes to handle the PCI-to-PCIe bridge
> > correctly, and I can use the "lspci -xxxx" output to create an lspci
> > test case.
> 
> [Sorry was AFK for a few days]
> 
> I have updated the above directory with the log. Also tested your next branch
> and it works fine on ThunderX2.

Thanks!

With regard to my lspci changes, they add "Slot-" here:

   01:0a.0 PCI bridge: Broadcom Corporation Device 9039
   ...
  -   Capabilities: [40] Express (v2) PCI/PCI-X to PCI-Express Bridge, MSI 00
  +   Capabilities: [40] Express (v2) PCI/PCI-X to PCI-Express Bridge (Slot-), MSI 00

for all your PCI-to-PCIe bridges.  I assume the "Slot-" is correct, i.e.,
the link is not connected to a slot, right?  This comes from the "Slot
Implemented" bit in the PCIe Capabilities Register.

I did notice that all the Root Port devices claim to *not* be connected to
slots, which doesn't seem right.  For example,

  12:00.0 PCI bridge: Broadcom Corporation Device 9084
      Bus: primary=12, secondary=13, subordinate=14, sec-latency=0
      Capabilities: [ac] Express (v2) Root Port (Slot-), MSI 00

  13:00.0 Ethernet controller: Intel Corporation 82599EB 10-Gigabit SFI/SFP+ Network Connection

It seems strange because the 12:00.0 Root Port looks like it probably
*does* lead to a slot where the NIC is plugged in.  Or is that NIC really
soldered down?

But I assume there are *some* PCIe slots, so at some of those Root Ports
should advertise "Slot+" (which by itself does not imply hotplug support,
if that's the concern).

Bjorn

[PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling

[Jayachandran C]

On Fri, Apr 21, 2017 at 12:57:05PM -0500, Bjorn Helgaas wrote:
> On Fri, Apr 21, 2017 at 05:05:41PM +0000, Jayachandran C wrote:
> > On Fri, Apr 21, 2017 at 10:48:15AM -0500, Bjorn Helgaas wrote:
> > > On Mon, Apr 17, 2017 at 12:47 PM, Jayachandran C
> > > <jnair@caviumnetworks.com> wrote:
> > > > On Fri, Apr 14, 2017 at 09:00:06PM -0500, Bjorn Helgaas wrote:
> 
> > > >> Could you collect "lspci -vv" output from this system?  I'd like to
> > > >> archive that as background for this IOMMU issue and the ASPM tweaks I
> > > >> suspect we'll have to do.  I *wish* we had more information about that
> > > >> VIA thing, because I suspect we could get rid of it if we had more
> > > >> details.
> > > >
> > > > The full logs are slightly large, so I have kept them at:
> > > > https://github.com/jchandra-cavm/thunderx2/blob/master/logs/
> > > > The lspci -vv output is lspci-vv.txt and lspci -tvn output is lspci-tvn.txt
> > > >
> > > > The output is from 2 socket system, the cards are not on the first slot
> > > > like the example above, so the bus and device numbers are different.
> > > 
> > > Can somebody with this system collect the "lspci -xxxx" output as well?
> > > 
> > > I'm making some lspci changes to handle the PCI-to-PCIe bridge
> > > correctly, and I can use the "lspci -xxxx" output to create an lspci
> > > test case.
> > 
> > [Sorry was AFK for a few days]
> > 
> > I have updated the above directory with the log. Also tested your next branch
> > and it works fine on ThunderX2.
> 
> Thanks!
> 
> With regard to my lspci changes, they add "Slot-" here:
> 
>    01:0a.0 PCI bridge: Broadcom Corporation Device 9039
>    ...
>   -   Capabilities: [40] Express (v2) PCI/PCI-X to PCI-Express Bridge, MSI 00
>   +   Capabilities: [40] Express (v2) PCI/PCI-X to PCI-Express Bridge (Slot-), MSI 00
> 
> for all your PCI-to-PCIe bridges.  I assume the "Slot-" is correct, i.e.,
> the link is not connected to a slot, right?  This comes from the "Slot
> Implemented" bit in the PCIe Capabilities Register.

Yes, Slot- should be correct.
 
> I did notice that all the Root Port devices claim to *not* be connected to
> slots, which doesn't seem right.  For example,
> 
>   12:00.0 PCI bridge: Broadcom Corporation Device 9084
>       Bus: primary=12, secondary=13, subordinate=14, sec-latency=0
>       Capabilities: [ac] Express (v2) Root Port (Slot-), MSI 00
> 
>   13:00.0 Ethernet controller: Intel Corporation 82599EB 10-Gigabit SFI/SFP+ Network Connection
> 
> It seems strange because the 12:00.0 Root Port looks like it probably
> *does* lead to a slot where the NIC is plugged in.  Or is that NIC really
> soldered down?
> 
> But I assume there are *some* PCIe slots, so at some of those Root Ports
> should advertise "Slot+" (which by itself does not imply hotplug support,
> if that's the concern).

The Root Ports are connected to a slot, so I am not sure why the slot implemented
bit is not set. There seems to be nothing useful in the slot capabilites, so this
may be ok for now. I have reported this to the hardware team.

Thanks for the review and the comments,
JC.

[PATCH v5 2/2] PCI: quirks: Fix ThunderX2 dma alias handling

[Bjorn Helgaas]

On Tue, Apr 25, 2017 at 8:03 AM, Jayachandran C
<jnair@caviumnetworks.com> wrote:
> On Fri, Apr 21, 2017 at 12:57:05PM -0500, Bjorn Helgaas wrote:

>> I did notice that all the Root Port devices claim to *not* be connected to
>> slots, which doesn't seem right.  For example,
>>
>>   12:00.0 PCI bridge: Broadcom Corporation Device 9084
>>       Bus: primary=12, secondary=13, subordinate=14, sec-latency=0
>>       Capabilities: [ac] Express (v2) Root Port (Slot-), MSI 00
>>
>>   13:00.0 Ethernet controller: Intel Corporation 82599EB 10-Gigabit SFI/SFP+ Network Connection
>>
>> It seems strange because the 12:00.0 Root Port looks like it probably
>> *does* lead to a slot where the NIC is plugged in.  Or is that NIC really
>> soldered down?
>>
>> But I assume there are *some* PCIe slots, so at some of those Root Ports
>> should advertise "Slot+" (which by itself does not imply hotplug support,
>> if that's the concern).
>
> The Root Ports are connected to a slot, so I am not sure why the slot implemented
> bit is not set. There seems to be nothing useful in the slot capabilites, so this
> may be ok for now. I have reported this to the hardware team.

Thanks.  The "Slot Implemented" bit and the slot registers aren't
essential if you don't want to support hotplug on those slots.  But
even without hotplug, they do contain things like a slot number, which
may be useful in the user interface.