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

[Mika Westerberg <mika.westerberg@linux.intel.com>]

On Tue, Oct 28, 2025 at 12:06:39PM -0500, Bjorn Helgaas wrote:
> On Tue, Oct 28, 2025 at 07:04:27AM +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):
> > 
> >   - 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.
> > 
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> > Previous version can be seen:
> > 
> >   https://lore.kernel.org/linux-pci/20251021104833.3729120-1-mika.westerberg@linux.intel.com/
> > 
> > Changes from the previous version:
> > 
> >   - Limit Switch Upstream Port only to Responder, not both Requester and
> >     Responder.
> > 
> >  drivers/pci/pcie/ptm.c | 31 +++++++++++++++++++++++++++----
> >  1 file changed, 27 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
> > index 65e4b008be00..5ebb2edb4dec 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)
> > @@ -125,7 +140,7 @@ static int __pci_enable_ptm(struct pci_dev *dev)
> >  {
> >  	u16 ptm = dev->ptm_cap;
> >  	struct pci_dev *ups;
> > -	u32 ctrl;
> > +	u32 cap, ctrl;
> >  
> >  	if (!ptm)
> >  		return -EINVAL;
> > @@ -144,6 +159,14 @@ static int __pci_enable_ptm(struct pci_dev *dev)
> >  			return -EINVAL;
> >  	}
> >  
> > +	/*
> > +	 * 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;
> 
> Isn't this going to prevent enabling PTM on Root Ports?

Isn't this function called only for Endpoints? Root Ports and Switch Ports
are enabled in pci_ptm_init() instead.

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