[PATCH v3] PCI/PTM: Do not enable PTM solely based on the capability existense

[PATCH v3] PCI/PTM: Do not enable PTM solely based on the capability existense

[Mika Westerberg]

It is not advisable to enable PTM solely based on the fact that the
capability exists. Instead there are separate bits in the capability
register that need to be set for the feature to be enabled for a given
component (this is suggestion from Intel PCIe folks, and also shown in
PCIe r7.0 sec 6.21.1 figure 6-21):

  - PCIe Endpoint that has PTM capability must to declare requester
    capable
  - PCIe Switch Upstream Port that has PTM capability must declare
    at least responder capable
  - PCIe Root Port must declare root port capable.

Currently we see following:

  pci 0000:01:00.0: [8086:5786] type 01 class 0x060400 PCIe Switch Upstream Port
  pci 0000:01:00.0: PCI bridge to [bus 00]
  pci 0000:01:00.0:   bridge window [io  0x0000-0x0fff]
  pci 0000:01:00.0:   bridge window [mem 0x00000000-0x000fffff]
  pci 0000:01:00.0:   bridge window [mem 0x00000000-0x000fffff 64bit pref]
  ...
  pci 0000:01:00.0: PTM enabled, 4ns granularity
  ...
  pcieport 0000:00:07.0: AER: Multiple Uncorrectable (Non-Fatal) error message received from 0000:00:07.0
  pcieport 0000:00:07.0: PCIe Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
  pcieport 0000:00:07.0:   device [8086:e44e] error status/mask=00200000/00000000
  pcieport 0000:00:07.0:    [21] ACSViol                (First)

The 01:00.0 PCIe Upstream Port has this:

  Capabilities: [220 v1] Precision Time Measurement
		PTMCap: Requester- Responder- Root-

This happens because Linux sees the PTM capability and blindly enables
PTM which then causes the AER error to trigger.

Fix this by enabling PTM only if the above described criteria is met.

Fixes: 9bb04a0c4e26 ("PCI: Add Precision Time Measurement (PTM) support")
Cc: stable@vger.kernel.org  # v4.9+
Reviewed-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
Previous versions can be seen:

  v2: https://lore.kernel.org/linux-pci/20251028060427.2163115-1-mika.westerberg@linux.intel.com/
  v1: https://lore.kernel.org/linux-pci/20251021104833.3729120-1-mika.westerberg@linux.intel.com/

Changes from v2:

  - Limit the check in __pci_enable_ptm() to Endpoints and Legacy
    Endpoints.
  - Added stable tags suggested by Lukas, and PCIe spec reference.
  - Added Reviewed-by tag from Lukas (hope it is okay to keep).

Changes from v1:

  - Limit Switch Upstream Port only to Responder, not both Requester and
    Responder.

 drivers/pci/pcie/ptm.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
index 65e4b008be00..3381bfeaccd7 100644
--- a/drivers/pci/pcie/ptm.c
+++ b/drivers/pci/pcie/ptm.c
@@ -81,9 +81,24 @@ void pci_ptm_init(struct pci_dev *dev)
 		dev->ptm_granularity = 0;
 	}
 
-	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
-	    pci_pcie_type(dev) == PCI_EXP_TYPE_UPSTREAM)
-		pci_enable_ptm(dev, NULL);
+	switch (pci_pcie_type(dev)) {
+	case PCI_EXP_TYPE_ROOT_PORT:
+		/*
+		 * Root Port must declare Root Capable if we want to
+		 * enable PTM for it.
+		 */
+		if (dev->ptm_root)
+			pci_enable_ptm(dev, NULL);
+		break;
+	case PCI_EXP_TYPE_UPSTREAM:
+		/*
+		 * Switch Upstream Ports must at least declare Responder
+		 * Capable if we want to enable PTM for it.
+		 */
+		if (cap & PCI_PTM_CAP_RES)
+			pci_enable_ptm(dev, NULL);
+		break;
+	}
 }
 
 void pci_save_ptm_state(struct pci_dev *dev)
