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

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

[Jason Andryuk]

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@amd.com>
---
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")
---
 xen/arch/x86/smpboot.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index ff05955bae..a5d485b732 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -643,10 +643,16 @@ static int do_boot_cpu(int apicid, int cpu)
 
 #define STUB_BUF_CPU_OFFS(cpu) (((cpu) & (STUBS_PER_PAGE - 1)) * STUB_BUF_SIZE)
 
+/* Fill values indicating state of stub. */
+#define STUB_BUF_USED 0xcc
+#define STUB_BUF_FREE 0xd6
+
 unsigned long alloc_stub_page(unsigned int cpu, unsigned long *mfn)
 {
+    unsigned char *stub_page;
     unsigned long stub_va;
     struct page_info *pg;
+    bool initialize = false;
 
     BUILD_BUG_ON(STUBS_PER_PAGE & (STUBS_PER_PAGE - 1));
 
@@ -661,7 +667,7 @@ unsigned long alloc_stub_page(unsigned int cpu, unsigned long *mfn)
         if ( !pg )
             return 0;
 
-        unmap_domain_page(memset(__map_domain_page(pg), 0xcc, PAGE_SIZE));
+        initialize = true;
     }
 
     stub_va = XEN_VIRT_END - FIXADDR_X_SIZE - (cpu + 1) * PAGE_SIZE;
@@ -675,6 +681,14 @@ unsigned long alloc_stub_page(unsigned int cpu, unsigned long *mfn)
     else if ( !*mfn )
         *mfn = mfn_x(page_to_mfn(pg));
 
+    stub_page = __map_domain_page(pg);
+    /* Newly allocated page is marked entirely unused. */
+    if ( initialize )
+        memset(stub_page, STUB_BUF_FREE, PAGE_SIZE);
+    /* Specific CPU is marked used. */
+    memset(stub_page + STUB_BUF_CPU_OFFS(cpu), STUB_BUF_USED, STUB_BUF_SIZE);
+    unmap_domain_page(stub_page);
+
     return stub_va;
 }
 
@@ -992,9 +1006,10 @@ static void cpu_smpboot_free(unsigned int cpu, bool remove)
         unsigned char *stub_page = map_domain_page(mfn);
         unsigned int i;
 
-        memset(stub_page + STUB_BUF_CPU_OFFS(cpu), 0xcc, STUB_BUF_SIZE);
+        memset(stub_page + STUB_BUF_CPU_OFFS(cpu), STUB_BUF_FREE,
+               STUB_BUF_SIZE);
         for ( i = 0; i < STUBS_PER_PAGE; ++i )
