alloc_vcpu_struct() and free_vcpu_struct() contain little
architecture-specific logic and are suitable for sharing across
architectures. Move both helpers to common code.

To support the remaining architectural differences, introduce
arch_vcpu_struct_memflags(), allowing architectures to override the
memory flags passed to alloc_xenheap_pages(). This is currently needed
by x86, which may require MEMF_bits(32) for HVM guests using shadow
paging.

Move the definition of MAX_PAGES_PER_VCPU to xen/domain.h and default
it to 1. Retain the ARM64 exception (with CONFIG_NEW_VGIC) where two
pages are required due to larger per-IRQ structures.

The ARM implementation of alloc/free_vcpu_struct() is removed and
replaced by the common version. Stub implementations are also dropped
from PPC and RISC-V.

Finally, make alloc_vcpu_struct() and free_vcpu_struct() static to
common/domain.c, as they are no longer used outside common code.

No functional changes.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
CI tests: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2219693055

Shouldn't we make alloc_domain_struct() and free_domain_struct() static and
drop their declarations from xen/domain.h, since these functions are only
used in common/domain.c?
---
 xen/arch/arm/domain.c             | 32 -------------------------------
 xen/arch/arm/include/asm/domain.h |  8 ++++++++
 xen/arch/ppc/stubs.c              | 10 ----------
 xen/arch/riscv/stubs.c            | 10 ----------
 xen/arch/x86/domain.c             | 23 ++--------------------
 xen/arch/x86/include/asm/domain.h |  3 +++
 xen/common/domain.c               | 27 ++++++++++++++++++++++++++
 xen/include/xen/domain.h          |  8 ++++----
 8 files changed, 44 insertions(+), 77 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 47973f99d9..507df807ed 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -473,38 +473,6 @@ void dump_pageframe_info(struct domain *d)
 
 }
 
