[PATCH] Arm: cpu_*_map adjustments

Jan Beulich posted 1 patch 8 months, 1 week ago
Failed in applying to current master (apply log)
[PATCH] Arm: cpu_*_map adjustments
Posted by Jan Beulich 8 months, 1 week ago
First, they all start out zeroed. There's no point doing an initial
cpumask_clear() on them.

Next, only cpu_online_map may be altered post-boot, and even that only
rarely. Add respective placement attributes.

Finally, cpu_present_map really isn't anything more than an alias of
cpu_possible_map. Avoid the copying, and have the linker provide the
symbol (if needed in the first place; it is needed right now as
common code references the symbol).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I would have preferred to use __attribute__((alias(...))), but that
apparently (and following the doc) can't be used when a declaration of
the to-be-provided symbol is present earlier. 

With the cpumask_clear()s dropped, the containing function's name is
now yet more out of sync with what the function does. On x86 we call a
similar thing smp_prepare_boot_cpu().

With x86 getting away without cpu_possible_map I wonder if we shouldn't
globally fold that and cpu_present_map, under whichever is deemed the
more suitable name of the two. There's little point in having new ports
adding perhaps yet more inconsistencies there.

--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -35,9 +35,8 @@
 #undef virt_to_mfn
 #define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
 
-cpumask_t cpu_online_map;
-cpumask_t cpu_present_map;
-cpumask_t cpu_possible_map;
+cpumask_t __read_mostly cpu_online_map;
+cpumask_t __ro_after_init cpu_possible_map;
 
 struct cpuinfo_arm cpu_data[NR_CPUS];
 
@@ -111,8 +110,6 @@ static void remove_cpu_sibling_map(int c
 void __init
 smp_clear_cpu_maps (void)
 {
-    cpumask_clear(&cpu_possible_map);
-    cpumask_clear(&cpu_online_map);
     cpumask_set_cpu(0, &cpu_online_map);
     cpumask_set_cpu(0, &cpu_possible_map);
     cpu_logical_map(0) = READ_SYSREG(MPIDR_EL1) & MPIDR_HWID_MASK;
@@ -312,8 +309,6 @@ smp_prepare_cpus(void)
 {
     int rc;
 
-    cpumask_copy(&cpu_present_map, &cpu_possible_map);
-
     rc = setup_cpu_sibling_map(0);
     if ( rc )
         panic("Unable to allocate CPU sibling/core maps\n");
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -206,6 +206,8 @@ SECTIONS
   ELF_DETAILS_SECTIONS
 }
 
+PROVIDE(cpu_present_map = cpu_possible_map);
+
 /*
  * The assembly code use _start and XEN_VIRT_START interchangeably to
  * match the context.
Re: [PATCH] Arm: cpu_*_map adjustments
Posted by Orzel, Michal 8 months ago

On 10/04/2025 16:54, Jan Beulich wrote:
> First, they all start out zeroed. There's no point doing an initial
> cpumask_clear() on them.
> 
> Next, only cpu_online_map may be altered post-boot, and even that only
> rarely. Add respective placement attributes.
> 
> Finally, cpu_present_map really isn't anything more than an alias of
> cpu_possible_map. Avoid the copying, and have the linker provide the
> symbol (if needed in the first place; it is needed right now as
> common code references the symbol).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

> ---
> I would have preferred to use __attribute__((alias(...))), but that
> apparently (and following the doc) can't be used when a declaration of
> the to-be-provided symbol is present earlier. 
> 
> With the cpumask_clear()s dropped, the containing function's name is
> now yet more out of sync with what the function does. On x86 we call a
> similar thing smp_prepare_boot_cpu().
I agree the function name no longer makes sense. I'd be ok with renaming it to
smp_prepare_boot_cpu().

~Michal