-            if ( stub_page[i * STUB_BUF_SIZE] != 0xcc )
+            if ( stub_page[i * STUB_BUF_SIZE] != STUB_BUF_FREE )
                 break;
         unmap_domain_page(stub_page);
         destroy_xen_mappings(per_cpu(stubs.addr, cpu) & PAGE_MASK,
-- 
2.54.0

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

[Andrew Cooper]

On 26/05/2026 9:31 pm, 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.

It's more complicated than this.

With CONFIG_PV (and !opt_fred in 4.22 which is perhaps newer than you're
testing), the LSTAR and CSTAR stubs guarantee that the 0xcc's are
overwritten with real instructions.

In !CONFIG_PV, the 0xcc's only get overwritten by the exception recovery
selftests (CPU0 only, and gated on CONFIG_SELF_TESTS), and "complicated"
instructions in the emulator (which in your safety environment, you
likely have compiled out).

So, in your environment, I think you probably can exclude the stubs
entirely and trim even more LoC.

>
> 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@amd.com>
> ---
> 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")

Honestly, I dislike all of this "try to free data for going-offline
CPUs".  It is both complex and a non-stop source of bugs for tantamount
to 0 benefit.

On x86, we must boot all CPUs we find in the MADT.  You're seeing this
behaviour already.  This is because if an #MC hits any group of CPUs
where any CR4.MCE=0, it's an instant reset.

For this reason, firmware doesn't hand APs over to the OS in the
Wait-for-SIPI state (which resets CR4 to 0); they're in MWAIT or IO-wait
typically these days, using firmware provided stacks.  But firmware
cannot handle an #MC intended for the OS, so the OS must set up stacks
and at least an NMI and #MC handler even for those CPUs not wanting to run.

This what park_offline_cpus is trying to do, and while it's set for
Intel and clear for AMD, I'm pretty sure this is a bug on AMD because
you can still get MCEs with core-scope groups.


Beyond that,  smt=0 is an emergency bodge for speculation safety, which
is always better done by changing SMT settings in the firmware. 
xen-hptool is useful for testing but it's not a thing anyone uses in a
production system.

ACPI CPU hot-add does exist in virtual environments, but hot-remove is
theoretical at best.  I've not seen any evidence of ACPI hotplug
actually working on Xen, and I think the chances that it does are slim;
it requires AML execution, and is right in the middle of the split-brain
problem with physical vs virtual details that dom0 suffers.


So, lets just allocate the stubs and "leak" them in testing scenarios. 
It removes bugs and removes code, and has no effect on well-configured
systems (where cpu offline is not used in practice).

~Andrew

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

[Jason Andryuk]

On 2026-05-26 18:03, Andrew Cooper wrote:
> On 26/05/2026 9:31 pm, 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.
> 
> It's more complicated than this.
> 
> With CONFIG_PV (and !opt_fred in 4.22 which is perhaps newer than you're
> testing), the LSTAR and CSTAR stubs guarantee that the 0xcc's are
> overwritten with real instructions.
> 
> In !CONFIG_PV, the 0xcc's only get overwritten by the exception recovery
> selftests (CPU0 only, and gated on CONFIG_SELF_TESTS), and "complicated"
> instructions in the emulator (which in your safety environment, you
> likely have compiled out).
> 
> So, in your environment, I think you probably can exclude the stubs
> entirely and trim even more LoC.

Thanks.  Ok, my build was !CONFIG_PV, so 0xcc's were not overwritten. 
The fault happened before the self tests ran.

>>
>> 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@amd.com>
>> ---
>> 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")
> 
> Honestly, I dislike all of this "try to free data for going-offline
> CPUs".  It is both complex and a non-stop source of bugs for tantamount
> to 0 benefit.
> 
> On x86, we must boot all CPUs we find in the MADT.  You're seeing this
> behaviour already.  This is because if an #MC hits any group of CPUs
> where any CR4.MCE=0, it's an instant reset.
> 
> For this reason, firmware doesn't hand APs over to the OS in the
> Wait-for-SIPI state (which resets CR4 to 0); they're in MWAIT or IO-wait
> typically these days, using firmware provided stacks.  But firmware
> cannot handle an #MC intended for the OS, so the OS must set up stacks
> and at least an NMI and #MC handler even for those CPUs not wanting to run.
> 
> This what park_offline_cpus is trying to do, and while it's set for
> Intel and clear for AMD, I'm pretty sure this is a bug on AMD because
> you can still get MCEs with core-scope groups.
> 
> 
> Beyond that,  smt=0 is an emergency bodge for speculation safety, which
> is always better done by changing SMT settings in the firmware.

I was asked to check something with SMT disabled, and I could not find 
SMT in my firmware.  I looked for a while without success, and then set 
smt=0 :/

> xen-hptool is useful for testing but it's not a thing anyone uses in a
> production system.
> 
> ACPI CPU hot-add does exist in virtual environments, but hot-remove is
> theoretical at best.  I've not seen any evidence of ACPI hotplug
> actually working on Xen, and I think the chances that it does are slim;
> it requires AML execution, and is right in the middle of the split-brain
> problem with physical vs virtual details that dom0 suffers.
> 
> 
> So, lets just allocate the stubs and "leak" them in testing scenarios.
> It removes bugs and removes code, and has no effect on well-configured
> systems (where cpu offline is not used in practice).

Ok.

Thanks,
Jason

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

[Jason Andryuk]

On 2026-05-27 10:14, Jason Andryuk wrote:
> On 2026-05-26 18:03, Andrew Cooper wrote:
>> On 26/05/2026 9:31 pm, 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.
>>
>> It's more complicated than this.
>>
>> With CONFIG_PV (and !opt_fred in 4.22 which is perhaps newer than you're
>> testing), the LSTAR and CSTAR stubs guarantee that the 0xcc's are
>> overwritten with real instructions.
>>
>> In !CONFIG_PV, the 0xcc's only get overwritten by the exception recovery
>> selftests (CPU0 only, and gated on CONFIG_SELF_TESTS), and "complicated"
>> instructions in the emulator (which in your safety environment, you
>> likely have compiled out).
>>
>> So, in your environment, I think you probably can exclude the stubs
>> entirely and trim even more LoC.
> 
> Thanks.  Ok, my build was !CONFIG_PV, so 0xcc's were not overwritten. 
> The fault happened before the self tests ran.

Correction: It was after the self tests ran and during dom0 construction.

(XEN) Pagetable walk from ffff830842652008:
(XEN)  L4[0x106] = 8000000079c72063 ffffffffffffffff
(XEN)  L3[0x021] = 0000000079ff3063 ffffffffffffffff
(XEN)  L2[0x013] = 000000085680f063 ffffffffffffffff
(XEN)  L1[0x052] = cccccccccccccccc ffffffffffffffff

It looks like the page is reallocated after free-ing, so after CPU1 is 
down.  The re-use would write the page with PTEs.  However, when later 
CPUs are brought down, their portion of the stubs page is overwritten 
with 0xcc.  I think that is how the page, as a page table, is corrupted.

Regards,
Jason

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

[Roger Pau Monné]

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@amd.com>
> ---
> 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.

Thanks, Roger.

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

[Jason Andryuk]

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@amd.com>
>> ---
>> 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

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

[Roger Pau Monné]

On Mon, Jun 01, 2026 at 05:07:52PM -0400, Jason Andryuk wrote:
> 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@amd.com>
> > > ---
> > > 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) )

