Re: [PATCH 1/2] x86: during boot, anticipate identifying the boot cpu

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 22/08/14 18:15, Dario Faggioli wrote:
> and provide helpers to access the core and socket IDs,
> resulting from that identification phase.
>
> Also, initialize the socket ID of all the cpus to something
> invalid (-1), rather than leaving it as 0, which is at risk
> of being confused with "this CPU is on socket 0".
>
> All this is right now relevant to credit2 only, but may be
> useful in other schedulers, or elsewhere in general.
>
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> ---
>  xen/arch/x86/setup.c            |    8 ++++++--
>  xen/arch/x86/smpboot.c          |    3 ++-
>  xen/include/asm-x86/processor.h |    6 ++++--
>  3 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 6a814cd..f39fdf0 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1262,6 +1262,12 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>  
>      timer_init();
>  
> +    /*
> +     * Identify the boot CPU, in case the scheduler initialization
> +     * needs to know about it (e.g., topology, etc.)
> +     */
> +    identify_cpu(&boot_cpu_data);
> +
>      init_idle_domain();
>  
>      trap_init();
> @@ -1272,8 +1278,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>  
>      arch_init_memory();
>  
> -    identify_cpu(&boot_cpu_data);
> -
>      if ( cpu_has_fxsr )
>          set_in_cr4(X86_CR4_OSFXSR);
>      if ( cpu_has_xmm )
> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
> index 84f2d25..16a7474 100644
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -59,7 +59,8 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_mask);
>  cpumask_t cpu_online_map __read_mostly;
>  EXPORT_SYMBOL(cpu_online_map);
>  
> -struct cpuinfo_x86 cpu_data[NR_CPUS];
> +struct cpuinfo_x86 cpu_data[NR_CPUS] =
> +	{ [0 ... NR_CPUS-1] = { .phys_proc_id=-1 } };

This moves a huge chunk of data from .bss to .data.  Can it not be fixed
by enumerating topology on APs before setting scheduling up?

>  
>  u32 x86_cpu_to_apicid[NR_CPUS] __read_mostly =
>  	{ [0 ... NR_CPUS-1] = BAD_APICID };
> diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
> index a156e01..76d47de 100644
> --- a/xen/include/asm-x86/processor.h
> +++ b/xen/include/asm-x86/processor.h
> @@ -213,8 +213,10 @@ extern void detect_extended_topology(struct cpuinfo_x86 *c);
>  
>  extern void detect_ht(struct cpuinfo_x86 *c);
>  
> -#define cpu_to_core(_cpu)   (cpu_data[_cpu].cpu_core_id)
> -#define cpu_to_socket(_cpu) (cpu_data[_cpu].phys_proc_id)
> +#define cpu_to_core(_cpu)    (cpu_data[_cpu].cpu_core_id)
> +#define cpu_to_socket(_cpu)  (cpu_data[_cpu].phys_proc_id)
> +#define boot_cpu_to_core()   (boot_cpu_data.phys_core_id)
> +#define boot_cpu_to_socket() (boot_cpu_data.phys_proc_id)

This change is pointless.  Anything explicitly referencing the bsp
should just use boot_cpu_data directly, rather than obscuring the IDs
behind macros like this.

~Andrew

>  
>  unsigned int apicid_to_socket(unsigned int);
>  
>