Re: [PATCH v5] PCI/PTM: Enable PTM only if it advertises a role

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Nov 12, 2025 at 08:46:14AM +0100, Mika Westerberg wrote:
> We have a Upstream Port (2b:00.0) that has following in the PTM capability:
> 
>   Capabilities: [220 v1] Precision Time Measurement
> 		PTMCap: Requester- Responder- Root-

Sec 7.9.15.2 says "Switches supporting PTM must Set PTM Responder
Capable".  To me that sounds like "Switches with a PTM Capability must
Set PTM Responder Capable".  But I guess whoever designed this device
didn't think the same way.

> Linux enables PTM for this without looking into what roles it actually
> supports. Immediately after enabling PTM we start getting these:
> 
>   pci 0000:2b:00.0: [8086:5786] type 01 class 0x060400 PCIe Switch Upstream Port
>   ...
>   pci 0000:2b:00.0: PTM enabled, 4ns granularity
>   ...
>   pcieport 0000:00:07.1: AER: Multiple Uncorrectable (Non-Fatal) error message received from 0000:00:07.1
>   pcieport 0000:00:07.1: PCIe Bus Error: severity=Uncorrectable (Non-Fatal), type=Transaction Layer, (Receiver ID)
>   pcieport 0000:00:07.1:   device [8086:e44f] error status/mask=00200000/00000000
>   pcieport 0000:00:07.1:    [21] ACSViol                (First)
>   pcieport 0000:00:07.1: AER:   TLP Header: 0x34000000 0x00000052 0x00000000 0x00000000
> 
> Fix this by enabling PTM only if any of the following conditions are
> true (see more 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.
> 
> While there make the enabling happen for all in __pci_enable_ptm() instead
> of enabling some in pci_ptm_init() and some in __pci_enable_ptm().
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Applied to pci/ptm for v6.19, thanks!

> ---
> Hi,
> 
> I hope I did not make any stupid mistakes this time ;-) My testing still
> passed: the Root Port that has ->ptm_root (and ->ptm_responder) PTM is
> enabled and the Switch Upstream Port that does not have ->ptm_responder is
> not enabled (and I don't see the flood of AER errors).
>
> Previous versions can be seen:
> 
>   v4: https://lore.kernel.org/linux-pci/20251111061048.681752-1-mika.westerberg@linux.intel.com/
>   v3: https://lore.kernel.org/linux-pci/20251030134606.3782352-1-mika.westerberg@linux.intel.com/
>   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 v4:
> 
>   - Do not enable PTM automatically for all components (e.g keep the
>     existing behavior).
>   - Make the switch-case new lines consistent.
> 
> Changes from v3:
> 
>   - Cache the responder and requester capability bits.
>   - Enable PTM only in __pci_enable_ptm().
>   - Update $subject and commit message.
>   - Since this is changed quite a lot, I dropped the Reviewed-by from Lukas
>     and also stable tag.
> 
> 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 | 35 +++++++++++++++++++++++++++++++++++
>  include/linux/pci.h    |  2 ++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/drivers/pci/pcie/ptm.c b/drivers/pci/pcie/ptm.c
> index 65e4b008be00..fb1f3d0d8448 100644
> --- a/drivers/pci/pcie/ptm.c
> +++ b/drivers/pci/pcie/ptm.c
> @@ -81,6 +81,11 @@ void pci_ptm_init(struct pci_dev *dev)
>  		dev->ptm_granularity = 0;
>  	}
>  
> +	if (cap & PCI_PTM_CAP_RES)
> +		dev->ptm_responder = 1;
> +	if (cap & PCI_PTM_CAP_REQ)
> +		dev->ptm_requester = 1;
> +
>  	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
>  	    pci_pcie_type(dev) == PCI_EXP_TYPE_UPSTREAM)
>  		pci_enable_ptm(dev, NULL);
> @@ -144,6 +149,36 @@ static int __pci_enable_ptm(struct pci_dev *dev)
>  			return -EINVAL;
>  	}
>  
> +	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)
> +			return -EINVAL;
> +		break;
> +	case PCI_EXP_TYPE_UPSTREAM:
> +		/*
> +		 * Switch Upstream Ports must at least declare Responder
> +		 * Capable if we want to enable PTM for it.
> +		 */
> +		if (!dev->ptm_responder)
> +			return -EINVAL;
> +		break;
> +	case PCI_EXP_TYPE_ENDPOINT:
> +	case PCI_EXP_TYPE_LEG_END:
> +		/*
> +		 * PCIe Endpoint must declare Requester Capable before we
> +		 * can enable PTM for it.
> +		 */
> +		if (!dev->ptm_requester)
> +			return -EINVAL;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
>  	pci_read_config_dword(dev, ptm + PCI_PTM_CTRL, &ctrl);
>  
>  	ctrl |= PCI_PTM_CTRL_ENABLE;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d1fdf81fbe1e..d5018cb5c331 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -500,6 +500,8 @@ struct pci_dev {
>  #ifdef CONFIG_PCIE_PTM
>  	u16		ptm_cap;		/* PTM Capability */
>  	unsigned int	ptm_root:1;
> +	unsigned int	ptm_responder:1;
> +	unsigned int	ptm_requester:1;
>  	unsigned int	ptm_enabled:1;
>  	u8		ptm_granularity;
>  #endif
> -- 
> 2.50.1
>