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

[fengchengwen]

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

>