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.he...@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,
--
Krystian Hebel
Firmware Engineer
https://3mdeb.com | @3mdeb_com