@@ -144,6 +159,18 @@ static int __pci_enable_ptm(struct pci_dev *dev)
 			return -EINVAL;
 	}
 
+	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT ||
+	    pci_pcie_type(dev) == PCI_EXP_TYPE_LEG_END) {
+		u32 cap;
+		/*
+		 * PCIe Endpoint must declare Requester Capable before we
+		 * can enable PTM for it.
+		 */
+		pci_read_config_dword(dev, ptm + PCI_PTM_CAP, &cap);
+		if (!(cap & PCI_PTM_CAP_REQ))
+			return -EINVAL;
+	}
+
 	pci_read_config_dword(dev, ptm + PCI_PTM_CTRL, &ctrl);
 
 	ctrl |= PCI_PTM_CTRL_ENABLE;
-- 
2.50.1

Re: [PATCH v3] PCI/PTM: Do not enable PTM solely based on the capability existense

[Bjorn Helgaas]

In subject, s/existense/existence/

Actually, I'd try to include something more specific like "enable PTM
only if it advertises a role".

On Thu, Oct 30, 2025 at 02:46:05PM +0100, Mika Westerberg wrote:
> It is not advisable to enable PTM solely based on the fact that the
> capability exists. Instead there are separate bits in the capability
> register that need to be set for the feature to be enabled for a given
> component (this is suggestion from Intel PCIe folks, and also shown in
> PCIe r7.0 sec 6.21.1 figure 6-21):

Can we start with a minimal statement of what's wrong?  Is the problem
that 01:00.0 sent a PTM Request Message that 00:07.0 detected as an
ACS violation?

I guess we enabled PTM on 01:00.0 even though it doesn't advertise any
roles in the PTM Capability, and it sent a PTM Request Message anyway?

Weird to expose a PTM Capability and not advertise any roles, and also
weird to send PTM Messages when enabled in that case.

>   - PCIe Endpoint that has PTM capability must to declare requester
>     capable
>   - PCIe Switch Upstream Port that has PTM capability must declare
>     at least responder capable
>   - PCIe Root Port must declare root port capable.
> 
> Currently we see following:
> 
>   pci 0000:01:00.0: [8086:5786] type 01 class 0x060400 PCIe Switch Upstream Port
>   pci 0000:01:00.0: PCI bridge to [bus 00]
>   pci 0000:01:00.0:   bridge window [io  0x0000-0x0fff]
>   pci 0000:01:00.0:   bridge window [mem 0x00000000-0x000fffff]
>   pci 0000:01:00.0:   bridge window [mem 0x00000000-0x000fffff 64bit pref]

I don't think the windows are relevant.

>   pci 0000:01:00.0: PTM enabled, 4ns granularity
>   ...
>   pcieport 0000:00:07.0: AER: Multiple Uncorrectable (Non-Fatal) error message received from 0000:00:07.0
>   pcieport 0000:00:07.0: PCIe Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
>   pcieport 0000:00:07.0:   device [8086:e44e] error status/mask=00200000/00000000
>   pcieport 0000:00:07.0:    [21] ACSViol                (First)

Is there any Header Log info here?  I assume if there is, it would
show a PTM Message?

> The 01:00.0 PCIe Upstream Port has this:
> 
>   Capabilities: [220 v1] Precision Time Measurement
> 		PTMCap: Requester- Responder- Root-
> 
> This happens because Linux sees the PTM capability and blindly enables
> PTM which then causes the AER error to trigger.
> 
> Fix this by enabling PTM only if the above described criteria is met.
> ...

