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

[Bjorn Helgaas <helgaas@kernel.org>]

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?

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