On Thu, May 21, 2020 at 03:37:30PM +0200, Gustavo Pimentel wrote:
> In order to enable PTM feature in a PCIe Endpoint, it is required to
> enable this feature as well in all devices capable (if present) in the
> datapath between the Root complex and the referred Endpoint e.g. PCIe
> switches.
>
> RC <--> Switch (USP) <-> Switch (DSP) <-> EP
>
> According to PCIe specification Rev5 (6.22.3.2) and (7.9.16), in order
> to enable this feature on a PTM capable switch, it's required to write a
> enable bit in the switch upstream port (USP) control register, which after
> that must respond to all PTM request messages received at any of its
> downstream ports (DSP).
>
> The previous implementation verifies if the PCIe switch has the PTM
> feature enabled on both streams ports (USP and DSP). Since the DSP
> doesn't support PTM capability, the previous implementation doesn't
> allow the PTM feature to be enabled in the Endpoint, the current patch
> fixes this.
>
> Fixes: eec097d43100 ("PCI: Add pci_enable_ptm() for drivers to enable PTM on endpoints")
> Signed-off-by: Aditya Paluri <venkata.adityapaluri@xxxxxxxxxxxx>
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@xxxxxxxxxxxx>
> Cc: linux-pci@xxxxxxxxxxxxxxx
> Cc: Joao Pinto <jpinto@xxxxxxxxxxxx>
The existing code is definitely broken. I would prefer to fix this on
the enumeration side, as opposed to walking the tree at enable-time.
Can you try the alternate patch below?
Bjorn
> ---
> drivers/pci/pcie/ptm.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
> index 9361f3a..cd85d44 100644
> --- a/drivers/pci/pcie/ptm.c
> +++ b/drivers/pci/pcie/ptm.c
> @@ -111,6 +111,14 @@ int pci_enable_ptm(struct pci_dev *dev, u8 *granularity)
> */
> if (pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT) {
> ups = pci_upstream_bridge(dev);
> + /*
> + * Per PCIe r5.0, sec 7.9.16, the PTM capability is not
> + * permitted in Switch Downstream Ports
> + */
> + if (ups && ups->hdr_type == PCI_HEADER_TYPE_BRIDGE &&
> + pci_pcie_type(ups) == PCI_EXP_TYPE_DOWNSTREAM)
> + ups = pci_upstream_bridge(ups);
> +
> if (!ups || !ups->ptm_enabled)
> return -EINVAL;
>
commit b9e92258d486 ("PCI/PTM: Inherit Switch Downstream Port PTM settings from Upstream Port")
Author: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
Date: Thu May 21 15:40:07 2020 -0500
PCI/PTM: Inherit Switch Downstream Port PTM settings from Upstream Port
Except for Endpoints, we enable PTM at enumeration-time. Previously we did
not account for the fact that Switch Downstream Ports may not have a PTM
capability; their PTM behavior is controlled by the Upstream Port (PCIe
r5.0, sec 7.9.16). Since Downstream Ports don't have a PTM capability, we
did not mark them as "ptm_enabled", which meant that pci_enable_ptm() on an
Endpoint failed because there was no PTM path to it.
Mark Downstream Ports as "ptm_enabled" if their Upstream Port has PTM
enabled.
Fixes: eec097d43100 ("PCI: Add pci_enable_ptm() for drivers to enable PTM on endpoints")
Reported-by: Aditya Paluri <Venkata.AdityaPaluri@xxxxxxxxxxxx>
Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
index 9361f3aa26ab..0c42573a66d8 100644
--- a/drivers/pci/pcie/ptm.c
+++ b/drivers/pci/pcie/ptm.c
@@ -39,10 +39,6 @@ void pci_ptm_init(struct pci_dev *dev)
if (!pci_is_pcie(dev))
return;
- pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
- if (!pos)
- return;
-
/*
* Enable PTM only on interior devices (root ports, switch ports,
* etc.) on the assumption that it causes no link traffic until an
@@ -52,6 +48,23 @@ void pci_ptm_init(struct pci_dev *dev)
pci_pcie_type(dev) == PCI_EXP_TYPE_RC_END))
return;
+ /*
+ * Switch Downstream Ports may not have a PTM capability; their PTM
+ * behavior is controlled by the Upstream Port (PCIe r5.0, sec
+ * 7.9.16).
+ */
+ ups = pci_upstream_bridge(dev);
+ if (pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM &&
+ ups && ups->ptm_enabled) {
+ dev->ptm_granularity = ups->ptm_granularity;
+ dev->ptm_enabled = 1;
+ return;
+ }
+
+ pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_PTM);
+ if (!pos)
+ return;
+
pci_read_config_dword(dev, pos + PCI_PTM_CAP, &cap);
local_clock = (cap & PCI_PTM_GRANULARITY_MASK) >> 8;
@@ -61,7 +74,6 @@ void pci_ptm_init(struct pci_dev *dev)
* the spec recommendation (PCIe r3.1, sec 7.32.3), select the
* furthest upstream Time Source as the PTM Root.
*/
- ups = pci_upstream_bridge(dev);
if (ups && ups->ptm_enabled) {
ctrl = PCI_PTM_CTRL_ENABLE;
if (ups->ptm_granularity == 0)