> +++ b/drivers/pci/pcie/ptm.c
> @@ -81,9 +81,24 @@ void pci_ptm_init(struct pci_dev *dev)
>  		dev->ptm_granularity = 0;
>  	}
>  
> -	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
> -	    pci_pcie_type(dev) == PCI_EXP_TYPE_UPSTREAM)
> -		pci_enable_ptm(dev, NULL);
> +	switch (pci_pcie_type(dev)) {
> +	case PCI_EXP_TYPE_ROOT_PORT:
> +		/*
> +		 * Root Port must declare Root Capable if we want to
> +		 * enable PTM for it.
> +		 */
> +		if (dev->ptm_root)
> +			pci_enable_ptm(dev, NULL);
> +		break;
> +	case PCI_EXP_TYPE_UPSTREAM:
> +		/*
> +		 * Switch Upstream Ports must at least declare Responder
> +		 * Capable if we want to enable PTM for it.
> +		 */
> +		if (cap & PCI_PTM_CAP_RES)
> +			pci_enable_ptm(dev, NULL);
> +		break;
> +	}
>  }
>  
>  void pci_save_ptm_state(struct pci_dev *dev)
> @@ -144,6 +159,18 @@ static int __pci_enable_ptm(struct pci_dev *dev)
>  			return -EINVAL;
>  	}
>  
> +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT ||
> +	    pci_pcie_type(dev) == PCI_EXP_TYPE_LEG_END) {
> +		u32 cap;
> +		/*
> +		 * PCIe Endpoint must declare Requester Capable before we
> +		 * can enable PTM for it.
> +		 */
> +		pci_read_config_dword(dev, ptm + PCI_PTM_CAP, &cap);
> +		if (!(cap & PCI_PTM_CAP_REQ))
> +			return -EINVAL;
> +	}

The asymmetry of testing PCI_PTM_CAP_ROOT back in pci_ptm_init() (via
dev->ptm_root) but testing PCI_PTM_CAP_REQ here feels a little
confusing to me.

Also, we already read PCI_PTM_CAP in pci_ptm_init(), and we did cache
ptm_root.  Maybe we should also cache ptm_responder and ptm_requester
and test all of them here in __pci_enable_ptm() and drop the tests in
pci_ptm_init()?

>  	pci_read_config_dword(dev, ptm + PCI_PTM_CTRL, &ctrl);
>  
>  	ctrl |= PCI_PTM_CTRL_ENABLE;
> -- 
> 2.50.1
> 

Re: [PATCH v3] PCI/PTM: Do not enable PTM solely based on the capability existense

[Mika Westerberg]

On Thu, Oct 30, 2025 at 03:59:37PM -0500, Bjorn Helgaas wrote:
> In subject, s/existense/existence/
> 
> Actually, I'd try to include something more specific like "enable PTM
> only if it advertises a role".

Okay.

> On Thu, Oct 30, 2025 at 02:46:05PM +0100, Mika Westerberg wrote:
> > It is not advisable to enable PTM solely based on the fact that the
> > capability exists. Instead there are separate bits in the capability
> > register that need to be set for the feature to be enabled for a given
> > component (this is suggestion from Intel PCIe folks, and also shown in
> > PCIe r7.0 sec 6.21.1 figure 6-21):
> 
> Can we start with a minimal statement of what's wrong?  Is the problem
> that 01:00.0 sent a PTM Request Message that 00:07.0 detected as an
> ACS violation?

The problem is that once the PCIe Switch is hotplugged we get tons of AER
errors like below (here upstream port is 2b:00.0, in the previous example
it was 01:00.0):

[  156.337979] pci 0000:2b:00.0: PTM enabled, 4ns granularity
[  156.350822] pcieport 0000:00:07.1: AER: Multiple Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
[  156.361417] pcieport 0000:00:07.1: PCIe Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver I
D)
[  156.372656] pcieport 0000:00:07.1:   device [8086:e44f] error status/mask=00200000/00000000
[  156.381041] pcieport 0000:00:07.1:    [21] ACSViol                (First)
[  156.387842] pcieport 0000:00:07.1: AER:   TLP Header: 0x34000000 0x00000052 0x00000000 0x00000000
[  156.396731] pcieport 0000:00:07.1: AER: broadcast error_detected message
[  156.403498] pcieport 0000:00:07.1: AER: broadcast mmio_enabled message
[  156.410060] pcieport 0000:00:07.1: AER: broadcast resume message
[  156.416131] pcieport 0000:00:07.1: AER: device recovery successful
[  156.422345] pcieport 0000:00:07.1: AER: Uncorrectable (Non-Fatal) error message received from 0000:00:07.1

