[PATCH for-4.22 v3] xen/x86: Change stub page allocation/free

[PATCH for-4.22 v3] xen/x86: Change stub page allocation/free

[Roger Pau Monne]

From: Jason Andryuk <jason.andryuk@amd.com>

Today the inline tracking of the stub page is problematic.  0xcc is used to
indicate unused, but it is also a "clear value."  A !CONFIG_PV build or
when running with FRED support will not populate the LSTAR/CSTAR stubs at
CPU bringup.  If a CPU is then offlined, the stubs page will be freed as
its content will be all 0xcc, regardless of the stubs page still begin
referenced by other CPUs.

The new approach uses a global, CPU-indexed dynamically allocated array of
stub addresses.  However, to handle NUMA aware allocations, we cannot
allocate all the memory in advance because of the NUMA dependency.  Take
advantage of the fact that Xen will attempt to contiguously pack CPUs on
the same NUMA node (see normalise_cpu_order()), and on CPU bringup use the
same stubs page the previous CPU did if suitable.  Note the code would
still function properly even if CPUs from NUMA nodes are not contiguously
packed, it just consumes more memory.

stub pages are no longer freed.  They remain referenced in the global
CPU-indexed array and are re-used if the CPU is re-onlined.

The stubs array doesn't have an explicit lock.  During boot it's accessed
single threaded.  During runtime, &cpu_add_remove_lock serializes access.

Fixes: 7a66ac8d1633 ("x86: move syscall trampolines off the stack")
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Tested-by: Jason Andryuk <jason.andryuk@amd.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Release-Acked-by: Oleksii Kurochko <oleskii.kurochko@gmail.com>
---
Changes since v2:
 - Adjust commit message.
 - Handle the case where stubs[cpu - 1] == PADDR_INVALID.
---
 xen/arch/x86/include/asm/stubs.h |  2 +-
 xen/arch/x86/setup.c             |  4 +-
 xen/arch/x86/smpboot.c           | 94 +++++++++++++++++---------------
 3 files changed, 52 insertions(+), 48 deletions(-)

diff --git a/xen/arch/x86/include/asm/stubs.h b/xen/arch/x86/include/asm/stubs.h
index a520928e9a50..467551136a2a 100644
--- a/xen/arch/x86/include/asm/stubs.h
+++ b/xen/arch/x86/include/asm/stubs.h
@@ -32,6 +32,6 @@ struct stubs {
 };
 
 DECLARE_PER_CPU(struct stubs, stubs);
-unsigned long alloc_stub_page(unsigned int cpu, unsigned long *mfn);
+void init_stub(void);
 
 #endif /* X86_ASM_STUBS_H */
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 4192edf635b6..0253d22c349d 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -2089,9 +2089,7 @@ void asmlinkage __init noreturn __start_xen(void)
 
     init_idle_domain();
 
-    this_cpu(stubs.addr) = alloc_stub_page(smp_processor_id(),
-                                           &this_cpu(stubs).mfn);
-    BUG_ON(!this_cpu(stubs.addr));
+    init_stub();
 
     bsp_traps_reinit(); /* Needs stubs allocated, must be before presmp_initcalls. */
 
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index d8fd71ffab37..76e7ee6af6ed 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -20,6 +20,7 @@
 #include <xen/serial.h>
 #include <xen/softirq.h>
 #include <xen/tasklet.h>
+#include <xen/xvmalloc.h>
 
 #include <asm/apic.h>
 #include <asm/cpuidle.h>
@@ -641,41 +642,64 @@ static int do_boot_cpu(int apicid, int cpu)
     return rc;
 }
 
-#define STUB_BUF_CPU_OFFS(cpu) (((cpu) & (STUBS_PER_PAGE - 1)) * STUB_BUF_SIZE)
+/* Dynamically allocated, indexed by CPU.  Store physical address of stubs. */
+static paddr_t *__ro_after_init stubs;
 
