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

[Jeremy Linton <jeremy.linton@xxxxxxx>]

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.> +
> +       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;
> +
> +       *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)
> +                       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.