Re: [PATCH v5 4/4] arm/acpi: Parse PPTT to initialize CPU topology

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.07.2026 13:59, Hirokazu Takahashi wrote:
>>> --- a/xen/drivers/acpi/topology.c
>>> +++ b/xen/drivers/acpi/topology.c
>>> @@ -5,18 +5,90 @@
>>>  #include <xen/cpumask.h>
>>>  #include <xen/init.h>
>>>
>>> -/*
>>> - * TODO: Populate the topology information by scanning the ACPI
>>> - *       PPTT (Processor Properties Topology Table).
>>> - */
>>> -void __init acpi_init_cpu_topology(void)
>>> +uint32_t map_cpu_acpiid[NR_CPUS] __initdata =
>>> +         { [0 ... NR_CPUS - 1] = INVALID_ACPIID };
>>> +uint32_t socket_map[NR_CPUS] __initdata;
>>> +uint32_t cluster_map[NR_CPUS] __initdata;
>>> +uint32_t core_map[NR_CPUS] __initdata;
>>> +uint32_t thread_map[NR_CPUS] __initdata;
>>> +unsigned int __initdata num_sockets;
>>> +unsigned int __initdata num_clusters;
>>> +unsigned int __initdata num_cores;
>>
>> static for almost all of these? And please place __initdata uniformly,
>> between type and identifier.
> 
> Okay.
> 
>> For large NR_CPUS this also looks to be adding quite a bit of data. Is all
>> of this really needed?
> 
> The map_cpu_acpiid[NR_CPUS] array is required. This is because 
> acpi_smp_init_cpus()
> parses the MADT and populates map_cpu_acpiid[] with the discovered CPUs. At 
> the
> point when acpi_smp_init_cpus() is called, the total number of CPUs has not 
> yet been
> determined.
> 
> The memory for socket_map[NR_CPUS], cluster_map[NR_CPUS], and 
> core_map[NR_CPUS]
> is automatically freed after Xen initialization completes, as they are marked 
> as __initdata.

As is map_cpu_acpiid[]'s memory, sure.

> Therefore, I am not sure if there is any clear advantage to switching them to 
> dynamic
> allocation and deallocation.

I wasn't necessarily trying to hint at dynamic allocation, but yes, that may
be an option. The main point is that, as previously indicated, the number of
NR_CPUS-dimensioned arrays we have would better go down, not up. While here
it's memory that's freed post-init, it still grows image size: This isn't
.bss-like data; it's .data-like. You may want to take a look at the BRK
allocator (patches to have it in x86 have been pending for quite a while [1]).
That may be possible to leverage here (requiring it to become generic though,
not x86-specific): No space taken in image, but (almost) arbitrary amounts of
space reservable at build time.

Of course, none of the above eliminates the request to try to get away with
less memory overall.

Jan

[1] https://lists.xen.org/archives/html/xen-devel/2025-11/msg00390.html