-unsigned long alloc_stub_page(unsigned int cpu, unsigned long *mfn)
+static bool assign_stub_page(unsigned int cpu)
 {
     unsigned long stub_va;
-    struct page_info *pg;
+    paddr_t addr = stubs[cpu];
 
-    BUILD_BUG_ON(STUBS_PER_PAGE & (STUBS_PER_PAGE - 1));
-
-    if ( *mfn )
-        pg = mfn_to_page(_mfn(*mfn));
-    else
+    if ( addr == INVALID_PADDR )
     {
-        nodeid_t node = cpu_to_node(cpu);
-        unsigned int memflags = node != NUMA_NO_NODE ? MEMF_node(node) : 0;
+        nodeid_t nid = cpu_to_node(cpu);
+
+        /*
+         * Attempt to use the same page as the previous CPU if possible,
+         * otherwise allocate a new one.
+         */
+        if ( cpu && nid == cpu_to_node(cpu - 1) &&
+             stubs[cpu - 1] != INVALID_PADDR &&
+             PAGE_OFFSET(stubs[cpu - 1] + STUB_BUF_SIZE) )
+            addr = stubs[cpu - 1] + STUB_BUF_SIZE;
+        else
+        {
+            struct page_info *pg = alloc_domheap_page(NULL, MEMF_node(nid));
 
-        pg = alloc_domheap_page(NULL, memflags);
-        if ( !pg )
-            return 0;
+            if ( !pg )
+                return false;
 
-        unmap_domain_page(memset(__map_domain_page(pg), 0xcc, PAGE_SIZE));
+            unmap_domain_page(memset(__map_domain_page(pg), 0xcc, PAGE_SIZE));
+            addr = page_to_maddr(pg);
+        }
+        stubs[cpu] = addr;
     }
 
     stub_va = XEN_VIRT_END - FIXADDR_X_SIZE - (cpu + 1) * PAGE_SIZE;
-    if ( map_pages_to_xen(stub_va, page_to_mfn(pg), 1,
+    if ( map_pages_to_xen(stub_va, maddr_to_mfn(addr), 1,
                           PAGE_HYPERVISOR_RX | MAP_SMALL_PAGES) )
-    {
-        if ( !*mfn )
-            free_domheap_page(pg);
-        stub_va = 0;
-    }
-    else if ( !*mfn )
-        *mfn = mfn_x(page_to_mfn(pg));
+        return false;
+
+    per_cpu(stubs.mfn, cpu) = PFN_DOWN(addr);
+    per_cpu(stubs.addr, cpu) = stub_va + PAGE_OFFSET(addr);
+    return true;
+}
+
+void __init init_stub(void)
+{
+    const unsigned int num_cpus = num_present_cpus();
+    unsigned int i;
+
+    ASSERT(!stubs);
+    stubs = xvmalloc_array(typeof(*stubs), num_cpus);
+    if ( !stubs )
+        panic("Unable to allocate stub array\n");
 
-    return stub_va;
+    for ( i = 0; i < num_cpus; i++ )
+        stubs[i] = INVALID_PADDR;
+
+    if ( !assign_stub_page(0) )
+        panic("Unable to initialize BSP stub region\n");
 }
 
 void cpu_exit_clear(unsigned int cpu)
@@ -990,19 +1014,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;
+        memset(stub_page + PAGE_OFFSET(stubs[cpu]), 0xcc, STUB_BUF_SIZE);
         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) )
