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

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

[Jason Andryuk]

This is a different approach to fixing the stub page handling that is
broken with !CONFIG_PV and smt=0.

Jason Andryuk (3):
  xen/x86: Remove unneeded stub_page setting
  xen/x86: Split out page_walk_mfn() helper
  xen/x86: Change stub page allocation/free logic

 xen/arch/x86/include/asm/mm.h |  1 +
 xen/arch/x86/smpboot.c        | 32 ++++++++++++++++++++------------
 xen/arch/x86/x86_64/mm.c      | 31 +++++++++++++++++++++----------
 3 files changed, 42 insertions(+), 22 deletions(-)

-- 
2.54.0

[PATCH v2 1/3] xen/x86: Remove unneeded stub_page setting

[Jason Andryuk]

stub_page is not used in the for loop, and it unconditionally set below
just outside the patch context with:
    stub_page = alloc_stub_page(cpu, &per_cpu(stubs.mfn, cpu));

Remove it.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
 xen/arch/x86/smpboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index ff05955bae..7241dba621 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -1091,7 +1091,7 @@ 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);
+    for ( 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 )
         {
-- 
2.54.0

[PATCH v2 2/3] xen/x86: Split out page_walk_mfn() helper

[Jason Andryuk]

do_page_walk() returns a mapping of the underlying address.  Split out
page_walk_mfn() which returns the mfn_ti, and have do_page_walk() use
that.

The new standalone page_walk_mfn() is generally useful for looking up the
MFN pointed to by an a virtual address.  It will be used in the x86 CPU
stub logic to determine if a stub page is already populated.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
 xen/arch/x86/include/asm/mm.h |  1 +
 xen/arch/x86/x86_64/mm.c      | 31 +++++++++++++++++++++----------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/include/asm/mm.h b/xen/arch/x86/include/asm/mm.h
index 06c20ab8de..6dc5115093 100644
--- a/xen/arch/x86/include/asm/mm.h
+++ b/xen/arch/x86/include/asm/mm.h
@@ -587,6 +587,7 @@ void audit_domains(void);
 void make_cr3(struct vcpu *v, mfn_t mfn);
 pagetable_t update_cr3(struct vcpu *v);
 int vcpu_destroy_pagetables(struct vcpu *v);
+mfn_t page_walk_mfn(unsigned long cr3, unsigned long addr);
 void *do_page_walk(struct vcpu *v, unsigned long addr);
 
 /* Allocator functions for Xen pagetables. */
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index 8eadab7933..d197dce9f8 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -59,29 +59,26 @@ extern unsigned int compat_machine_to_phys_mapping[];
 
 #endif /* CONFIG_PV32 */
 
-void *do_page_walk(struct vcpu *v, unsigned long addr)
+mfn_t page_walk_mfn(unsigned long cr3, unsigned long addr)
 {
-    unsigned long mfn = pagetable_get_pfn(v->arch.guest_table);
+    unsigned long mfn = cr3;
     l4_pgentry_t l4e, *l4t;
     l3_pgentry_t l3e, *l3t;
     l2_pgentry_t l2e, *l2t;
     l1_pgentry_t l1e, *l1t;
 
-    if ( !is_pv_vcpu(v) || !is_canonical_address(addr) )
-        return NULL;
-
     l4t = map_domain_page(_mfn(mfn));
     l4e = l4t[l4_table_offset(addr)];
     unmap_domain_page(l4t);
     if ( !(l4e_get_flags(l4e) & _PAGE_PRESENT) )
-        return NULL;
+        return INVALID_MFN;
 
     l3t = map_l3t_from_l4e(l4e);
     l3e = l3t[l3_table_offset(addr)];
     unmap_domain_page(l3t);
     mfn = l3e_get_pfn(l3e);
     if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) || !mfn_valid(_mfn(mfn)) )
