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

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Mar 04, 2026 at 05:28:36PM +0800, fengchengwen wrote:
> On 3/4/2026 3:02 AM, Bjorn Helgaas wrote:
> > [+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.
> ...

> >>  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?
> 
> Your idea to unify the ACPI CPU ID function across architectures is
> great. I noticed that all exported ACPI functions use the `acpi_`
> prefix, so I think we shouldn’t simply modify x86’s implementation
> directly – we should also align the naming convention for other
> platforms.
> 
> Since only x86, arm64, risc-v, and loongarch currently support ACPI,
> how about we remove the new config and instead use this approach:
> 
> 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 defined(CONFIG_ARM64) || defined(CONFIG_RISCV) || defined(CONFIG_LOONGARCH)
>         return get_acpi_id_for_cpu(cpu);
> #endif
> }
> 
> For any new platform that plans to support ACPI in the future, this
> will trigger a compile error – which is intentional. It will prompt
> the maintainers of that platform to add their own implementation of
> either `get_acpi_id_for_cpu` or `cpu_acpi_id` as needed.

I agree that an "acpi_" prefix would be appropriate.  I'd suggest
separate implementations in arch/*/kernel/acpi.c instead of #ifdefs in
a common version.

Maybe users of cpu_acpi_id() and get_acpi_id_for_cpu() could be
converted to use the generic interface.