I think I've made an oversight in the code above: if all 32 CPUs
sharing the same stubs page are offlined, the reference to the stubs
page is possibly lost (if CPUs are not parked) and a new stubs page
would be allocated if any of those CPUs is brought back online, thus
leaking the previous allocation.  The simplest way to solve this would
be to introduce an array that indexes the stub pages, and replace the
logic in cpu_smpboot_alloc() that figures out whether stubs.mfn is set
for adjacent CPUs.

> > (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?

Depends on whether the CPUs are parked or not (see park_offline_cpus).
I think leaving stubs.mfn is fine, in the parked case we avoid part of
the setup logic by already having the mfn cached (no big deal either
way).

> Also, I was waiting to see if anyone chimed in with other ideas.

Maybe you could assign the page to dom_xen and then use
{get,put}_page(), but again it seems overly complicated.

Thanks, Roger.

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

[Jason Andryuk]

On 2026-06-02 03:01, Roger Pau Monné wrote:
> On Mon, Jun 01, 2026 at 05:07:52PM -0400, Jason Andryuk wrote:
>> 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@amd.com>
>>>> ---
>>>> 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) )
> 
> I think I've made an oversight in the code above: if all 32 CPUs
> sharing the same stubs page are offlined, the reference to the stubs
> page is possibly lost (if CPUs are not parked) and a new stubs page
> would be allocated if any of those CPUs is brought back online, thus
> leaking the previous allocation.  The simplest way to solve this would
> be to introduce an array that indexes the stub pages, and replace the
> logic in cpu_smpboot_alloc() that figures out whether stubs.mfn is set
> for adjacent CPUs.

Right, but I thought Andrew's point was that offlining 32 CPUs is 
unrealistic, so don't even bother tracking.  If CPUs are offlined (and 
you somehow keep running), you can leak the page.

>>> (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?
> 
> Depends on whether the CPUs are parked or not (see park_offline_cpus).
> I think leaving stubs.mfn is fine, in the parked case we avoid part of
> the setup logic by already having the mfn cached (no big deal either
> way).

Right.

>> Also, I was waiting to see if anyone chimed in with other ideas.
> 
> Maybe you could assign the page to dom_xen and then use
> {get,put}_page(), but again it seems overly complicated.

Code-wise this doesn't look bad, but it blows up:

(XEN) d[IDLE]v0 Over-allocation for d[XEN]: 1 > 0

I don't think we should pursue that.

Regards,
Jason

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

[Roger Pau Monné]

On Wed, Jun 03, 2026 at 10:03:53AM -0400, Jason Andryuk wrote:
> On 2026-06-02 03:01, Roger Pau Monné wrote:
> > On Mon, Jun 01, 2026 at 05:07:52PM -0400, Jason Andryuk wrote:
> > > 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@amd.com>
> > > > > ---
> > > > > 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) )
> > 
> > I think I've made an oversight in the code above: if all 32 CPUs
> > sharing the same stubs page are offlined, the reference to the stubs
> > page is possibly lost (if CPUs are not parked) and a new stubs page
> > would be allocated if any of those CPUs is brought back online, thus
> > leaking the previous allocation.  The simplest way to solve this would
> > be to introduce an array that indexes the stub pages, and replace the
> > logic in cpu_smpboot_alloc() that figures out whether stubs.mfn is set
> > for adjacent CPUs.
> 
> Right, but I thought Andrew's point was that offlining 32 CPUs is
> unrealistic, so don't even bother tracking.  If CPUs are offlined (and you
> somehow keep running), you can leak the page.

I thin we should avoid freeing the page and ensure it's always reused,
rather than possibly leaking it.  It's also possible there's a single
trailing CPU using the last stubs page alone, and offlining and
onlining it would trigger such a page leak, without requiring a block
of 32 CPUs going offline.

