Hello, Damien Zammit, le mer. 07 janv. 2026 03:07:39 +0000, a ecrit: > diff --git a/x86_64/boothdr.S b/x86_64/boothdr.S > index 43c6d54c..42423b31 100644 > --- a/x86_64/boothdr.S > +++ b/x86_64/boothdr.S > @@ -52,6 +55,17 @@ boot_hdr: > .global _start > _start: > boot_entry: > + /* Enable local apic in xAPIC mode */ > + xorl %eax, %eax > + xorl %edx, %edx > + movl $APIC_MSR, %ecx > + rdmsr > + orl $APIC_MSR_ENABLE, %eax > + orl $APIC_MSR_BSP, %eax > + andl $(~APIC_MSR_X2APIC), %eax > + movl $APIC_MSR, %ecx > + wrmsr
I'm realizing that at the time that was added to i386 we didn't pay attention: this should be under #ifdef APIC, shouldn't it? Also, please separate unrelated changes in separate commits, apic and percpu have really nothing to do with each other. As you have noticed, precise bisection is crucial to find commits that bring regressions. > @@ -166,6 +180,39 @@ boot_entry64: > andq $(~15),%rax > movq %rax,%rsp > > + /* Set GS base address */ > + movq $percpu_array, %rdx > + movl %edx, %eax > + shrq $32, %rdx > + movl $MSR_REG_GSBASE, %ecx > + wrmsr > + /* Reload gdt in long mode takes 2 args */ > + movw gdt64pointer, %di > + movq gdt64pointer+2, %rsi These two moves don't seem needed? (leftover debug I guess?) > + lgdt gdt64pointer Is it really needed to reload the gdt? The content hasn't changed. > + movw $PERCPU_DS,%ax > + movw %ax,%gs > + > + /* instead of ljmp */ > + movq $fixup64, %rcx > + pushq $8 > + pushq %rcx > + /* far return to below - performs actual reload of gdt */ > + retfq > + > +fixup64: Again, is this *really* needed? The gdt content has already been loaded way above, and we have already jumped to BOOT_CS. > + /* Set KernelGS base address */ > + movq $percpu_array, %rdx > + movl %edx, %eax > + shrq $32, %rdx > + movl $MSR_REG_KGSBASE, %ecx > + wrmsr While in kernel mode, KGSBASE is supposed to contain the user gs base, not the kernel gs base. That whole part should rather be in #if NCPUS > 1, shouldn't it? > + > + /* Reset stack */ > + andq $(~0xf),%rsp Don't we have already aligned that on boot_entry64? Or perhaps it's your pushq $8 which is wrong and should be pushw $8. > @@ -200,16 +247,47 @@ iplt_done: > .section .boot.data > .align 4096 > #define SEG_ACCESS_OFS 40 > -#define SEG_GRANULARITY_OFS 52 > +#define SEG_FLAGS_OFS 52 > gdt64: > /* NULL segment = 0x0 */ > .quad 0 > > /* BOOT_CS = 0x8 */ > - .quad (ACC_P << SEG_ACCESS_OFS) | (ACC_CODE_R << > SEG_ACCESS_OFS) | (SZ_64 << SEG_GRANULARITY_OFS) > + .quad ((ACC_A | ACC_P | ACC_CODE_R) << SEG_ACCESS_OFS) | > (SZ_64 << SEG_FLAGS_OFS) Why adding ACC_A? It seems completely useless. Samuel
