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

[fengchengwen]

On 3/20/2026 2:32 AM, Bjorn Helgaas wrote:
> Let's make the subject a little more specific, e.g.,
> 
>   PCI/TPH: Pass ACPI Processor UID to Cache Locality _DSM

done in v10

> 
> 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.

It is indeed simplified, done in v10

> 
>> [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>

Thanks Bjorn for your review and kind words! I really appreciate it.

> 
>> ---
>>  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
>>
>