Here 00:07.1 is the PCIe Root Port.

> I guess we enabled PTM on 01:00.0 even though it doesn't advertise any
> roles in the PTM Capability, and it sent a PTM Request Message anyway?

Yes, I think so.

> Weird to expose a PTM Capability and not advertise any roles, and also
> weird to send PTM Messages when enabled in that case.
> 
> >   - PCIe Endpoint that has PTM capability must to declare requester
> >     capable
> >   - PCIe Switch Upstream Port that has PTM capability must declare
> >     at least responder capable
> >   - PCIe Root Port must declare root port capable.
> > 
> > Currently we see following:
> > 
> >   pci 0000:01:00.0: [8086:5786] type 01 class 0x060400 PCIe Switch Upstream Port
> >   pci 0000:01:00.0: PCI bridge to [bus 00]
> >   pci 0000:01:00.0:   bridge window [io  0x0000-0x0fff]
> >   pci 0000:01:00.0:   bridge window [mem 0x00000000-0x000fffff]
> >   pci 0000:01:00.0:   bridge window [mem 0x00000000-0x000fffff 64bit pref]
> 
> I don't think the windows are relevant.

Okay.

> >   pci 0000:01:00.0: PTM enabled, 4ns granularity
> >   ...
> >   pcieport 0000:00:07.0: AER: Multiple Uncorrectable (Non-Fatal) error message received from 0000:00:07.0
> >   pcieport 0000:00:07.0: PCIe Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
> >   pcieport 0000:00:07.0:   device [8086:e44e] error status/mask=00200000/00000000
> >   pcieport 0000:00:07.0:    [21] ACSViol                (First)
> 
> Is there any Header Log info here?  I assume if there is, it would
> show a PTM Message?

I pasted it above. Does it tell anything useful to you?

> > The 01:00.0 PCIe Upstream Port has this:
> > 
> >   Capabilities: [220 v1] Precision Time Measurement
> > 		PTMCap: Requester- Responder- Root-
> > 
> > This happens because Linux sees the PTM capability and blindly enables
> > PTM which then causes the AER error to trigger.
> > 
> > Fix this by enabling PTM only if the above described criteria is met.
> > ...
> 
> > +++ b/drivers/pci/pcie/ptm.c
> > @@ -81,9 +81,24 @@ void pci_ptm_init(struct pci_dev *dev)
> >  		dev->ptm_granularity = 0;
> >  	}
> >  
> > -	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
> > -	    pci_pcie_type(dev) == PCI_EXP_TYPE_UPSTREAM)
> > -		pci_enable_ptm(dev, NULL);
> > +	switch (pci_pcie_type(dev)) {
> > +	case PCI_EXP_TYPE_ROOT_PORT:
> > +		/*
> > +		 * Root Port must declare Root Capable if we want to
> > +		 * enable PTM for it.
> > +		 */
> > +		if (dev->ptm_root)
> > +			pci_enable_ptm(dev, NULL);
> > +		break;
> > +	case PCI_EXP_TYPE_UPSTREAM:
> > +		/*
> > +		 * Switch Upstream Ports must at least declare Responder
> > +		 * Capable if we want to enable PTM for it.
> > +		 */
> > +		if (cap & PCI_PTM_CAP_RES)
> > +			pci_enable_ptm(dev, NULL);
> > +		break;
> > +	}
> >  }
> >  
> >  void pci_save_ptm_state(struct pci_dev *dev)
> > @@ -144,6 +159,18 @@ static int __pci_enable_ptm(struct pci_dev *dev)
> >  			return -EINVAL;
> >  	}
> >  
> > +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT ||
> > +	    pci_pcie_type(dev) == PCI_EXP_TYPE_LEG_END) {
> > +		u32 cap;
> > +		/*
> > +		 * PCIe Endpoint must declare Requester Capable before we
> > +		 * can enable PTM for it.
> > +		 */
> > +		pci_read_config_dword(dev, ptm + PCI_PTM_CAP, &cap);
> > +		if (!(cap & PCI_PTM_CAP_REQ))
> > +			return -EINVAL;
> > +	}
> 
> The asymmetry of testing PCI_PTM_CAP_ROOT back in pci_ptm_init() (via
> dev->ptm_root) but testing PCI_PTM_CAP_REQ here feels a little
> confusing to me.
> 
> Also, we already read PCI_PTM_CAP in pci_ptm_init(), and we did cache
> ptm_root.  Maybe we should also cache ptm_responder and ptm_requester
> and test all of them here in __pci_enable_ptm() and drop the tests in
> pci_ptm_init()?