-/*
- * The new VGIC has a bigger per-IRQ structure, so we need more than one
- * page on ARM64. Cowardly increase the limit in this case.
- */
-#if defined(CONFIG_NEW_VGIC) && defined(CONFIG_ARM_64)
-#define MAX_PAGES_PER_VCPU  2
-#else
-#define MAX_PAGES_PER_VCPU  1
-#endif
-
-struct vcpu *alloc_vcpu_struct(const struct domain *d)
-{
-    struct vcpu *v;
-
-    BUILD_BUG_ON(sizeof(*v) > MAX_PAGES_PER_VCPU * PAGE_SIZE);
-    v = alloc_xenheap_pages(get_order_from_bytes(sizeof(*v)), 0);
-    if ( v != NULL )
-    {
-        unsigned int i;
-
-        for ( i = 0; i < DIV_ROUND_UP(sizeof(*v), PAGE_SIZE); i++ )
-            clear_page((void *)v + i * PAGE_SIZE);
-    }
-
-    return v;
-}
-
-void free_vcpu_struct(struct vcpu *v)
-{
-    free_xenheap_pages(v, get_order_from_bytes(sizeof(*v)));
-}
-
 int arch_vcpu_create(struct vcpu *v)
 {
     int rc = 0;
diff --git a/xen/arch/arm/include/asm/domain.h b/xen/arch/arm/include/asm/domain.h
index 758ad807e4..f33d886bb8 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -12,6 +12,14 @@
 #include <asm/vpl011.h>
 #include <public/hvm/params.h>
 
+/*
+ * The new VGIC has a bigger per-IRQ structure, so we need more than one
+ * page on ARM64. Cowardly increase the limit in this case.
+ */
+#if defined(CONFIG_NEW_VGIC) && defined(CONFIG_ARM_64)
+#define MAX_PAGES_PER_VCPU  2
+#endif
+
 struct hvm_domain
 {
     uint64_t              params[HVM_NR_PARAMS];
diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
index 9953ea1c6c..f7f6e7ed97 100644
--- a/xen/arch/ppc/stubs.c
+++ b/xen/arch/ppc/stubs.c
@@ -152,11 +152,6 @@ void dump_pageframe_info(struct domain *d)
     BUG_ON("unimplemented");
 }
 
-void free_vcpu_struct(struct vcpu *v)
-{
-    BUG_ON("unimplemented");
-}
-
 int arch_vcpu_create(struct vcpu *v)
 {
     BUG_ON("unimplemented");
@@ -264,11 +259,6 @@ void vcpu_kick(struct vcpu *v)
     BUG_ON("unimplemented");
 }
 
-struct vcpu *alloc_vcpu_struct(const struct domain *d)
-{
-    BUG_ON("unimplemented");
-}
-
 unsigned long
 hypercall_create_continuation(unsigned int op, const char *format, ...)
 {
diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
index fe7d85ee1d..579c4215c8 100644
--- a/xen/arch/riscv/stubs.c
+++ b/xen/arch/riscv/stubs.c
@@ -126,11 +126,6 @@ void dump_pageframe_info(struct domain *d)
     BUG_ON("unimplemented");
 }
 
-void free_vcpu_struct(struct vcpu *v)
-{
-    BUG_ON("unimplemented");
-}
-
 int arch_vcpu_create(struct vcpu *v)
 {
     BUG_ON("unimplemented");
@@ -238,11 +233,6 @@ void vcpu_kick(struct vcpu *v)
     BUG_ON("unimplemented");
 }
 
-struct vcpu *alloc_vcpu_struct(const struct domain *d)
-{
-    BUG_ON("unimplemented");
-}
-
 unsigned long
 hypercall_create_continuation(unsigned int op, const char *format, ...)
 {
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 7632d5e2d6..0e3f7de524 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -493,28 +493,9 @@ unsigned int arch_domain_struct_memflags(void)
     return MEMF_bits(bits);
 }
 
-struct vcpu *alloc_vcpu_struct(const struct domain *d)
+unsigned int arch_vcpu_struct_memflags(const struct domain *d)
 {
-    struct vcpu *v;
-    /*
-     * This structure contains embedded PAE PDPTEs, used when an HVM guest
-     * runs on shadow pagetables outside of 64-bit mode. In this case the CPU
-     * may require that the shadow CR3 points below 4GB, and hence the whole
-     * structure must satisfy this restriction. Thus we specify MEMF_bits(32).
-     */
-    unsigned int memflags =
-        (is_hvm_domain(d) && paging_mode_shadow(d)) ? MEMF_bits(32) : 0;
-
-    BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE);
-    v = alloc_xenheap_pages(0, memflags);
-    if ( v != NULL )
-        clear_page(v);
-    return v;
-}
-
-void free_vcpu_struct(struct vcpu *v)
-{
-    free_xenheap_page(v);
+    return (is_hvm_domain(d) && paging_mode_shadow(d)) ? MEMF_bits(32) : 0;
 }
 
 /* Initialise various registers to their architectural INIT/RESET state. */
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index 7e5cbd11a4..576f9202b4 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -15,6 +15,9 @@
 unsigned int arch_domain_struct_memflags(void);
 #define arch_domain_struct_memflags arch_domain_struct_memflags
 
