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

[fengchengwen <fengchengwen@xxxxxxxxxx>]

Sorry to self-reply

On 3/18/2026 10:02 AM, fengchengwen wrote:
> 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.

I just found drivers/irqchip/irq-gic-v3.c has a call for get_cpu_for_acpi_id,
so We should not marking as static.

According to your advise, I moved it in acpi.c in v8.

Thanks

> 
> Thanks
> 
>>
>