Entering an ACPI sleep state causes all APs to be offlined (see the
disable_nonboot_cpus() call in enter_state()), and it would be
undesirable that putting a system to sleep causes page leaking.

> > > > (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?
> > 
> > Depends on whether the CPUs are parked or not (see park_offline_cpus).
> > I think leaving stubs.mfn is fine, in the parked case we avoid part of
> > the setup logic by already having the mfn cached (no big deal either
> > way).
> 
> Right.
> 
> > > Also, I was waiting to see if anyone chimed in with other ideas.
> > 
> > Maybe you could assign the page to dom_xen and then use
> > {get,put}_page(), but again it seems overly complicated.
> 
> Code-wise this doesn't look bad, but it blows up:
> 
> (XEN) d[IDLE]v0 Over-allocation for d[XEN]: 1 > 0
> 
> I don't think we should pursue that.

Hm, I see, yes, you will need to use MEMF_no_refcount to skip the
domain allocation accounting, but as you say it might not be a very
wise idea.

Thanks, Roger.

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

[Jan Beulich]

On 03.06.2026 16:16, Roger Pau Monné wrote:
> On Wed, Jun 03, 2026 at 10:03:53AM -0400, Jason Andryuk wrote:
>> On 2026-06-02 03:01, Roger Pau Monné wrote:
>>> On Mon, Jun 01, 2026 at 05:07:52PM -0400, Jason Andryuk wrote:
>>>> 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@amd.com>
>>>>>> ---
>>>>>> 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) )
>>>
>>> I think I've made an oversight in the code above: if all 32 CPUs
>>> sharing the same stubs page are offlined, the reference to the stubs
>>> page is possibly lost (if CPUs are not parked) and a new stubs page
>>> would be allocated if any of those CPUs is brought back online, thus
>>> leaking the previous allocation.  The simplest way to solve this would
>>> be to introduce an array that indexes the stub pages, and replace the
>>> logic in cpu_smpboot_alloc() that figures out whether stubs.mfn is set
>>> for adjacent CPUs.
>>
>> Right, but I thought Andrew's point was that offlining 32 CPUs is
>> unrealistic, so don't even bother tracking.  If CPUs are offlined (and you
>> somehow keep running), you can leak the page.
> 
> I thin we should avoid freeing the page and ensure it's always reused,
> rather than possibly leaking it.  It's also possible there's a single
> trailing CPU using the last stubs page alone, and offlining and
> onlining it would trigger such a page leak, without requiring a block
> of 32 CPUs going offline.
> 
> Entering an ACPI sleep state causes all APs to be offlined (see the
> disable_nonboot_cpus() call in enter_state()), and it would be
> undesirable that putting a system to sleep causes page leaking.

Suspend is handled specially by some CPU notifier handlers, see in
particular common/percpu.c:cpu_percpu_callback(). That in particular
means that per-CPU data survives suspend/resume. Which may be possible
to leverage here to avoid a leak across S3 (and perhaps even across
the pretty similar parking), while still accepting a leak for "real"
CPU offlining. (Which isn't to say that avoiding leaks altogether
wouldn't be the most desirable goal.)

Jan

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

[Andrew Cooper]

On 03/06/2026 3:03 pm, Jason Andryuk wrote:
> On 2026-06-02 03:01, Roger Pau Monné wrote:
>> On Mon, Jun 01, 2026 at 05:07:52PM -0400, Jason Andryuk wrote:
>>> 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@amd.com>
>>>>> ---
>>>>> 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) )
>>
>> I think I've made an oversight in the code above: if all 32 CPUs
>> sharing the same stubs page are offlined, the reference to the stubs
>> page is possibly lost (if CPUs are not parked) and a new stubs page
>> would be allocated if any of those CPUs is brought back online, thus
>> leaking the previous allocation.  The simplest way to solve this would
>> be to introduce an array that indexes the stub pages, and replace the
>> logic in cpu_smpboot_alloc() that figures out whether stubs.mfn is set
>> for adjacent CPUs.
>
> Right, but I thought Andrew's point was that offlining 32 CPUs is
> unrealistic, so don't even bother tracking.  If CPUs are offlined (and
> you somehow keep running), you can leak the page.

Perhaps I should rephrase that slightly.

I don't think we want to fully leak the page (after all, there *is* a
reference to it staying in l2_xenmap[]), but I also think we should
bother having logic attempting to free it.  Software-offlining 32
adjacent threads is unrealistic, and not worth the effort (particularly
when the result is this).

Rework the code to use l2_xenmap[] as the source of truth, and allocate
a new ownerless page if the pagetables say one doesn't exist.  When a
CPU comes up, it can derive addr from its CPU number, and pull MFN out
of the pagetable.

~Andrew