Re: [PATCH] PCI/TPH: Fix get cpu steer-tag fail on ARM64 platform

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Jeremy, Sunil, Huacai, Liupu, authors of get_acpi_id_for_cpu()
for arm64, riscv, loongson]

On Tue, Mar 03, 2026 at 08:36:25AM +0800, Chengwen Feng wrote:
> Currently the pcie_tph_get_cpu_st() has a problem on ARM64 platform:
> 1. The pcie_tph_get_cpu_st() function directly uses cpu_uid as the input
>    parameter to call the PCI ACPI DSM method. According to the DSM
>    definition, the input value should be the ACPI Processor UID. For
>    details, please see [1].
>
> 2. In the Broadcom driver implementation [2] (which invoke
>    pcie_tph_get_cpu_st()), cpu_uid is obtained based on
>    cpumask_first(irq->cpu_mask), that is the logical ID of a CPU core,
>    which is generated and managed by the kernel. For example, [0,255]
>    if the system has 256 logical CPU cores.
> 3. Unfortunately, on ARM64 platform, ACPI assigns Processor UID to the
>    core which listed in the MADT table, the Processor UID may not equal
>    the logical ID of a CPU core in the kernel. So the current
>    implementation cannot obtain the cpu's real steer-tag in such case.
> 4. The reason why it can run on the AMD platform is that the mapping
>    between the logical ID and ACPI Processor UID is similar.
> 
> This commit fixes it by:
> 1. Introduce config ARCH_HAS_GET_CPU_ACPI_ID_API and its corresponding
>    API acpi_get_cpu_acpi_id() to obtain the ACPI Processor UID of a CPU
>    core. This API invokes get_acpi_id_for_cpu() to obtain the UID on
>    ARM64 platform.
> 2. Because using the logical ID as the ACPI Processor UID directly on
>    X86 platform is not standard. This commit uses cpu_acpi_id() to
>    obtain the UID.
> 3. At the same time, the input parameter cpu_uid of
>    pcie_tph_get_cpu_st() is renamed to cpu for clarity.

Thanks for raising this issue!

TLP Processing Hints (TPH) and Steering Tags are generic PCIe features
that we should support for both ACPI and non-ACPI systems.

The current implementation of pcie_tph_get_cpu_st() only supports
ACPI, and it assumes the cpu_uid parameter is an ACPI CPU UID that can
be passed directly to the _DSM.  But since we want this to be a
generic interface, I think the "cpu" parameter should be the Linux
logical CPU ID, not an ACPI UID, as you point out.

> [1] According to the _DSM ECN, the input is defined as: "If the target
>     is a processor, then this field represents the ACPI Processor
>     UID of the processor as specified in the MADT. If the target is
>     a processor container, then this field represents the ACPI
>     Processor UID of the processor container as specified in the
>     PPTT."

This needs a specific spec citation for the _DSM function.  Ideally it
would be "PCI Firmware spec r3.3, sec xx", but I don't think there's a
revision of the spec that includes this ECN.  But we can at least
include the actual name and URL for the approved ECN.

