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

[Bjorn Helgaas <helgaas@xxxxxxxxxx>]

Let's make the subject a little more specific, e.g.,

  PCI/TPH: Pass ACPI Processor UID to Cache Locality _DSM

On Thu, Mar 19, 2026 at 02:57:35PM +0800, Chengwen Feng wrote:
> pcie_tph_get_cpu_st() is broken on ARM64:
> 1. pcie_tph_get_cpu_st() passes cpu_uid to the PCI ACPI DSM method.
>    cpu_uid should be the ACPI Processor UID [1].
> 2. In BNXT, pcie_tph_get_cpu_st() is passed a cpu_uid obtained via
>    cpumask_first(irq->cpu_mask) - the logical CPU ID of a CPU core,
>    generated and managed by kernel (e.g., [0,255] for a system  with 256
>    logical CPU cores).
> 3. On ARM64 platforms, ACPI assigns Processor UID to cores listed in the
>    MADT table, and this UID may not match the kernel's logical CPU ID.
>    When this occurs, the mismatch results in the wrong CPU steer-tag.
> 4. On AMD x86 the logical CPU ID is identical to the ACPI Processor UID
>    so the mismatch is not seen.
> 
> Resolution:
> 1. Use acpi_get_cpu_uid() in pcie_tph_get_cpu_st() to translate from
>    logical CPU ID to ACPI Processor UID needed for the DSM call.
> 2. Rename pcie_tpu_get_cpu_st() parameter from cpu_uid to cpu to
>    reflect that it is a logical CPU_ID.

And simplify this, e.g.,

  pcie_tph_get_cpu_st() uses the Query Cache Locality Features _DSM
  [1] to retrieve the TPH Steering Tag for memory associated with the
  CPU identified by its "cpu_uid" parameter, a Linux logical CPU ID.

  The _DSM requires a ACPI Processor UID, which pcie_tph_get_cpu_st()
  previously assumed was the same as the Linux logical CPU ID.  This
  is true on x86 but not on arm64, so pcie_tph_get_cpu_st() failed on
  arm64.

  Convert the Linux logical CPU ID to the ACPI Processor UID with
  acpi_get_cpu_uid() before passing it to the _DSM.

If there's a specific error message from BNXT or similar that is a
symptom of this problem, it'd be nice to include that.  Or if the
problem was just poor performance because the _DSM returned the wrong
Steering Tag, you could say something like:

  ... so pcie_tph_get_cpu_st() returned the wrong Steering Tag,
  resulting in poor network performance.

> [1] According to ECN_TPH-ST_Revision_20200924
>     (https://members.pcisig.com/wg/PCI-SIG/document/15470), 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."
> 
> Fixes: d2e8a34876ce ("PCI/TPH: Add Steering Tag support")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Chengwen Feng <fengchengwen@xxxxxxxxxx>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>

The patch looks good to me.  Thanks for all your work on this!

Reviewed-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>

> ---
>  Documentation/PCI/tph.rst |  4 ++--
>  drivers/pci/tph.c         | 16 +++++++++++-----
>  include/linux/pci-tph.h   |  4 ++--
>  3 files changed, 15 insertions(+), 9 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/pci/tph.c b/drivers/pci/tph.c
> index ca4f97be7538..b67c9ad14bda 100644
> --- a/drivers/pci/tph.c
> +++ b/drivers/pci/tph.c
> @@ -236,21 +236,27 @@ 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
>       struct pci_dev *rp;
>       acpi_handle rp_acpi_handle;
>       union st_info info;
> +     u32 cpu_uid;
> +     int ret;
> +
> +     ret = acpi_get_cpu_uid(cpu, &cpu_uid);
> +     if (ret != 0)
> +             return ret;
>  
>       rp = pcie_find_root_port(pdev);
>       if (!rp || !rp->bus || !rp->bus->bridge)
> @@ -265,9 +271,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/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
>