+unsigned int arch_vcpu_struct_memflags(const struct domain *d);
+#define arch_vcpu_struct_memflags arch_vcpu_struct_memflags
+
 #define has_32bit_shinfo(d)    ((d)->arch.has_32bit_shinfo)
 
 /*
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 93c71bc766..3982d9e466 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -13,6 +13,7 @@
 #include <xen/sched.h>
 #include <xen/sections.h>
 #include <xen/domain.h>
+#include <xen/macros.h>
 #include <xen/mm.h>
 #include <xen/event.h>
 #include <xen/vm_event.h>
@@ -392,6 +393,32 @@ static int vcpu_teardown(struct vcpu *v)
     return 0;
 }
 
+static struct vcpu *alloc_vcpu_struct(const struct domain *d)
+{
+#ifndef arch_vcpu_struct_memflags
+# define arch_vcpu_struct_memflags(d) 0
+#endif
+    struct vcpu *v;
+    unsigned int order = get_order_from_bytes(sizeof(*v));
+
+    BUILD_BUG_ON(sizeof(*v) > MAX_PAGES_PER_VCPU * PAGE_SIZE);
+    v = alloc_xenheap_pages(order, arch_vcpu_struct_memflags(d));
+    if ( v )
+    {
+        unsigned int i;
+
+        for ( i = 0; i < DIV_ROUND_UP(sizeof(*v), PAGE_SIZE); i++ )
+            clear_page((void *)v + i * PAGE_SIZE);
+    }
+
+    return v;
+}
+
+static void free_vcpu_struct(struct vcpu *v)
+{
+    free_xenheap_pages(v, get_order_from_bytes(sizeof(*v)));
+}
+
 /*
  * Destoy a vcpu once all references to it have been dropped.  Used either
  * from domain_destroy()'s RCU path, or from the vcpu_create() error path
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 8aab05ae93..3946ec9dad 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -15,6 +15,10 @@ struct guest_area {
 
 #include <asm/domain.h>
 
+#ifndef MAX_PAGES_PER_VCPU
+#define MAX_PAGES_PER_VCPU 1
+#endif
+
 typedef union {
     struct vcpu_guest_context *nat;
     struct compat_vcpu_guest_context *cmp;
@@ -70,10 +74,6 @@ void domid_free(domid_t domid);
 struct domain *alloc_domain_struct(void);
 void free_domain_struct(struct domain *d);
 
-/* Allocate/free a VCPU structure. */
-struct vcpu *alloc_vcpu_struct(const struct domain *d);
-void free_vcpu_struct(struct vcpu *v);
-
 /* Allocate/free a PIRQ structure. */
 #ifndef alloc_pirq_struct
 struct pirq *alloc_pirq_struct(struct domain *d);
-- 
2.52.0

