Re: [PATCH v7 1/3] ACPI: Refactor get_acpi_id_for_cpu() to acpi_get_cpu_uid() on non-x86

[fengchengwen <fengchengwen@xxxxxxxxxx>]

Hi,

On 3/18/2026 5:38 AM, Jeremy Linton wrote:
> Hi,
> 
> Lets try this again, since the last one looks like it got caught in the 
> moderation system and wasn't quite right anyway.
> 
> On 3/12/26 9:21 PM, Chengwen Feng wrote:
>> Unify CPU ACPI ID retrieval interface across architectures by
>> refactoring get_acpi_id_for_cpu() to acpi_get_cpu_uid() on
>> arm64/riscv/loongarch:
>> - Add input parameter validation
>> - Adjust interface to int acpi_get_cpu_uid(unsigned int cpu, u32 *uid)
>>    (old: u32 get_acpi_id_for_cpu(unsigned int cpu), no input check)
>>
>> This refactoring (not a pure rename) enhances interface robustness while
>> preparing for consistent ACPI Processor UID retrieval across all
>> ACPI-enabled platforms. Valid inputs retain original behavior.
>>
>> Note: Move the ARM64-specific get_cpu_for_acpi_id() implementation to
>>        arch/arm64/kernel/acpi_numa.c to fix compilation errors from
>>        circular header dependencies introduced by the rename.
>>
>> Cc: stable@xxxxxxxxxxxxxxx
>> Signed-off-by: Chengwen Feng <fengchengwen@xxxxxxxxxx>
>> Reviewed-by: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>
>> ---
>>   arch/arm64/include/asm/acpi.h      | 16 +---------
>>   arch/arm64/kernel/acpi.c           | 16 ++++++++++
>>   arch/arm64/kernel/acpi_numa.c      | 14 +++++++++
>>   arch/loongarch/include/asm/acpi.h  |  5 ---
>>   arch/loongarch/kernel/acpi.c       |  9 ++++++
>>   arch/riscv/include/asm/acpi.h      |  4 ---
>>   arch/riscv/kernel/acpi.c           | 16 ++++++++++
>>   arch/riscv/kernel/acpi_numa.c      |  9 ++++--
>>   drivers/acpi/pptt.c                | 50 ++++++++++++++++++++++--------
>>   drivers/acpi/riscv/rhct.c          |  7 ++++-
>>   drivers/perf/arm_cspmu/arm_cspmu.c |  6 ++--
>>   include/linux/acpi.h               | 13 ++++++++
>>   12 files changed, 122 insertions(+), 43 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
>> index c07a58b96329..106a08556cbf 100644
>> --- a/arch/arm64/include/asm/acpi.h
>> +++ b/arch/arm64/include/asm/acpi.h
>> @@ -114,22 +114,8 @@ static inline bool acpi_has_cpu_in_madt(void)
>>   }
>>     struct acpi_madt_generic_interrupt *acpi_cpu_get_madt_gicc(int cpu);
>> -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 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..f3866606fc46 100644
>> --- a/arch/arm64/kernel/acpi.c
>> +++ b/arch/arm64/kernel/acpi.c
>> @@ -458,3 +458,19 @@ 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;
> If this actually happens, its probably useful to know it with a 
> pr_warn/pr_warn_once.> +

The function maybe called from userspace which on later roadmap, so I prefer 
not add
warning or error here.
BTW: the function will return -EINVAL, so caller could know the case.

>> +    gicc = acpi_cpu_get_madt_gicc(cpu);
>> +    if (!gicc)
> I think this check is redundant because we can't have logical cpu's that 
> aren't in the cpu_possible() list, which on arm64 doesn't AFAIK have holes. 
> In the past this might have made sense if we weren't maintaining a copy of 
> the gicc structure from the MADT for each core.> +        return -ENODEV;

This commit will backport to stable branch at least 6.6. So I think it's OK to 
keep it.

>> +
>> +    *uid = gicc->uid;
>> +    return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(acpi_get_cpu_uid);
>> diff --git a/arch/arm64/kernel/acpi_numa.c b/arch/arm64/kernel/acpi_numa.c
>> index 2465f291c7e1..41d1e46a4338 100644
>> --- a/arch/arm64/kernel/acpi_numa.c
>> +++ b/arch/arm64/kernel/acpi_numa.c
>> @@ -34,6 +34,20 @@ int __init acpi_numa_get_nid(unsigned int cpu)
>>       return acpi_early_node_map[cpu];
>>   }
>>   +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);
> This might have been a simplification, but since we are basically doing a 
> for_each_possible_cpu(cpu) and every possible cpu will have a GICC entry 
> before it becomes 'possible' there will be a UID, so all the error checking 
> AFAIK, is impossible here.> +        if (ret == 0 && uid == cpu_uid)

I prefer to keep the current impl, as it may catch future error.

>> +            return cpu;
>> +    }
>> +
>> +    return -EINVAL;
>> +}
>> +
> I also moved this below acpi_get_cpu_uid() in acpi.c and I don't see the a 
> forward error issue you mentioned. It seems to me that they should be kept 
> close to each other since they are basically inverses of each other.

As long as you ensure that it is not placed in asm/acpi.h, that's fine.
So it's OK to move this function to acpi.c

But I just checked the callers of this function again and found that there are
all in acpi_numa.c, so I will now add the static keyword to this function and
make it an internal function.

Thanks

>