Re: [PATCH 2/5] x86/boot/64: Clear BSS as early as possible

[Brian Gerst]

On Fri, Jul 24, 2026 at 7:34 AM Brian Gerst <brgerst@xxxxxxxxx> wrote:
>
> On Fri, Jul 24, 2026 at 7:04 AM Nikolay Borisov <nik.borisov@xxxxxxxx> wrote:
> >
> >
> >
> > On 7/24/26 06:02, Brian Gerst wrote:
> > > Currently, the BSS section is not cleared until x86_64_start_kernel().
> > > Using unitialized BSS data before that point leads to difficult to debug
> > > problems.  Fix this by moving clear_bss() to as early as possible.
> > >
> > > Signed-off-by: Brian Gerst <brgerst@xxxxxxxxx>
> >
> > <snip>
> >
> >
> > > diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
> > > index 7ed5520dd52e..15ef5ea52b9a 100644
> > > --- a/arch/x86/kernel/head_64.S
> > > +++ b/arch/x86/kernel/head_64.S
> > > @@ -37,6 +37,8 @@
> > >       .code64
> > >   SYM_CODE_START_NOALIGN(startup_64)
> > >       UNWIND_HINT_END_OF_STACK
> > > +     cld
> > > +
> > >       /*
> > >        * At this point the CPU runs in 64bit mode CS.L = 1 CS.D = 0,
> > >        * and someone has loaded an identity mapped page table
> > > @@ -61,6 +63,8 @@ SYM_CODE_START_NOALIGN(startup_64)
> > >       /* Set up the stack for verify_cpu() */
> > >       leaq    __top_init_kernel_stack(%rip), %rsp
> > >
> > > +     call    clear_bss
> > > +
> > >       /*
> > >        * Set up GSBASE.
> > >        * Note that on SMP the boot CPU uses the init data section until
> > > @@ -140,6 +144,22 @@ SYM_CODE_START_NOALIGN(startup_64)
> > >       jmp     *.Lcommon_startup_64(%rip)
> > >   SYM_CODE_END(startup_64)
> > >
> > > +SYM_FUNC_START(clear_bss)
> > > +     xorl    %eax, %eax
> > > +
> > > +     leaq    __bss_start(%rip), %rdi
> > > +     leaq    __bss_stop(%rip), %rcx
> > > +     subq    %rdi, %rcx
> > > +     rep stosb
> >
> > Why not stosq (with appropriate adjustment to xor %rax, %rax as well)?
> > The length is guaranteed to be multiple of page_size ergo 8 byte as well ?
>
> I used code similar to what GCC compiled the original memset() to.
> Using a larger width store would require adding a shift instruction
> for RCX.  This is one-time boot code so performance isn't critical,
> and some modern processors favor REP STOSB anyways (X86_FEATURE_ERMS).
>
> > > +
> > > +     leaq    __brk_base(%rip), %rdi
> > > +     leaq    __brk_limit(%rip), %rcx
> > > +     subq    %rdi, %rcx
> > > +     rep stosb
> > > +
> > > +     RET
> > > +SYM_FUNC_END(clear_bss)
> > > +
> > >       __INITRODATA
> > >   SYM_DATA_LOCAL(.Lcommon_startup_64, .quad common_startup_64)
> > >
> > > diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> > > index 2c64b388f616..6aa13d19c0a4 100644
> > > --- a/arch/x86/xen/enlighten_pv.c
> > > +++ b/arch/x86/xen/enlighten_pv.c
> > > @@ -1334,8 +1334,6 @@ asmlinkage __visible void __init xen_start_kernel(struct start_info *si)
> > >       if (!si)
> > >               return;
> > >
> > > -     clear_bss();
> > > -
> > >       xen_start_info = si;
> > >
> > >       __text_gen_insn(&early_xen_iret_patch,
> > > diff --git a/arch/x86/xen/xen-head.S b/arch/x86/xen/xen-head.S
> > > index 5dad6c51cdc3..9b56659246fe 100644
> > > --- a/arch/x86/xen/xen-head.S
> > > +++ b/arch/x86/xen/xen-head.S
> > > @@ -31,6 +31,9 @@ SYM_CODE_START(startup_xen)
> > >
> > >       leaq    __top_init_kernel_stack(%rip), %rsp
> > >
> > > +     movq    %rsi, %r15
> >
> > Why is this change necessary, rsi is ont used in clear_bss ?
>
> It contains the start_info parameter that is passed to
> xen_start_kernel(), which needs to be preserved over the call to
> clear_bss().

On rereading your question, I now see the point you were trying to
make.  Yes, technically clear_bss() does not clobber RSI.  But I'd
rather treat it as a standard calling convention function (RSI not
preserved) to avoid confusion.