Sure I can do it that way too.

> >  	pci_read_config_dword(dev, ptm + PCI_PTM_CTRL, &ctrl);
> >  
> >  	ctrl |= PCI_PTM_CTRL_ENABLE;
> > -- 
> > 2.50.1
> > 

Re: [PATCH v3] PCI/PTM: Do not enable PTM solely based on the capability existense

[Bjorn Helgaas]

On Fri, Oct 31, 2025 at 07:09:59AM +0100, Mika Westerberg wrote:
> On Thu, Oct 30, 2025 at 03:59:37PM -0500, Bjorn Helgaas wrote:
> > On Thu, Oct 30, 2025 at 02:46:05PM +0100, Mika Westerberg wrote:
> > > It is not advisable to enable PTM solely based on the fact that the
> > > capability exists. Instead there are separate bits in the capability
> > > register that need to be set for the feature to be enabled for a given
> > > component (this is suggestion from Intel PCIe folks, and also shown in
> > > PCIe r7.0 sec 6.21.1 figure 6-21):
> > 
> > Can we start with a minimal statement of what's wrong?  Is the problem
> > that 01:00.0 sent a PTM Request Message that 00:07.0 detected as an
> > ACS violation?
> 
> The problem is that once the PCIe Switch is hotplugged we get tons of AER
> errors like below (here upstream port is 2b:00.0, in the previous example
> it was 01:00.0):
> 
> [  156.337979] pci 0000:2b:00.0: PTM enabled, 4ns granularity
> [  156.350822] pcieport 0000:00:07.1: AER: Multiple Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
> [  156.361417] pcieport 0000:00:07.1: PCIe Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
> [  156.372656] pcieport 0000:00:07.1:   device [8086:e44f] error status/mask=00200000/00000000
> [  156.381041] pcieport 0000:00:07.1:    [21] ACSViol                (First)
> [  156.387842] pcieport 0000:00:07.1: AER:   TLP Header: 0x34000000 0x00000052 0x00000000 0x00000000

If I read this right:

  0x34000000 is 0011 0100 0...0
    Fmt  001    4 DW header, no data (PCIe r7.0, sec 2.2.1.1)
    Type 10100  Message Request, Local - Terminate at Receiver (2.2.1.1, 2.2.8)

  0x00000052 is 0...0 0101 0010
    0x0000     Requester ID
    0101 0010  PTM Request (2.2.8.10)

The fact that the Request ID is 0x0000 and the error is an ACS
Violation looks like the implementation note in sec 6.12.1.1:

  Functions are permitted to transmit Upstream Messages before they
  have been assigned a Bus Number. Such messages will have a Requester
  ID with a Bus Number of 00h. If the Downstream Port has ACS Source
  Validation enabled, these Messages (see Table F-1, Section 2.2.8.2,
  and Section 6.22.1) will likely be detected as an ACS Violation
  error.

So I assume 2b:00.0 sent a PTM Request with Requester ID of 0, and
00:07.1 logged the ACS violation.  It's odd that 2b:00.0 would send a
PTM request if it doesn't advertise the PTM Requester role.  Also odd
that it doesn't seem to know its Bus Number.  It's supposed to capture
that from every config write request (sec 2.2.9.1), and I would think
it should have seen several by now including the one that enable PTM.

