| Message ID | 8121d9b472b305be751158aa3af3fed98ff0572e.1699982111.git.krystian.hebel@3mdeb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: parallelize AP bring-up during boot | expand |
Hi, On 14/11/2023 17:50, Krystian Hebel wrote: > Both fields held the same data. > > Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com> > --- > xen/arch/x86/boot/x86_64.S | 8 +++++--- > xen/arch/x86/include/asm/asm_defns.h | 2 +- > xen/arch/x86/include/asm/processor.h | 2 ++ > xen/arch/x86/include/asm/smp.h | 4 ---- > xen/arch/x86/numa.c | 15 +++++++-------- > xen/arch/x86/smpboot.c | 8 ++++---- > xen/arch/x86/x86_64/asm-offsets.c | 4 +++- > 7 files changed, 22 insertions(+), 21 deletions(-) > > diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S > index b85b47b5c1a0..195550b5c0ea 100644 > --- a/xen/arch/x86/boot/x86_64.S > +++ b/xen/arch/x86/boot/x86_64.S > @@ -20,15 +20,17 @@ ENTRY(__high_start) > jz .L_stack_set > > /* APs only: get stack base from APIC ID saved in %esp. */ > - mov $-1, %rax > - lea x86_cpu_to_apicid(%rip), %rcx > + mov $0, %rax > + lea cpu_data(%rip), %rcx > + /* cpu_data[0] is BSP, skip it. */ > 1: > add $1, %rax > + add $CPUINFO_X86_sizeof, %rcx > cmp $NR_CPUS, %eax > jb 2f > hlt > 2: > - cmp %esp, (%rcx, %rax, 4) > + cmp %esp, CPUINFO_X86_apicid(%rcx) > jne 1b > > /* %eax is now Xen CPU index. */ As mentioned in an earlier patch, I think you want to re-order the patches. This will avoid to modify twice the same code within the same series (it is best to avoid if you can). > diff --git a/xen/arch/x86/include/asm/asm_defns.h b/xen/arch/x86/include/asm/asm_defns.h > index baaaccb26e17..6b05d9d140b8 100644 > --- a/xen/arch/x86/include/asm/asm_defns.h > +++ b/xen/arch/x86/include/asm/asm_defns.h > @@ -158,7 +158,7 @@ register unsigned long current_stack_pointer asm("rsp"); > #endif > > #define CPUINFO_FEATURE_OFFSET(feature) \ > - (CPUINFO_features + (cpufeat_word(feature) * 4)) > + (CPUINFO_X86_features + (cpufeat_word(feature) * 4)) > > #else > > diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h > index b0d2a62c075f..8345d58094da 100644 > --- a/xen/arch/x86/include/asm/processor.h > +++ b/xen/arch/x86/include/asm/processor.h > @@ -92,6 +92,8 @@ struct x86_cpu_id { > extern struct cpuinfo_x86 cpu_data[]; > #define current_cpu_data cpu_data[smp_processor_id()] > > +#define cpu_physical_id(cpu) cpu_data[cpu].apicid > + > extern bool probe_cpuid_faulting(void); > extern void ctxt_switch_levelling(const struct vcpu *next); > extern void (*ctxt_switch_masking)(const struct vcpu *next); > diff --git a/xen/arch/x86/include/asm/smp.h b/xen/arch/x86/include/asm/smp.h > index c0b5d7cdd8dd..94c557491860 100644 > --- a/xen/arch/x86/include/asm/smp.h > +++ b/xen/arch/x86/include/asm/smp.h > @@ -39,10 +39,6 @@ extern void (*mtrr_hook) (void); > > extern void zap_low_mappings(void); > > -extern u32 x86_cpu_to_apicid[]; > - > -#define cpu_physical_id(cpu) x86_cpu_to_apicid[cpu] > - > #define cpu_is_offline(cpu) unlikely(!cpu_online(cpu)) > extern void cpu_exit_clear(unsigned int cpu); > extern void cpu_uninit(unsigned int cpu); > diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c > index 39e131cb4f35..91527be5b406 100644 > --- a/xen/arch/x86/numa.c > +++ b/xen/arch/x86/numa.c > @@ -54,14 +54,13 @@ bool __init arch_numa_unavailable(void) > /* > * Setup early cpu_to_node. > * > - * Populate cpu_to_node[] only if x86_cpu_to_apicid[], > - * and apicid_to_node[] tables have valid entries for a CPU. > - * This means we skip cpu_to_node[] initialisation for NUMA > - * emulation and faking node case (when running a kernel compiled > - * for NUMA on a non NUMA box), which is OK as cpu_to_node[] > - * is already initialized in a round robin manner at numa_init_array, > - * prior to this call, and this initialization is good enough > - * for the fake NUMA cases. > + * Populate cpu_to_node[] only if cpu_data[], and apicid_to_node[] > + * tables have valid entries for a CPU. This means we skip > + * cpu_to_node[] initialisation for NUMA emulation and faking node > + * case (when running a kernel compiled for NUMA on a non NUMA box), > + * which is OK as cpu_to_node[] is already initialized in a round > + * robin manner at numa_init_array, prior to this call, and this > + * initialization is good enough for the fake NUMA cases. > */ > void __init init_cpu_to_node(void) > { > diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c > index de87c5a41926..f061486e56eb 100644 > --- a/xen/arch/x86/smpboot.c > +++ b/xen/arch/x86/smpboot.c > @@ -61,10 +61,8 @@ unsigned int __read_mostly nr_sockets; > cpumask_t **__read_mostly socket_cpumask; > static cpumask_t *secondary_socket_cpumask; > > -struct cpuinfo_x86 cpu_data[NR_CPUS]; > - > -u32 x86_cpu_to_apicid[NR_CPUS] __read_mostly = > - { [0 ... NR_CPUS-1] = BAD_APICID }; > +struct cpuinfo_x86 cpu_data[NR_CPUS] = > + { [0 ... NR_CPUS-1] .apicid = BAD_APICID }; > > static int cpu_error; > static enum cpu_state { > @@ -81,7 +79,9 @@ void *stack_base[NR_CPUS]; > > void initialize_cpu_data(unsigned int cpu) > { > + uint32_t apicid = cpu_physical_id(cpu); It would be probably worth it to add a comment explaining why you save apicid. What about? /* The APIC ID is saved in cpu_data[cpu] which will be overriden below. */ Coding style: Newline after the declaration. > cpu_data[cpu] = boot_cpu_data; > + cpu_physical_id(cpu) = apicid; > } > > static bool smp_store_cpu_info(unsigned int id) > diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c > index 57b73a4e6214..e881cd5de0a0 100644 > --- a/xen/arch/x86/x86_64/asm-offsets.c > +++ b/xen/arch/x86/x86_64/asm-offsets.c > @@ -159,7 +159,9 @@ void __dummy__(void) > OFFSET(IRQSTAT_softirq_pending, irq_cpustat_t, __softirq_pending); > BLANK(); > > - OFFSET(CPUINFO_features, struct cpuinfo_x86, x86_capability); > + OFFSET(CPUINFO_X86_features, struct cpuinfo_x86, x86_capability); The rename seems to be unrelated to this patch. Can you clarify? > + OFFSET(CPUINFO_X86_apicid, struct cpuinfo_x86, apicid); > + DEFINE(CPUINFO_X86_sizeof, sizeof(struct cpuinfo_x86)); > BLANK(); > > OFFSET(MB_flags, multiboot_info_t, flags); Cheers,
On 02.02.2024 19:11, Julien Grall wrote: > Hi, > > On 14/11/2023 17:50, Krystian Hebel wrote: >> Both fields held the same data. >> >> Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com> >> --- >> xen/arch/x86/boot/x86_64.S | 8 +++++--- >> xen/arch/x86/include/asm/asm_defns.h | 2 +- >> xen/arch/x86/include/asm/processor.h | 2 ++ >> xen/arch/x86/include/asm/smp.h | 4 ---- >> xen/arch/x86/numa.c | 15 +++++++-------- >> xen/arch/x86/smpboot.c | 8 ++++---- >> xen/arch/x86/x86_64/asm-offsets.c | 4 +++- >> 7 files changed, 22 insertions(+), 21 deletions(-) >> >> diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S >> index b85b47b5c1a0..195550b5c0ea 100644 >> --- a/xen/arch/x86/boot/x86_64.S >> +++ b/xen/arch/x86/boot/x86_64.S >> @@ -20,15 +20,17 @@ ENTRY(__high_start) >> jz .L_stack_set >> >> /* APs only: get stack base from APIC ID saved in %esp. */ >> - mov $-1, %rax >> - lea x86_cpu_to_apicid(%rip), %rcx >> + mov $0, %rax >> + lea cpu_data(%rip), %rcx >> + /* cpu_data[0] is BSP, skip it. */ >> 1: >> add $1, %rax >> + add $CPUINFO_X86_sizeof, %rcx >> cmp $NR_CPUS, %eax >> jb 2f >> hlt >> 2: >> - cmp %esp, (%rcx, %rax, 4) >> + cmp %esp, CPUINFO_X86_apicid(%rcx) >> jne 1b >> >> /* %eax is now Xen CPU index. */ > > As mentioned in an earlier patch, I think you want to re-order the > patches. This will avoid to modify twice the same code within the same > series (it is best to avoid if you can). I second this request. Even more so that there's an unexplained move from starting at $-1 to starting at $0 (in which case you really want to use xor, not mov). >> --- a/xen/arch/x86/numa.c >> +++ b/xen/arch/x86/numa.c >> @@ -54,14 +54,13 @@ bool __init arch_numa_unavailable(void) >> /* >> * Setup early cpu_to_node. >> * >> - * Populate cpu_to_node[] only if x86_cpu_to_apicid[], >> - * and apicid_to_node[] tables have valid entries for a CPU. >> - * This means we skip cpu_to_node[] initialisation for NUMA >> - * emulation and faking node case (when running a kernel compiled >> - * for NUMA on a non NUMA box), which is OK as cpu_to_node[] >> - * is already initialized in a round robin manner at numa_init_array, >> - * prior to this call, and this initialization is good enough >> - * for the fake NUMA cases. >> + * Populate cpu_to_node[] only if cpu_data[], and apicid_to_node[] You mean cpu_physical_id() here, and then this change wants doing when switching to that, imo. >> + * tables have valid entries for a CPU. This means we skip >> + * cpu_to_node[] initialisation for NUMA emulation and faking node >> + * case (when running a kernel compiled for NUMA on a non NUMA box), >> + * which is OK as cpu_to_node[] is already initialized in a round >> + * robin manner at numa_init_array, prior to this call, and this >> + * initialization is good enough for the fake NUMA cases. >> */ Also if you're already re-wrapping this comment, please make better use of line width. >> --- a/xen/arch/x86/x86_64/asm-offsets.c >> +++ b/xen/arch/x86/x86_64/asm-offsets.c >> @@ -159,7 +159,9 @@ void __dummy__(void) >> OFFSET(IRQSTAT_softirq_pending, irq_cpustat_t, __softirq_pending); >> BLANK(); >> >> - OFFSET(CPUINFO_features, struct cpuinfo_x86, x86_capability); >> + OFFSET(CPUINFO_X86_features, struct cpuinfo_x86, x86_capability); > > The rename seems to be unrelated to this patch. Can you clarify? I agree some renaming wants doing, but separately. That's because we use CPUINFO_ as a prefix for two entirely different structure's offsets right now. I'm not convinced of CPUINFO_X86_ as the new prefix though: Uses are against cpu_data[], so CPUDATA_ may be better. Might be good if Andrew and/or Roger could voice their view. Jan >> + OFFSET(CPUINFO_X86_apicid, struct cpuinfo_x86, apicid); >> + DEFINE(CPUINFO_X86_sizeof, sizeof(struct cpuinfo_x86)); >> BLANK(); >> >> OFFSET(MB_flags, multiboot_info_t, flags); > > Cheers, >
On 14.11.2023 18:50, Krystian Hebel wrote: > Both fields held the same data. Supposedly the same data only. They come from different origins, and you're hiding this quite well by leaving all sites in place where the field is written. Both items are also used for entirely separate purposes. So you need to - explain why both sources of information necessarily provide the same data, - especially if there's remaining concern from the above explanation that the two values might end up different in corner cases (running virtualized ourselves comes to mind as a possible example), explain why nevertheless it is fine (risk free) to use the consolidated item for all of the originally separate purposes, - either explain or do away with the multiple places setting this single remaining field. > --- a/xen/arch/x86/smpboot.c > +++ b/xen/arch/x86/smpboot.c > @@ -61,10 +61,8 @@ unsigned int __read_mostly nr_sockets; > cpumask_t **__read_mostly socket_cpumask; > static cpumask_t *secondary_socket_cpumask; > > -struct cpuinfo_x86 cpu_data[NR_CPUS]; > - > -u32 x86_cpu_to_apicid[NR_CPUS] __read_mostly = > - { [0 ... NR_CPUS-1] = BAD_APICID }; > +struct cpuinfo_x86 cpu_data[NR_CPUS] = > + { [0 ... NR_CPUS-1] .apicid = BAD_APICID }; Nit: Stray blank after closing square bracket. Jan
Hi, On 7.02.2024 17:41, Jan Beulich wrote: > On 02.02.2024 19:11, Julien Grall wrote: >> Hi, >> >> On 14/11/2023 17:50, Krystian Hebel wrote: >>> Both fields held the same data. >>> >>> Signed-off-by: Krystian Hebel<krystian.hebel@3mdeb.com> >>> --- >>> xen/arch/x86/boot/x86_64.S | 8 +++++--- >>> xen/arch/x86/include/asm/asm_defns.h | 2 +- >>> xen/arch/x86/include/asm/processor.h | 2 ++ >>> xen/arch/x86/include/asm/smp.h | 4 ---- >>> xen/arch/x86/numa.c | 15 +++++++-------- >>> xen/arch/x86/smpboot.c | 8 ++++---- >>> xen/arch/x86/x86_64/asm-offsets.c | 4 +++- >>> 7 files changed, 22 insertions(+), 21 deletions(-) >>> >>> diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S >>> index b85b47b5c1a0..195550b5c0ea 100644 >>> --- a/xen/arch/x86/boot/x86_64.S >>> +++ b/xen/arch/x86/boot/x86_64.S >>> @@ -20,15 +20,17 @@ ENTRY(__high_start) >>> jz .L_stack_set >>> >>> /* APs only: get stack base from APIC ID saved in %esp. */ >>> - mov $-1, %rax >>> - lea x86_cpu_to_apicid(%rip), %rcx >>> + mov $0, %rax >>> + lea cpu_data(%rip), %rcx >>> + /* cpu_data[0] is BSP, skip it. */ >>> 1: >>> add $1, %rax >>> + add $CPUINFO_X86_sizeof, %rcx >>> cmp $NR_CPUS, %eax >>> jb 2f >>> hlt >>> 2: >>> - cmp %esp, (%rcx, %rax, 4) >>> + cmp %esp, CPUINFO_X86_apicid(%rcx) >>> jne 1b >>> >>> /* %eax is now Xen CPU index. */ >> As mentioned in an earlier patch, I think you want to re-order the >> patches. This will avoid to modify twice the same code within the same >> series (it is best to avoid if you can). > I second this request. Even more so that there's an unexplained move > from starting at $-1 to starting at $0 (in which case you really want > to use xor, not mov). Will do. This may even result in squashing some patches together. >>> --- a/xen/arch/x86/numa.c >>> +++ b/xen/arch/x86/numa.c >>> @@ -54,14 +54,13 @@ bool __init arch_numa_unavailable(void) >>> /* >>> * Setup early cpu_to_node. >>> * >>> - * Populate cpu_to_node[] only if x86_cpu_to_apicid[], >>> - * and apicid_to_node[] tables have valid entries for a CPU. >>> - * This means we skip cpu_to_node[] initialisation for NUMA >>> - * emulation and faking node case (when running a kernel compiled >>> - * for NUMA on a non NUMA box), which is OK as cpu_to_node[] >>> - * is already initialized in a round robin manner at numa_init_array, >>> - * prior to this call, and this initialization is good enough >>> - * for the fake NUMA cases. >>> + * Populate cpu_to_node[] only if cpu_data[], and apicid_to_node[] > You mean cpu_physical_id() here, and then this change wants doing when > switching to that, imo. You mean s/cpu_data[]/cpu_physical_id()/ or something else? >>> + * tables have valid entries for a CPU. This means we skip >>> + * cpu_to_node[] initialisation for NUMA emulation and faking node >>> + * case (when running a kernel compiled for NUMA on a non NUMA box), >>> + * which is OK as cpu_to_node[] is already initialized in a round >>> + * robin manner at numa_init_array, prior to this call, and this >>> + * initialization is good enough for the fake NUMA cases. >>> */ > Also if you're already re-wrapping this comment, please make better use > of line width. > >>> --- a/xen/arch/x86/x86_64/asm-offsets.c >>> +++ b/xen/arch/x86/x86_64/asm-offsets.c >>> @@ -159,7 +159,9 @@ void __dummy__(void) >>> OFFSET(IRQSTAT_softirq_pending, irq_cpustat_t, __softirq_pending); >>> BLANK(); >>> >>> - OFFSET(CPUINFO_features, struct cpuinfo_x86, x86_capability); >>> + OFFSET(CPUINFO_X86_features, struct cpuinfo_x86, x86_capability); >> The rename seems to be unrelated to this patch. Can you clarify? > I agree some renaming wants doing, but separately. That's because we > use CPUINFO_ as a prefix for two entirely different structure's offsets > right now. I'm not convinced of CPUINFO_X86_ as the new prefix though: > Uses are against cpu_data[], so CPUDATA_ may be better. Might be good > if Andrew and/or Roger could voice their view. Yes, this was because after adding APIC ID to this structure I tried to use CPUINFO_sizeof in the assembly, and bad things happened. > > Jan > >>> + OFFSET(CPUINFO_X86_apicid, struct cpuinfo_x86, apicid); >>> + DEFINE(CPUINFO_X86_sizeof, sizeof(struct cpuinfo_x86)); >>> BLANK(); >>> >>> OFFSET(MB_flags, multiboot_info_t, flags); >> Cheers, >> Best regards,
On 8.02.2024 08:29, Jan Beulich wrote: > On 14.11.2023 18:50, Krystian Hebel wrote: >> Both fields held the same data. > Supposedly the same data only. They come from different origins, and you're > hiding this quite well by leaving all sites in place where the field is > written. Both items are also used for entirely separate purposes. So you > need to > - explain why both sources of information necessarily provide the same > data, > - especially if there's remaining concern from the above explanation that > the two values might end up different in corner cases (running > virtualized ourselves comes to mind as a possible example), explain why > nevertheless it is fine (risk free) to use the consolidated item for > all of the originally separate purposes, > - either explain or do away with the multiple places setting this single > remaining field. I missed those writes, thanks for pointing this out. I'll have to take a closer look before deciding what to do with this. > >> --- a/xen/arch/x86/smpboot.c >> +++ b/xen/arch/x86/smpboot.c >> @@ -61,10 +61,8 @@ unsigned int __read_mostly nr_sockets; >> cpumask_t **__read_mostly socket_cpumask; >> static cpumask_t *secondary_socket_cpumask; >> >> -struct cpuinfo_x86 cpu_data[NR_CPUS]; >> - >> -u32 x86_cpu_to_apicid[NR_CPUS] __read_mostly = >> - { [0 ... NR_CPUS-1] = BAD_APICID }; >> +struct cpuinfo_x86 cpu_data[NR_CPUS] = >> + { [0 ... NR_CPUS-1] .apicid = BAD_APICID }; > Nit: Stray blank after closing square bracket. > > Jan
On 12.03.2024 16:29, Krystian Hebel wrote: > On 7.02.2024 17:41, Jan Beulich wrote: >> On 02.02.2024 19:11, Julien Grall wrote: >>> On 14/11/2023 17:50, Krystian Hebel wrote: >>>> --- a/xen/arch/x86/numa.c >>>> +++ b/xen/arch/x86/numa.c >>>> @@ -54,14 +54,13 @@ bool __init arch_numa_unavailable(void) >>>> /* >>>> * Setup early cpu_to_node. >>>> * >>>> - * Populate cpu_to_node[] only if x86_cpu_to_apicid[], >>>> - * and apicid_to_node[] tables have valid entries for a CPU. >>>> - * This means we skip cpu_to_node[] initialisation for NUMA >>>> - * emulation and faking node case (when running a kernel compiled >>>> - * for NUMA on a non NUMA box), which is OK as cpu_to_node[] >>>> - * is already initialized in a round robin manner at numa_init_array, >>>> - * prior to this call, and this initialization is good enough >>>> - * for the fake NUMA cases. >>>> + * Populate cpu_to_node[] only if cpu_data[], and apicid_to_node[] >> You mean cpu_physical_id() here, and then this change wants doing when >> switching to that, imo. > You mean s/cpu_data[]/cpu_physical_id()/ or something else? Well, in general terms - whatever the function in fact accesses. That's, if I reconstruct it from patch 2, as you say then. Jan
diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S index b85b47b5c1a0..195550b5c0ea 100644 --- a/xen/arch/x86/boot/x86_64.S +++ b/xen/arch/x86/boot/x86_64.S @@ -20,15 +20,17 @@ ENTRY(__high_start) jz .L_stack_set /* APs only: get stack base from APIC ID saved in %esp. */ - mov $-1, %rax - lea x86_cpu_to_apicid(%rip), %rcx + mov $0, %rax + lea cpu_data(%rip), %rcx + /* cpu_data[0] is BSP, skip it. */ 1: add $1, %rax + add $CPUINFO_X86_sizeof, %rcx cmp $NR_CPUS, %eax jb 2f hlt 2: - cmp %esp, (%rcx, %rax, 4) + cmp %esp, CPUINFO_X86_apicid(%rcx) jne 1b /* %eax is now Xen CPU index. */ diff --git a/xen/arch/x86/include/asm/asm_defns.h b/xen/arch/x86/include/asm/asm_defns.h index baaaccb26e17..6b05d9d140b8 100644 --- a/xen/arch/x86/include/asm/asm_defns.h +++ b/xen/arch/x86/include/asm/asm_defns.h @@ -158,7 +158,7 @@ register unsigned long current_stack_pointer asm("rsp"); #endif #define CPUINFO_FEATURE_OFFSET(feature) \ - (CPUINFO_features + (cpufeat_word(feature) * 4)) + (CPUINFO_X86_features + (cpufeat_word(feature) * 4)) #else diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h index b0d2a62c075f..8345d58094da 100644 --- a/xen/arch/x86/include/asm/processor.h +++ b/xen/arch/x86/include/asm/processor.h @@ -92,6 +92,8 @@ struct x86_cpu_id { extern struct cpuinfo_x86 cpu_data[]; #define current_cpu_data cpu_data[smp_processor_id()] +#define cpu_physical_id(cpu) cpu_data[cpu].apicid + extern bool probe_cpuid_faulting(void); extern void ctxt_switch_levelling(const struct vcpu *next); extern void (*ctxt_switch_masking)(const struct vcpu *next); diff --git a/xen/arch/x86/include/asm/smp.h b/xen/arch/x86/include/asm/smp.h index c0b5d7cdd8dd..94c557491860 100644 --- a/xen/arch/x86/include/asm/smp.h +++ b/xen/arch/x86/include/asm/smp.h @@ -39,10 +39,6 @@ extern void (*mtrr_hook) (void); extern void zap_low_mappings(void); -extern u32 x86_cpu_to_apicid[]; - -#define cpu_physical_id(cpu) x86_cpu_to_apicid[cpu] - #define cpu_is_offline(cpu) unlikely(!cpu_online(cpu)) extern void cpu_exit_clear(unsigned int cpu); extern void cpu_uninit(unsigned int cpu); diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c index 39e131cb4f35..91527be5b406 100644 --- a/xen/arch/x86/numa.c +++ b/xen/arch/x86/numa.c @@ -54,14 +54,13 @@ bool __init arch_numa_unavailable(void) /* * Setup early cpu_to_node. * - * Populate cpu_to_node[] only if x86_cpu_to_apicid[], - * and apicid_to_node[] tables have valid entries for a CPU. - * This means we skip cpu_to_node[] initialisation for NUMA - * emulation and faking node case (when running a kernel compiled - * for NUMA on a non NUMA box), which is OK as cpu_to_node[] - * is already initialized in a round robin manner at numa_init_array, - * prior to this call, and this initialization is good enough - * for the fake NUMA cases. + * Populate cpu_to_node[] only if cpu_data[], and apicid_to_node[] + * tables have valid entries for a CPU. This means we skip + * cpu_to_node[] initialisation for NUMA emulation and faking node + * case (when running a kernel compiled for NUMA on a non NUMA box), + * which is OK as cpu_to_node[] is already initialized in a round + * robin manner at numa_init_array, prior to this call, and this + * initialization is good enough for the fake NUMA cases. */ void __init init_cpu_to_node(void) { diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c index de87c5a41926..f061486e56eb 100644 --- a/xen/arch/x86/smpboot.c +++ b/xen/arch/x86/smpboot.c @@ -61,10 +61,8 @@ unsigned int __read_mostly nr_sockets; cpumask_t **__read_mostly socket_cpumask; static cpumask_t *secondary_socket_cpumask; -struct cpuinfo_x86 cpu_data[NR_CPUS]; - -u32 x86_cpu_to_apicid[NR_CPUS] __read_mostly = - { [0 ... NR_CPUS-1] = BAD_APICID }; +struct cpuinfo_x86 cpu_data[NR_CPUS] = + { [0 ... NR_CPUS-1] .apicid = BAD_APICID }; static int cpu_error; static enum cpu_state { @@ -81,7 +79,9 @@ void *stack_base[NR_CPUS]; void initialize_cpu_data(unsigned int cpu) { + uint32_t apicid = cpu_physical_id(cpu); cpu_data[cpu] = boot_cpu_data; + cpu_physical_id(cpu) = apicid; } static bool smp_store_cpu_info(unsigned int id) diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c index 57b73a4e6214..e881cd5de0a0 100644 --- a/xen/arch/x86/x86_64/asm-offsets.c +++ b/xen/arch/x86/x86_64/asm-offsets.c @@ -159,7 +159,9 @@ void __dummy__(void) OFFSET(IRQSTAT_softirq_pending, irq_cpustat_t, __softirq_pending); BLANK(); - OFFSET(CPUINFO_features, struct cpuinfo_x86, x86_capability); + OFFSET(CPUINFO_X86_features, struct cpuinfo_x86, x86_capability); + OFFSET(CPUINFO_X86_apicid, struct cpuinfo_x86, apicid); + DEFINE(CPUINFO_X86_sizeof, sizeof(struct cpuinfo_x86)); BLANK(); OFFSET(MB_flags, multiboot_info_t, flags);
Both fields held the same data. Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com> --- xen/arch/x86/boot/x86_64.S | 8 +++++--- xen/arch/x86/include/asm/asm_defns.h | 2 +- xen/arch/x86/include/asm/processor.h | 2 ++ xen/arch/x86/include/asm/smp.h | 4 ---- xen/arch/x86/numa.c | 15 +++++++-------- xen/arch/x86/smpboot.c | 8 ++++---- xen/arch/x86/x86_64/asm-offsets.c | 4 +++- 7 files changed, 22 insertions(+), 21 deletions(-)