On 17.12.2025 11:53, Oleksii Kurochko wrote:
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -15,6 +15,10 @@ struct guest_area {
>  
>  #include <asm/domain.h>
>  
> +#ifndef MAX_PAGES_PER_VCPU
> +#define MAX_PAGES_PER_VCPU 1
> +#endif

In addition to what Andrew has said, I'd like to mention that this is also
a pretty ambiguous name. It can really mean about anything vCPU-ish, when
it's specifically about struct vcpu. But as Andrew said, preferably we
would get away without such a constant altogether.

Jan

On 17/12/2025 10:53 am, Oleksii Kurochko wrote:
> alloc_vcpu_struct() and free_vcpu_struct() contain little
> architecture-specific logic and are suitable for sharing across
> architectures. Move both helpers to common code.
>
> To support the remaining architectural differences, introduce
> arch_vcpu_struct_memflags(), allowing architectures to override the
> memory flags passed to alloc_xenheap_pages(). This is currently needed
> by x86, which may require MEMF_bits(32) for HVM guests using shadow
> paging.

You lost the comment explaining the restriction.  This needs adding back.

> Move the definition of MAX_PAGES_PER_VCPU to xen/domain.h and default
> it to 1. Retain the ARM64 exception (with CONFIG_NEW_VGIC) where two
> pages are required due to larger per-IRQ structures.

CONFIG_NEW_VGIC is still off by default, unsupported, and has had no
work on it since it's introduction in 2018.

There are a lot of good reasons to enforce struct vcpu being a single
page allocation, not least because an allocation can fail due to
fragmentation despite there being enough free RAM.

I would far rather that common code enforced it being page size, and
NEW_VGIC gets deleted or adjusted to cope, than to make it this easy for
architectures to shoot themselves in the foot.

>
> The ARM implementation of alloc/free_vcpu_struct() is removed and
> replaced by the common version. Stub implementations are also dropped
> from PPC and RISC-V.
>
> Finally, make alloc_vcpu_struct() and free_vcpu_struct() static to
> common/domain.c, as they are no longer used outside common code.
>
> No functional changes.
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> CI tests: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2219693055
>
> Shouldn't we make alloc_domain_struct() and free_domain_struct() static and
> drop their declarations from xen/domain.h, since these functions are only
> used in common/domain.c?

Yes - they should become static.

~Andrew

On 12/17/25 12:46 PM, Andrew Cooper wrote:
> On 17/12/2025 10:53 am, Oleksii Kurochko wrote:
>> alloc_vcpu_struct() and free_vcpu_struct() contain little
>> architecture-specific logic and are suitable for sharing across
>> architectures. Move both helpers to common code.
>>
>> To support the remaining architectural differences, introduce
>> arch_vcpu_struct_memflags(), allowing architectures to override the
>> memory flags passed to alloc_xenheap_pages(). This is currently needed
>> by x86, which may require MEMF_bits(32) for HVM guests using shadow
>> paging.
> You lost the comment explaining the restriction.  This needs adding back.
>
>> Move the definition of MAX_PAGES_PER_VCPU to xen/domain.h and default
>> it to 1. Retain the ARM64 exception (with CONFIG_NEW_VGIC) where two
>> pages are required due to larger per-IRQ structures.
> CONFIG_NEW_VGIC is still off by default, unsupported, and has had no
> work on it since it's introduction in 2018.
>
> There are a lot of good reasons to enforce struct vcpu being a single
> page allocation, not least because an allocation can fail due to
> fragmentation despite there being enough free RAM.
>
> I would far rather that common code enforced it being page size, and
> NEW_VGIC gets deleted or adjusted to cope, than to make it this easy for
> architectures to shoot themselves in the foot.

I am not sure that everyone would agree to simply drop|NEW_VGIC|.

Based on the commit message ...:
     ARM: new VGIC: Allocate two pages for struct vcpu
     
     At the moment we allocate exactly one page for struct vcpu on ARM, also
     have a check in place to prevent it growing beyond 4KB.
     As the struct includes the state of all 32 private (per-VCPU) interrupts,
     we are at 3840 bytes on arm64 at the moment already. Growing the per-IRQ
     VGIC structure even slightly makes the VCPU quickly exceed the 4K limit.
     The new VGIC will need more space per virtual IRQ. I spent a few hours
     trying to trim this down, but couldn't get it below 4KB, even with the
     nasty hacks piling up to save some bytes here and there.
     It turns out that beyond efficiency, maybe, there is no real technical
     reason this struct has to fit in one page, so lifting the limit to two
     pages seems like the most pragmatic solution.
     Restrict the compilation error to compiling with the new VGIC and for
     ARM64 only.

... Given this, I initially thought it would be difficult to adjust.
However, it seems there might be enough to do the following ...:
@@ -238,7 +238,7 @@ struct arch_vcpu
      union gic_state_data gic;
      uint64_t lr_mask;
  
-    struct vgic_cpu vgic;
+    struct vgic_cpu *vgic;
  
      /* Timer registers  */
      register_t cntkctl;

... with|vgic| allocated in|vcpu_vgic_init()| and freed in|vcpu_vgic_free()|.

I only checked whether this is sufficient to build with ...:
    #if defined(CONFIG_NEW_VGIC) && defined(CONFIG_ARM_64)
    -#define MAX_PAGES_PER_VCPU  2
    +#define MAX_PAGES_PER_VCPU  1
    #endif
   
   $ grep -nE "^(CONFIG_NEW_VGIC|CONFIG_ARM_64)=y" xen/.config
   13:CONFIG_ARM_64=y
   28:CONFIG_NEW_VGIC=y
... and it seems to work.

The question to the Arm maintainers is whether you would be fine with such a
solution.

~ Oleksii


>
>> The ARM implementation of alloc/free_vcpu_struct() is removed and
>> replaced by the common version. Stub implementations are also dropped
>> from PPC and RISC-V.
>>
>> Finally, make alloc_vcpu_struct() and free_vcpu_struct() static to
>> common/domain.c, as they are no longer used outside common code.
>>
>> No functional changes.
>>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>>