[PATCH v9 1/7] arm64/acpi: Add acpi_get_cpu_uid() and switch arm_cspmu to use it

Chengwen Feng posted 7 patches 4 days, 12 hours ago
There is a newer version of this series
[PATCH v9 1/7] arm64/acpi: Add acpi_get_cpu_uid() and switch arm_cspmu to use it
Posted by Chengwen Feng 4 days, 12 hours ago
Add arch-specific acpi_get_cpu_uid() for arm64, and update dependent
code:
- Declare acpi_get_cpu_uid() in arch/arm64/include/asm/acpi.h
- Implement acpi_get_cpu_uid() with input parameter validation
- Replace get_acpi_id_for_cpu() with acpi_get_cpu_uid() in
  drivers/perf/arm_cspmu/arm_cspmu.c
- Reimplement get_cpu_for_acpi_id() based on acpi_get_cpu_uid() (to
  align with new interface) and move its implementation next to
  acpi_get_cpu_uid()

This is the first step towards unifying ACPI CPU UID retrieval interface
across architectures, while adding input validation for robustness.

Cc: stable@vger.kernel.org
Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
---
 arch/arm64/include/asm/acpi.h      | 14 ++------------
 arch/arm64/kernel/acpi.c           | 30 ++++++++++++++++++++++++++++++
 drivers/perf/arm_cspmu/arm_cspmu.c |  6 ++++--
 3 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index c07a58b96329..2219a3301e72 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -118,18 +118,8 @@ static inline u32 get_acpi_id_for_cpu(unsigned int cpu)
 {
 	return	acpi_cpu_get_madt_gicc(cpu)->uid;
 }
-
-static inline int get_cpu_for_acpi_id(u32 uid)
-{
-	int cpu;
-
-	for (cpu = 0; cpu < nr_cpu_ids; cpu++)
-		if (acpi_cpu_get_madt_gicc(cpu) &&
-		    uid == get_acpi_id_for_cpu(cpu))
-			return cpu;
-
-	return -EINVAL;
-}
+int acpi_get_cpu_uid(unsigned int cpu, u32 *uid);
+int get_cpu_for_acpi_id(u32 uid);
 
 static inline void arch_fix_phys_package_id(int num, u32 slot) { }
 void __init acpi_init_cpus(void);
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index af90128cfed5..24b9d934be54 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -458,3 +458,33 @@ int acpi_unmap_cpu(int cpu)
 }
 EXPORT_SYMBOL(acpi_unmap_cpu);
 #endif /* CONFIG_ACPI_HOTPLUG_CPU */