But I think your fix is right even if we don't understand exactly how
we got there.  Are you planning an update, or ...?  Just wanted to
make sure we're not waiting for each other.

Bjorn

Re: [PATCH v3] PCI/PTM: Do not enable PTM solely based on the capability existense

[Mika Westerberg]

On Mon, Nov 10, 2025 at 06:10:23PM -0600, Bjorn Helgaas wrote:
> On Fri, Oct 31, 2025 at 07:09:59AM +0100, Mika Westerberg wrote:
> > On Thu, Oct 30, 2025 at 03:59:37PM -0500, Bjorn Helgaas wrote:
> > > On Thu, Oct 30, 2025 at 02:46:05PM +0100, Mika Westerberg wrote:
> > > > It is not advisable to enable PTM solely based on the fact that the
> > > > capability exists. Instead there are separate bits in the capability
> > > > register that need to be set for the feature to be enabled for a given
> > > > component (this is suggestion from Intel PCIe folks, and also shown in
> > > > PCIe r7.0 sec 6.21.1 figure 6-21):
> > > 
> > > Can we start with a minimal statement of what's wrong?  Is the problem
> > > that 01:00.0 sent a PTM Request Message that 00:07.0 detected as an
> > > ACS violation?
> > 
> > The problem is that once the PCIe Switch is hotplugged we get tons of AER
> > errors like below (here upstream port is 2b:00.0, in the previous example
> > it was 01:00.0):
> > 
> > [  156.337979] pci 0000:2b:00.0: PTM enabled, 4ns granularity
> > [  156.350822] pcieport 0000:00:07.1: AER: Multiple Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
> > [  156.361417] pcieport 0000:00:07.1: PCIe Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
> > [  156.372656] pcieport 0000:00:07.1:   device [8086:e44f] error status/mask=00200000/00000000
> > [  156.381041] pcieport 0000:00:07.1:    [21] ACSViol                (First)
> > [  156.387842] pcieport 0000:00:07.1: AER:   TLP Header: 0x34000000 0x00000052 0x00000000 0x00000000
> 
> If I read this right:
> 
>   0x34000000 is 0011 0100 0...0
>     Fmt  001    4 DW header, no data (PCIe r7.0, sec 2.2.1.1)
>     Type 10100  Message Request, Local - Terminate at Receiver (2.2.1.1, 2.2.8)
> 
>   0x00000052 is 0...0 0101 0010
>     0x0000     Requester ID
>     0101 0010  PTM Request (2.2.8.10)
> 
> The fact that the Request ID is 0x0000 and the error is an ACS
> Violation looks like the implementation note in sec 6.12.1.1:
> 
>   Functions are permitted to transmit Upstream Messages before they
>   have been assigned a Bus Number. Such messages will have a Requester
>   ID with a Bus Number of 00h. If the Downstream Port has ACS Source
>   Validation enabled, these Messages (see Table F-1, Section 2.2.8.2,
>   and Section 6.22.1) will likely be detected as an ACS Violation
>   error.

Okay thanks for looking.

> So I assume 2b:00.0 sent a PTM Request with Requester ID of 0, and
> 00:07.1 logged the ACS violation.  It's odd that 2b:00.0 would send a
> PTM request if it doesn't advertise the PTM Requester role.  Also odd
> that it doesn't seem to know its Bus Number.  It's supposed to capture
> that from every config write request (sec 2.2.9.1), and I would think
> it should have seen several by now including the one that enable PTM.

Indeed but for some reason the flood of AER ACS violations start
immediately after we enabled it. Even if it has the bus number already
assigned.

> But I think your fix is right even if we don't understand exactly how
> we got there.  Are you planning an update, or ...?  Just wanted to
> make sure we're not waiting for each other.

Yes, I have the new version ready. Tested it yesterday and was planning to
send it out today.