@@ -1041,10 +1058,9 @@ void *cpu_alloc_stack(unsigned int cpu)
 static int cpu_smpboot_alloc(unsigned int cpu)
 {
     struct cpu_info *info;
-    unsigned int i, memflags = 0;
+    unsigned int memflags = 0;
     nodeid_t node = cpu_to_node(cpu);
     seg_desc_t *gdt;
-    unsigned long stub_page;
     int rc = -ENOMEM;
 
     if ( node != NUMA_NO_NODE )
@@ -1092,18 +1108,8 @@ static int cpu_smpboot_alloc(unsigned int cpu)
     memcpy(per_cpu(idt, cpu), bsp_idt, sizeof(bsp_idt));
     disable_each_ist(per_cpu(idt, cpu));
 
-    for ( stub_page = 0, i = cpu & ~(STUBS_PER_PAGE - 1);
-          i < nr_cpu_ids && i <= (cpu | (STUBS_PER_PAGE - 1)); ++i )
-        if ( cpu_online(i) && cpu_to_node(i) == node )
-        {
-            per_cpu(stubs.mfn, cpu) = per_cpu(stubs.mfn, i);
-            break;
-        }
-    BUG_ON(i == cpu);
-    stub_page = alloc_stub_page(cpu, &per_cpu(stubs.mfn, cpu));
-    if ( !stub_page )
+    if ( !assign_stub_page(cpu) )
         goto out;
-    per_cpu(stubs.addr, cpu) = stub_page + STUB_BUF_CPU_OFFS(cpu);
 
     rc = setup_cpu_root_pgt(cpu);
     if ( rc )
-- 
2.53.0

Re: [PATCH for-4.22 v3] xen/x86: Change stub page allocation/free

[Jan Beulich]

On 12.06.2026 10:22, Roger Pau Monne wrote:
> From: Jason Andryuk <jason.andryuk@amd.com>
> 
> Today the inline tracking of the stub page is problematic.  0xcc is used to
> indicate unused, but it is also a "clear value."  A !CONFIG_PV build or
> when running with FRED support will not populate the LSTAR/CSTAR stubs at
> CPU bringup.  If a CPU is then offlined, the stubs page will be freed as
> its content will be all 0xcc, regardless of the stubs page still begin
> referenced by other CPUs.
> 
> The new approach uses a global, CPU-indexed dynamically allocated array of
> stub addresses.  However, to handle NUMA aware allocations, we cannot
> allocate all the memory in advance because of the NUMA dependency.  Take
> advantage of the fact that Xen will attempt to contiguously pack CPUs on
> the same NUMA node (see normalise_cpu_order()), and on CPU bringup use the
> same stubs page the previous CPU did if suitable.  Note the code would
> still function properly even if CPUs from NUMA nodes are not contiguously
> packed, it just consumes more memory.
> 
> stub pages are no longer freed.  They remain referenced in the global

Nit: Didn't you mean to s/stub/Stub/ as per Andrew's comment?

> CPU-indexed array and are re-used if the CPU is re-onlined.
> 
> The stubs array doesn't have an explicit lock.  During boot it's accessed
> single threaded.  During runtime, &cpu_add_remove_lock serializes access.
> 
> Fixes: 7a66ac8d1633 ("x86: move syscall trampolines off the stack")
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Tested-by: Jason Andryuk <jason.andryuk@amd.com>
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [PATCH for-4.22 v3] xen/x86: Change stub page allocation/free

[Roger Pau Monné]

On Fri, Jun 12, 2026 at 11:27:38AM +0200, Jan Beulich wrote:
> On 12.06.2026 10:22, Roger Pau Monne wrote:
> > From: Jason Andryuk <jason.andryuk@amd.com>
> > 
> > Today the inline tracking of the stub page is problematic.  0xcc is used to
> > indicate unused, but it is also a "clear value."  A !CONFIG_PV build or
> > when running with FRED support will not populate the LSTAR/CSTAR stubs at
> > CPU bringup.  If a CPU is then offlined, the stubs page will be freed as
> > its content will be all 0xcc, regardless of the stubs page still begin
> > referenced by other CPUs.
> > 
> > The new approach uses a global, CPU-indexed dynamically allocated array of
> > stub addresses.  However, to handle NUMA aware allocations, we cannot
> > allocate all the memory in advance because of the NUMA dependency.  Take
> > advantage of the fact that Xen will attempt to contiguously pack CPUs on
> > the same NUMA node (see normalise_cpu_order()), and on CPU bringup use the
> > same stubs page the previous CPU did if suitable.  Note the code would
> > still function properly even if CPUs from NUMA nodes are not contiguously
> > packed, it just consumes more memory.
> > 
> > stub pages are no longer freed.  They remain referenced in the global
> 
> Nit: Didn't you mean to s/stub/Stub/ as per Andrew's comment?

Hm, yes, I've fixed the instance below, but not the one here.

> > CPU-indexed array and are re-used if the CPU is re-onlined.
> > 
> > The stubs array doesn't have an explicit lock.  During boot it's accessed
> > single threaded.  During runtime, &cpu_add_remove_lock serializes access.
> > 
> > Fixes: 7a66ac8d1633 ("x86: move syscall trampolines off the stack")
> > Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > Tested-by: Jason Andryuk <jason.andryuk@amd.com>
> > Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

I will run a CI pre-commit loop and push it today if it's all fine.

Roger.

Re: [PATCH for-4.22 v3] xen/x86: Change stub page allocation/free

[Andrew Cooper]

On 12/06/2026 9:22 am, Roger Pau Monne wrote:
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 4192edf635b6..0253d22c349d 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -2089,9 +2089,7 @@ void asmlinkage __init noreturn __start_xen(void)
>  
>      init_idle_domain();
>  
> -    this_cpu(stubs.addr) = alloc_stub_page(smp_processor_id(),
> -                                           &this_cpu(stubs).mfn);
> -    BUG_ON(!this_cpu(stubs.addr));
> +    init_stub();

While I hate to nitpick further, "stubs".  There are 4 per CPU, and I'm
reasonably sure we're consistently plural elsewhere.

Otherwise (and with Jan's grammar note in the commit message too),
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

One further thought, which would be for a followup patch if we decide to
do it.

We do now use UDB for bugframes.  It's also marginally better than INT3
because if we do ever end up finding ourselves executing it, because
we'll #UD on the first one, rather than try to brute force our way
through the #BPs printing a log message per instruction.

One thing I didn't check and I really need to is whether UDB has the
same speculation-halting properties that UD2 has.  It's not a guarantee,
given that the encoding is valid in other modes.

~Andrew

Re: [PATCH for-4.22 v3] xen/x86: Change stub page allocation/free

[Roger Pau Monné]

On Fri, Jun 12, 2026 at 10:35:59AM +0100, Andrew Cooper wrote:
> On 12/06/2026 9:22 am, Roger Pau Monne wrote:
> > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> > index 4192edf635b6..0253d22c349d 100644
> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -2089,9 +2089,7 @@ void asmlinkage __init noreturn __start_xen(void)
> >  
> >      init_idle_domain();
> >  
> > -    this_cpu(stubs.addr) = alloc_stub_page(smp_processor_id(),
> > -                                           &this_cpu(stubs).mfn);
> > -    BUG_ON(!this_cpu(stubs.addr));
> > +    init_stub();
> 
> While I hate to nitpick further, "stubs".  There are 4 per CPU, and I'm
> reasonably sure we're consistently plural elsewhere.

Funny you ask - I had it as "stubs" initially, but then realized the
existing function is named alloc_stub_page(), and hence switched back
to use "stub" as that seemed more inline with the existing usage.

I will adjust on commit.

> Otherwise (and with Jan's grammar note in the commit message too),
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thanks, yes, that's been adjusted in the commit message.

> One further thought, which would be for a followup patch if we decide to
> do it.
> 
> We do now use UDB for bugframes.  It's also marginally better than INT3
> because if we do ever end up finding ourselves executing it, because
> we'll #UD on the first one, rather than try to brute force our way
> through the #BPs printing a log message per instruction.
> 
> One thing I didn't check and I really need to is whether UDB has the
> same speculation-halting properties that UD2 has.  It's not a guarantee,
> given that the encoding is valid in other modes.

Hm, yes, let's see if I find some time to change it, or whether
someone else beats me to it :).

Roger.