-        return NULL;
+        return INVALID_MFN;
     if ( (l3e_get_flags(l3e) & _PAGE_PSE) )
     {
         mfn += PFN_DOWN(addr & ((1UL << L3_PAGETABLE_SHIFT) - 1));
@@ -93,7 +90,7 @@ void *do_page_walk(struct vcpu *v, unsigned long addr)
     unmap_domain_page(l2t);
     mfn = l2e_get_pfn(l2e);
     if ( !(l2e_get_flags(l2e) & _PAGE_PRESENT) || !mfn_valid(_mfn(mfn)) )
-        return NULL;
+        return INVALID_MFN;
     if ( (l2e_get_flags(l2e) & _PAGE_PSE) )
     {
         mfn += PFN_DOWN(addr & ((1UL << L2_PAGETABLE_SHIFT) - 1));
@@ -105,10 +102,24 @@ void *do_page_walk(struct vcpu *v, unsigned long addr)
     unmap_domain_page(l1t);
     mfn = l1e_get_pfn(l1e);
     if ( !(l1e_get_flags(l1e) & _PAGE_PRESENT) || !mfn_valid(_mfn(mfn)) )
-        return NULL;
+        return INVALID_MFN;
 
  ret:
-    return map_domain_page(_mfn(mfn)) + (addr & ~PAGE_MASK);
+    return _mfn(mfn);
+}
+
+void *do_page_walk(struct vcpu *v, unsigned long addr)
+{
+    mfn_t mfn;
+
+    if ( !is_pv_vcpu(v) || !is_canonical_address(addr) )
+        return NULL;
+
+    mfn = page_walk_mfn(pagetable_get_pfn(v->arch.guest_table), addr);
+    if ( !mfn_valid(mfn) )
+        return NULL;
+
+    return map_domain_page(mfn) + (addr & ~PAGE_MASK);
 }
 
 /*
-- 
2.54.0

[PATCH v2 3/3] xen/x86: Change stub page allocation/free logic

[Jason Andryuk]

Today the inine tracking of the stub page is problematic.  0xcc is used
to indicate unused, but it is also a "clear value."  A !CONFIG_PV build
with smt=0 will bring up CPU0, bring up CPU1, bring down CPU1, and free
the in-use stub page.  Subsequent CPU onlining can write to the re-used
page.

Each stub page accomodates 32 stub regions, and each CPU uses an offset
into its portion of the page.  Each CPU used a CPU-specific mapping of
the whole page.  The virtual address of the CPU-specific mapping is
fixed, so it can be used to track the stub page.

Remove the actual free-ing from cpu_smpboot_free().  Use the stub_va PTE
to track the underlying page.  destroy_xen_mapping() would clear the
mapping, so replace it with modify_xen_mappings() to retain the PFN in
the PTE (with NX set).

In alloc_stub_page(), check for a valid PFN in the stub_va PTE.  When
found, it will be used.  This handles re-onlining a CPU.  Otherwise the
existing logic is retained to use a passed in mfn or allocate one.
These paths handle to bringing up new CPUs.

If all CPUs for a stub page are offlined, the page will be dangling and
unusable.  But it will be re-used if CPUs are re-onlined.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
 xen/arch/x86/smpboot.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 7241dba621..11937175a9 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -647,11 +647,21 @@ unsigned long alloc_stub_page(unsigned int cpu, unsigned long *mfn)
 {
     unsigned long stub_va;
     struct page_info *pg;
+    mfn_t stub_mfn;
 
     BUILD_BUG_ON(STUBS_PER_PAGE & (STUBS_PER_PAGE - 1));
 
-    if ( *mfn )
+    stub_va = XEN_VIRT_END - FIXADDR_X_SIZE - (cpu + 1) * PAGE_SIZE;
+    stub_mfn = page_walk_mfn(virt_to_mfn(idle_pg_table), stub_va);
+    if ( mfn_valid(stub_mfn) )
+    {
+        *mfn = mfn_x(stub_mfn);
+        pg = mfn_to_page(stub_mfn);
+    }
+    else if ( *mfn )
+    {
         pg = mfn_to_page(_mfn(*mfn));
+    }
     else
     {
         nodeid_t node = cpu_to_node(cpu);
@@ -664,7 +674,6 @@ unsigned long alloc_stub_page(unsigned int cpu, unsigned long *mfn)
         unmap_domain_page(memset(__map_domain_page(pg), 0xcc, PAGE_SIZE));
     }
 
-    stub_va = XEN_VIRT_END - FIXADDR_X_SIZE - (cpu + 1) * PAGE_SIZE;
     if ( map_pages_to_xen(stub_va, page_to_mfn(pg), 1,
                           PAGE_HYPERVISOR_RX | MAP_SMALL_PAGES) )
     {
@@ -990,19 +999,18 @@ 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);
+        /*
+         * destroy_xen_mappings() clears the PFN from the PTE, but we want to
+         * keep it for potential reuse if re-onlined.  Pass _PAGE_PRESENT to
+         * retain the PFN.
+         */
+        modify_xen_mappings(per_cpu(stubs.addr, cpu) & PAGE_MASK,
+                            (per_cpu(stubs.addr, cpu) | ~PAGE_MASK) + 1,
+                            _PAGE_PRESENT | _PAGE_NX);
         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) )
-- 
2.54.0

Re: [PATCH v2 1/3] xen/x86: Remove unneeded stub_page setting

[Roger Pau Monné]

