Re: [PATCH] xen/x86: Change stub page freeing to fix smt=0

[Jason Andryuk <jason.andryuk@xxxxxxx>]

On 2026-06-01 13:00, Roger Pau Monné wrote:
> On Tue, May 26, 2026 at 04:31:14PM -0400, Jason Andryuk wrote:
> > A single stubs page is initialized with 0xcc and re-used, with multiple
> > CPUs each using a portion of the shared page.  In cpu_smpboot_free(),
> > each stubs area is checked against 0xcc.  When all are set to 0xcc, the
> > page is freed.
> >
> > Booting a system with smt=0, CPU0 is initially setup, allocating the
> > stubs page and initializing to 0xcc.  When more CPUs are brought up,
> > CPU1 is initialized and then immediately brough offline as it is the
> > sibling of CPU0.  Since the page was initially memset with 0xcc,
> > cpu_smpboot_free() finds all stubs as 0xcc and frees the page.
> > However, the page is still assigned to CPU0 and continues to be assigned
> > to other CPUs.
> >
> > Meanwhile the page can be reallocated, which can lead to misbehavior.
> > The particular instance was the stubs page re-used as a page table which
> > later faulted when the entry was all 0xcc.
> >
> > Change to initializing the page as 0xd6/STUB_BUF_FREE, and initializing
> > individual stubs as 0xcc/STUB_BUF_USED.  0xd6 now indicates unused, and
> > 0xcc indicates used/assigned.  When freeing a CPU, the stub is set to
> > 0xd6, and the page is freed if all stubs are 0xd6.  Initializing with
> > STUB_BUF_FREE lets cpu_smpboot_free() a page that was only ever
> > partially used.
> >
> > 0xd6/UDB is a 1 byte invalid opcode, which is similar to the existing
> > use of 0xcc.  0xd6 is used to identify bug frames, but the stub addr
> > (e.g. 0xffff82d07fffe000) fails the is_active_kernel_text() check.  It
> > should be okay to use here.
> >
> > Fixes: 7a66ac8d1633 ("x86: move syscall trampolines off the stack")
> > Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> > ---
> > It would be nice to use get_page()/put_page() to let count_info handle
> > reference counting, but they require an owning domain.
> >
> > The listed Fixes introduced the use of 0xcc, but the smt commit may have
> > made it more problematic.
> > Fixes: d8f974f1a646 ("x86: command line option to avoid use of secondary 
> > hyper-threads")
> Speaking with Andrew, we believe it might be easier to simply forego
> the freeing of the page, possibly something like:
>
> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
> index ff05955bae40..62c6cbf4b561 100644
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -990,19 +990,12 @@ static void cpu_smpboot_free(unsigned int cpu, bool 
> remove)
>       {
>           mfn_t mfn = _mfn(per_cpu(stubs.mfn, cpu));
>           unsigned char *stub_page = map_domain_page(mfn);
> -        unsigned int i;
>   
>           memset(stub_page + STUB_BUF_CPU_OFFS(cpu), 0xcc, STUB_BUF_SIZE);
> -        for ( i = 0; i < STUBS_PER_PAGE; ++i )
> -            if ( stub_page[i * STUB_BUF_SIZE] != 0xcc )
> -                break;
>           unmap_domain_page(stub_page);
>           destroy_xen_mappings(per_cpu(stubs.addr, cpu) & PAGE_MASK,
>                                (per_cpu(stubs.addr, cpu) | ~PAGE_MASK) + 1);
>           per_cpu(stubs.addr, cpu) = 0;
> -        per_cpu(stubs.mfn, cpu) = 0;
> -        if ( i == STUBS_PER_PAGE )
> -            free_domheap_page(mfn_to_page(mfn));
>       }
>   
>       if ( IS_ENABLED(CONFIG_PV32) )
> (there might be further cleanup possible if the page is not freed, the
> above chunk is untested).
>
> It's a single page shared between 32 CPUs, and offlining 32 adjacent
> CPUs seems very unlikely.  IMO the extra complexity of having to deal
> with the freeing overshadows the very small memory gain we get from
> it.
Hi Roger,

Yes, I made and tested the same change locally last week.  Well, I retained:
     per_cpu(stubs.mfn, cpu) = 0;

Maybe it would be good to save the mfn in case the CPU returns?  But I 
thought per-cpu vars are cleared, so it wouldn't be available anyway?
Also, I was waiting to see if anyone chimed in with other ideas.

Thanks,
Jason