+
+int acpi_get_cpu_uid(unsigned int cpu, u32 *uid)
+{
+	struct acpi_madt_generic_interrupt *gicc;
+
+	if (cpu >= nr_cpu_ids)
+		return -EINVAL;
+
+	gicc = acpi_cpu_get_madt_gicc(cpu);
+	if (!gicc)
+		return -ENODEV;
+
+	*uid = gicc->uid;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(acpi_get_cpu_uid);
+
+int get_cpu_for_acpi_id(u32 uid)
+{
+	u32 cpu_uid;
+	int ret;
+
+	for (int cpu = 0; cpu < nr_cpu_ids; cpu++) {
+		ret = acpi_get_cpu_uid(cpu, &cpu_uid);
+		if (ret == 0 && uid == cpu_uid)
+			return cpu;
+	}
+
+	return -EINVAL;
+}
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 34430b68f602..ed72c3d1f796 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -1107,15 +1107,17 @@ static int arm_cspmu_acpi_get_cpus(struct arm_cspmu *cspmu)
 {
 	struct acpi_apmt_node *apmt_node;
 	int affinity_flag;
+	u32 cpu_uid;
 	int cpu;
+	int ret;
 
 	apmt_node = arm_cspmu_apmt_node(cspmu->dev);
 	affinity_flag = apmt_node->flags & ACPI_APMT_FLAGS_AFFINITY;
 
 	if (affinity_flag == ACPI_APMT_FLAGS_AFFINITY_PROC) {
 		for_each_possible_cpu(cpu) {
-			if (apmt_node->proc_affinity ==
-			    get_acpi_id_for_cpu(cpu)) {
+			ret = acpi_get_cpu_uid(cpu, &cpu_uid);
+			if (ret == 0 && apmt_node->proc_affinity == cpu_uid) {
 				cpumask_set_cpu(cpu, &cspmu->associated_cpus);
 				break;
 			}
-- 
2.17.1
Re: [PATCH v9 1/7] arm64/acpi: Add acpi_get_cpu_uid() and switch arm_cspmu to use it
Posted by Punit Agrawal 4 days, 4 hours ago
Chengwen Feng <fengchengwen@huawei.com> writes:

> Add arch-specific acpi_get_cpu_uid() for arm64, and update dependent
> code:
> - Declare acpi_get_cpu_uid() in arch/arm64/include/asm/acpi.h
> - Implement acpi_get_cpu_uid() with input parameter validation
> - Replace get_acpi_id_for_cpu() with acpi_get_cpu_uid() in
>   drivers/perf/arm_cspmu/arm_cspmu.c
> - Reimplement get_cpu_for_acpi_id() based on acpi_get_cpu_uid() (to
>   align with new interface) and move its implementation next to
>   acpi_get_cpu_uid()

There is no benefit in describing the code changes like this in the
commit log. It makes it hard to follow the intent of the patch.

> This is the first step towards unifying ACPI CPU UID retrieval interface
> across architectures, while adding input validation for robustness.

I would simplify the commit log to something along the lines of -

    As a step towards unifying the interface for retrieving ACPI CPU uid
    across architectures, introduce a new function
    acpi_get_cpu_uid(). While at it, also add input validation to make
    the code more robust.

Just my 2c.

The code changes looks fine.

> Cc: stable@vger.kernel.org
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> ---
>  arch/arm64/include/asm/acpi.h      | 14 ++------------
>  arch/arm64/kernel/acpi.c           | 30 ++++++++++++++++++++++++++++++
>  drivers/perf/arm_cspmu/arm_cspmu.c |  6 ++++--
>  3 files changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index c07a58b96329..2219a3301e72 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -118,18 +118,8 @@ static inline u32 get_acpi_id_for_cpu(unsigned int cpu)
>  {
>  	return	acpi_cpu_get_madt_gicc(cpu)->uid;
>  }
> -
> -static inline int get_cpu_for_acpi_id(u32 uid)
> -{
> -	int cpu;
> -
> -	for (cpu = 0; cpu < nr_cpu_ids; cpu++)
> -		if (acpi_cpu_get_madt_gicc(cpu) &&
> -		    uid == get_acpi_id_for_cpu(cpu))
> -			return cpu;
> -
> -	return -EINVAL;
> -}
> +int acpi_get_cpu_uid(unsigned int cpu, u32 *uid);
> +int get_cpu_for_acpi_id(u32 uid);
>  
>  static inline void arch_fix_phys_package_id(int num, u32 slot) { }
>  void __init acpi_init_cpus(void);
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index af90128cfed5..24b9d934be54 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -458,3 +458,33 @@ int acpi_unmap_cpu(int cpu)
>  }
>  EXPORT_SYMBOL(acpi_unmap_cpu);
>  #endif /* CONFIG_ACPI_HOTPLUG_CPU */
> +
> +int acpi_get_cpu_uid(unsigned int cpu, u32 *uid)
> +{
> +	struct acpi_madt_generic_interrupt *gicc;
> +
> +	if (cpu >= nr_cpu_ids)
> +		return -EINVAL;
> +
> +	gicc = acpi_cpu_get_madt_gicc(cpu);
> +	if (!gicc)
> +		return -ENODEV;
> +
> +	*uid = gicc->uid;
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(acpi_get_cpu_uid);
> +
> +int get_cpu_for_acpi_id(u32 uid)
> +{
> +	u32 cpu_uid;
> +	int ret;
> +
> +	for (int cpu = 0; cpu < nr_cpu_ids; cpu++) {
> +		ret = acpi_get_cpu_uid(cpu, &cpu_uid);
> +		if (ret == 0 && uid == cpu_uid)
> +			return cpu;
> +	}
> +
> +	return -EINVAL;
> +}
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index 34430b68f602..ed72c3d1f796 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -1107,15 +1107,17 @@ static int arm_cspmu_acpi_get_cpus(struct arm_cspmu *cspmu)
>  {
>  	struct acpi_apmt_node *apmt_node;
>  	int affinity_flag;
> +	u32 cpu_uid;
>  	int cpu;
> +	int ret;
>  
>  	apmt_node = arm_cspmu_apmt_node(cspmu->dev);
>  	affinity_flag = apmt_node->flags & ACPI_APMT_FLAGS_AFFINITY;
>  
>  	if (affinity_flag == ACPI_APMT_FLAGS_AFFINITY_PROC) {
>  		for_each_possible_cpu(cpu) {
> -			if (apmt_node->proc_affinity ==
> -			    get_acpi_id_for_cpu(cpu)) {
> +			ret = acpi_get_cpu_uid(cpu, &cpu_uid);
> +			if (ret == 0 && apmt_node->proc_affinity == cpu_uid) {
>  				cpumask_set_cpu(cpu, &cspmu->associated_cpus);
>  				break;
>  			}

I think cspmu changes go via a separate pull request. You might have to
split this change into a separate commit.
Re: [PATCH v9 1/7] arm64/acpi: Add acpi_get_cpu_uid() and switch arm_cspmu to use it
Posted by fengchengwen 3 days, 16 hours ago
On 3/19/2026 11:46 PM, Punit Agrawal wrote:
> Chengwen Feng <fengchengwen@huawei.com> writes:
> 
>> Add arch-specific acpi_get_cpu_uid() for arm64, and update dependent
>> code:
>> - Declare acpi_get_cpu_uid() in arch/arm64/include/asm/acpi.h
>> - Implement acpi_get_cpu_uid() with input parameter validation
>> - Replace get_acpi_id_for_cpu() with acpi_get_cpu_uid() in
>>   drivers/perf/arm_cspmu/arm_cspmu.c
>> - Reimplement get_cpu_for_acpi_id() based on acpi_get_cpu_uid() (to
>>   align with new interface) and move its implementation next to
>>   acpi_get_cpu_uid()
> 
> There is no benefit in describing the code changes like this in the
> commit log. It makes it hard to follow the intent of the patch.
> 
>> This is the first step towards unifying ACPI CPU UID retrieval interface
>> across architectures, while adding input validation for robustness.
> 
> I would simplify the commit log to something along the lines of -
> 
>     As a step towards unifying the interface for retrieving ACPI CPU uid
>     across architectures, introduce a new function
>     acpi_get_cpu_uid(). While at it, also add input validation to make
>     the code more robust.

Thank you for your advice.
I've reviewed all the commit logs and made some optimizations, and done in v10

> 
> Just my 2c.
> 
> The code changes looks fine.
> 
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>> ---
>>  arch/arm64/include/asm/acpi.h      | 14 ++------------
>>  arch/arm64/kernel/acpi.c           | 30 ++++++++++++++++++++++++++++++
>>  drivers/perf/arm_cspmu/arm_cspmu.c |  6 ++++--
>>  3 files changed, 36 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>> index c07a58b96329..2219a3301e72 100644
>> --- a/arch/arm64/include/asm/acpi.h
>> +++ b/arch/arm64/include/asm/acpi.h
>> @@ -118,18 +118,8 @@ static inline u32 get_acpi_id_for_cpu(unsigned int cpu)
>>  {
>>  	return	acpi_cpu_get_madt_gicc(cpu)->uid;
>>  }
>> -
>> -static inline int get_cpu_for_acpi_id(u32 uid)
>> -{
>> -	int cpu;
>> -
>> -	for (cpu = 0; cpu < nr_cpu_ids; cpu++)
>> -		if (acpi_cpu_get_madt_gicc(cpu) &&
>> -		    uid == get_acpi_id_for_cpu(cpu))
>> -			return cpu;
>> -
>> -	return -EINVAL;
>> -}
>> +int acpi_get_cpu_uid(unsigned int cpu, u32 *uid);
>> +int get_cpu_for_acpi_id(u32 uid);
>>  
>>  static inline void arch_fix_phys_package_id(int num, u32 slot) { }
>>  void __init acpi_init_cpus(void);
>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>> index af90128cfed5..24b9d934be54 100644
>> --- a/arch/arm64/kernel/acpi.c
>> +++ b/arch/arm64/kernel/acpi.c
>> @@ -458,3 +458,33 @@ int acpi_unmap_cpu(int cpu)
>>  }
>>  EXPORT_SYMBOL(acpi_unmap_cpu);
>>  #endif /* CONFIG_ACPI_HOTPLUG_CPU */
>> +
>> +int acpi_get_cpu_uid(unsigned int cpu, u32 *uid)
>> +{
>> +	struct acpi_madt_generic_interrupt *gicc;
>> +
>> +	if (cpu >= nr_cpu_ids)
>> +		return -EINVAL;
>> +
>> +	gicc = acpi_cpu_get_madt_gicc(cpu);
>> +	if (!gicc)
>> +		return -ENODEV;
>> +
>> +	*uid = gicc->uid;
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(acpi_get_cpu_uid);
>> +
>> +int get_cpu_for_acpi_id(u32 uid)
>> +{
>> +	u32 cpu_uid;
>> +	int ret;
>> +
>> +	for (int cpu = 0; cpu < nr_cpu_ids; cpu++) {
>> +		ret = acpi_get_cpu_uid(cpu, &cpu_uid);
>> +		if (ret == 0 && uid == cpu_uid)
>> +			return cpu;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
>> index 34430b68f602..ed72c3d1f796 100644
>> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
>> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
>> @@ -1107,15 +1107,17 @@ static int arm_cspmu_acpi_get_cpus(struct arm_cspmu *cspmu)
>>  {
>>  	struct acpi_apmt_node *apmt_node;
>>  	int affinity_flag;
>> +	u32 cpu_uid;
>>  	int cpu;
>> +	int ret;
>>  
>>  	apmt_node = arm_cspmu_apmt_node(cspmu->dev);
>>  	affinity_flag = apmt_node->flags & ACPI_APMT_FLAGS_AFFINITY;
>>  
>>  	if (affinity_flag == ACPI_APMT_FLAGS_AFFINITY_PROC) {
>>  		for_each_possible_cpu(cpu) {
>> -			if (apmt_node->proc_affinity ==
>> -			    get_acpi_id_for_cpu(cpu)) {
>> +			ret = acpi_get_cpu_uid(cpu, &cpu_uid);
>> +			if (ret == 0 && apmt_node->proc_affinity == cpu_uid) {
>>  				cpumask_set_cpu(cpu, &cspmu->associated_cpus);
>>  				break;
>>  			}
> 
> I think cspmu changes go via a separate pull request. You might have to
> split this change into a separate commit.

done in v10

Thanks