On Thu, Jun 04, 2026 at 07:18:35PM -0400, Jason Andryuk wrote:
> stub_page is not used in the for loop, and it unconditionally set below
> just outside the patch context with:
>     stub_page = alloc_stub_page(cpu, &per_cpu(stubs.mfn, cpu));
> 
> Remove it.
> 
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

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

[Roger Pau Monné]

You want to Cc Oleksii for those, as we would like to get them into
4.22.  I've added him to the cover letter so he is aware.

Regards, Roger.

On Thu, Jun 04, 2026 at 07:18:34PM -0400, Jason Andryuk wrote:
> This is a different approach to fixing the stub page handling that is
> broken with !CONFIG_PV and smt=0.
> 
> Jason Andryuk (3):
>   xen/x86: Remove unneeded stub_page setting
>   xen/x86: Split out page_walk_mfn() helper
>   xen/x86: Change stub page allocation/free logic
> 
>  xen/arch/x86/include/asm/mm.h |  1 +
>  xen/arch/x86/smpboot.c        | 32 ++++++++++++++++++++------------
>  xen/arch/x86/x86_64/mm.c      | 31 +++++++++++++++++++++----------
>  3 files changed, 42 insertions(+), 22 deletions(-)
> 
> -- 
> 2.54.0
> 

Re: [PATCH v2 3/3] xen/x86: Change stub page allocation/free logic

[Roger Pau Monné]

On Thu, Jun 04, 2026 at 07:18:37PM -0400, Jason Andryuk wrote:
> Today the inine tracking of the stub page is problematic.  0xcc is used
> to indicate unused, but it is also a "clear value."  A !CONFIG_PV build
> with smt=0 will bring up CPU0, bring up CPU1, bring down CPU1, and free
> the in-use stub page.  Subsequent CPU onlining can write to the re-used
> page.
> 
> Each stub page accomodates 32 stub regions, and each CPU uses an offset
> into its portion of the page.  Each CPU used a CPU-specific mapping of
> the whole page.  The virtual address of the CPU-specific mapping is
> fixed, so it can be used to track the stub page.
> 
> Remove the actual free-ing from cpu_smpboot_free().  Use the stub_va PTE
> to track the underlying page.  destroy_xen_mapping() would clear the
> mapping, so replace it with modify_xen_mappings() to retain the PFN in
> the PTE (with NX set).
> 
> In alloc_stub_page(), check for a valid PFN in the stub_va PTE.  When
> found, it will be used.  This handles re-onlining a CPU.  Otherwise the
> existing logic is retained to use a passed in mfn or allocate one.
> These paths handle to bringing up new CPUs.
> 
> If all CPUs for a stub page are offlined, the page will be dangling and
> unusable.  But it will be re-used if CPUs are re-onlined.

I think there's a corner case with this approach where Xen will end up
allocating multiple pages for a chunk of 32 CPUs: consider the first
CPU of a stub page is onlined and then offlined, and afterwards the
second CPU of the same chunk is onlined.  That second CPU would see
stubs.mfn as 0 from its sibling if the first CPU is not parked, and
hence allocate a new stubs page when there's already one allocated for
the chunk.

FWIW, I would take a way more simple approach and allocate all stub
pages (using possible cpus as the max number) in smp_prepare_cpus()
and store them in an array.  Then from cpu_smpboot_alloc() the
to-be-onlined AP can gets the page to use and map it.  We could also
remove stubs.mfn at that point, since the mfns will be immutable for
the lifetime of Xen, and stored in a global array.

> 
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>

You want to add a Fixes: tag above the SoB.

> ---
>  xen/arch/x86/smpboot.c | 30 +++++++++++++++++++-----------
>  1 file changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
> index 7241dba621..11937175a9 100644
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -647,11 +647,21 @@ unsigned long alloc_stub_page(unsigned int cpu, unsigned long *mfn)
>  {
>      unsigned long stub_va;
>      struct page_info *pg;
> +    mfn_t stub_mfn;
>  
>      BUILD_BUG_ON(STUBS_PER_PAGE & (STUBS_PER_PAGE - 1));
>  
> -    if ( *mfn )
> +    stub_va = XEN_VIRT_END - FIXADDR_X_SIZE - (cpu + 1) * PAGE_SIZE;
> +    stub_mfn = page_walk_mfn(virt_to_mfn(idle_pg_table), stub_va);
> +    if ( mfn_valid(stub_mfn) )
> +    {
> +        *mfn = mfn_x(stub_mfn);

Before overwriting what's possibly in mfn, we might want to sanity
check it matches the mfn recovered from the page-tables, ie:

ASSERT(!*mfn || *mfn == mfn_x(stub_mfn));

Thanks, Roger.