> [2] commit c214410c47d6e ("bnxt_en: Add TPH support in BNXT driver")
> 
> Fixes: d2e8a34876ce ("PCI/TPH: Add Steering Tag support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>  Documentation/PCI/tph.rst     |  4 ++--
>  drivers/acpi/Kconfig          |  8 ++++++++
>  drivers/acpi/processor_core.c | 13 +++++++++++++
>  drivers/pci/tph.c             | 13 +++++++------
>  include/linux/acpi.h          |  4 ++++
>  include/linux/pci-tph.h       |  4 ++--
>  6 files changed, 36 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/PCI/tph.rst b/Documentation/PCI/tph.rst
> index e8993be64fd6..b6cf22b9bd90 100644
> --- a/Documentation/PCI/tph.rst
> +++ b/Documentation/PCI/tph.rst
> @@ -79,10 +79,10 @@ To retrieve a Steering Tag for a target memory associated with a specific
>  CPU, use the following function::
>  
>    int pcie_tph_get_cpu_st(struct pci_dev *pdev, enum tph_mem_type type,
> -                          unsigned int cpu_uid, u16 *tag);
> +                          unsigned int cpu, u16 *tag);
>  
>  The `type` argument is used to specify the memory type, either volatile
> -or persistent, of the target memory. The `cpu_uid` argument specifies the
> +or persistent, of the target memory. The `cpu` argument specifies the
>  CPU where the memory is associated to.
>  
>  After the ST value is retrieved, the device driver can use the following
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index df0ff0764d0d..9d851a017cd1 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -606,6 +606,14 @@ config ACPI_PRMT
>  	  substantially increase computational overhead related to the
>  	  initialization of some server systems.
>  
> +config ARCH_HAS_GET_CPU_ACPI_ID_API
> +	bool "Architecture supports get cpu's ACPI Processor UID"
> +	depends on (X86 || ARM64)
> +	default y
> +	help
> +	  This config indicates whether the architecture provides a standard
> +	  API to get ACPI Processor UID of a cpu from MADT table.
> +
>  endif	# ACPI
>  
>  config X86_PM_TIMER
> diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
> index a4498357bd16..6150f5bdb62e 100644
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -335,6 +335,19 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
>  }
>  EXPORT_SYMBOL_GPL(acpi_get_cpuid);
>  
> +#ifdef CONFIG_ARCH_HAS_GET_CPU_ACPI_ID_API
> +unsigned int acpi_get_cpu_acpi_id(unsigned int cpu)
> +{
> +	if (cpu >= nr_cpu_ids)
> +		return 0;
> +#ifdef CONFIG_X86
> +	return cpu_acpi_id(cpu);
> +#elif CONFIG_ARM64
> +	return get_acpi_id_for_cpu(cpu);
> +#endif
> +}
> +#endif /* CONFIG_ARCH_HAS_GET_CPU_ACPI_ID_API */
> +
>  #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
>  static int get_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base,
>  			 u64 *phys_addr, int *ioapic_id)
> diff --git a/drivers/pci/tph.c b/drivers/pci/tph.c
> index ca4f97be7538..a47c2fbb6148 100644
> --- a/drivers/pci/tph.c
> +++ b/drivers/pci/tph.c
> @@ -236,18 +236,19 @@ static int write_tag_to_st_table(struct pci_dev *pdev, int index, u16 tag)
>   * with a specific CPU
>   * @pdev: PCI device
>   * @mem_type: target memory type (volatile or persistent RAM)
> - * @cpu_uid: associated CPU id
> + * @cpu: associated CPU id
>   * @tag: Steering Tag to be returned
>   *
>   * Return the Steering Tag for a target memory that is associated with a
> - * specific CPU as indicated by cpu_uid.
> + * specific CPU as indicated by cpu.
>   *
>   * Return: 0 if success, otherwise negative value (-errno)
>   */
>  int pcie_tph_get_cpu_st(struct pci_dev *pdev, enum tph_mem_type mem_type,
> -			unsigned int cpu_uid, u16 *tag)
> +			unsigned int cpu, u16 *tag)
>  {
> -#ifdef CONFIG_ACPI
> +#ifdef CONFIG_ARCH_HAS_GET_CPU_ACPI_ID_API
> +	unsigned int cpu_uid = acpi_get_cpu_acpi_id(cpu);

Any required conversion between Linux logical CPU and ACPI CPU UID
should be internal to pcie_tph_get_cpu_st(), as you're doing here.

But rather than adding CONFIG_ARCH_HAS_GET_CPU_ACPI_ID_API and the
ifdefs in acpi_get_cpu_acpi_id(), I think there should be a generic
ACPI interface that converts logical CPU ID to ACPI UID, and every
arch supporting ACPI should have to implement it.

We already have get_acpi_id_for_cpu(), implemented for arm64, riscv,
and loongarch:

  30d87bfacbee ("arm64/acpi: Create arch specific cpu to acpi id helper")
  f99561199470 ("RISC-V: ACPI: Cache and retrieve the RINTC structure")
  f6f0c9a74a48 ("LoongArch: Add SMT (Simultaneous Multi-Threading) support")

What if we just implemented it for x86 as well and moved it to
include/linux/acpi.h or similar?

>  	struct pci_dev *rp;
>  	acpi_handle rp_acpi_handle;
>  	union st_info info;
> @@ -265,9 +266,9 @@ int pcie_tph_get_cpu_st(struct pci_dev *pdev, enum tph_mem_type mem_type,
>  
>  	*tag = tph_extract_tag(mem_type, pdev->tph_req_type, &info);
>  
> -	pci_dbg(pdev, "get steering tag: mem_type=%s, cpu_uid=%d, tag=%#04x\n",
> +	pci_dbg(pdev, "get steering tag: mem_type=%s, cpu=%d, tag=%#04x\n",
>  		(mem_type == TPH_MEM_TYPE_VM) ? "volatile" : "persistent",
> -		cpu_uid, *tag);
> +		cpu, *tag);
>  
>  	return 0;
>  #else
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 4d2f0bed7a06..789bfcb8e0f3 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -324,6 +324,10 @@ int acpi_unmap_cpu(int cpu);
>  
>  acpi_handle acpi_get_processor_handle(int cpu);
>  
> +#ifdef CONFIG_ARCH_HAS_GET_CPU_ACPI_ID_API
> +unsigned int acpi_get_cpu_acpi_id(unsigned int cpu);
> +#endif
> +
>  #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
>  int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr);
>  #endif
> diff --git a/include/linux/pci-tph.h b/include/linux/pci-tph.h
> index ba28140ce670..be68cd17f2f8 100644
> --- a/include/linux/pci-tph.h
> +++ b/include/linux/pci-tph.h
> @@ -25,7 +25,7 @@ int pcie_tph_set_st_entry(struct pci_dev *pdev,
>  			  unsigned int index, u16 tag);
>  int pcie_tph_get_cpu_st(struct pci_dev *dev,
>  			enum tph_mem_type mem_type,
> -			unsigned int cpu_uid, u16 *tag);
> +			unsigned int cpu, u16 *tag);
>  void pcie_disable_tph(struct pci_dev *pdev);
>  int pcie_enable_tph(struct pci_dev *pdev, int mode);
>  u16 pcie_tph_get_st_table_size(struct pci_dev *pdev);
> @@ -36,7 +36,7 @@ static inline int pcie_tph_set_st_entry(struct pci_dev *pdev,
>  { return -EINVAL; }
>  static inline int pcie_tph_get_cpu_st(struct pci_dev *dev,
>  				      enum tph_mem_type mem_type,
> -				      unsigned int cpu_uid, u16 *tag)
> +				      unsigned int cpu, u16 *tag)
>  { return -EINVAL; }
>  static inline void pcie_disable_tph(struct pci_dev *pdev) { }
>  static inline int pcie_enable_tph(struct pci_dev *pdev, int mode)
> -- 
> 2.17.1
>