[PATCH v4 0/4] Move alloc/free_vcpu_struct() to common code

[PATCH v4 0/4] Move alloc/free_vcpu_struct() to common code

[Oleksii Kurochko]

As it was suggested in [1] it would be better to allocate one page for struct
vcpu for all arch-es. To do that it is needed to align Arm code to allocate
one page (as there is a case(when CONFIG_NEW_VGIC=y) when Arm64 will require
to allocate two pages). As a result, the following patches for Arm have been
introduced:
 - [PATCH v2 1/4] xen/arm: optimize the size of struct vcpu
 - [PATCH v2 2/4] xen/arm: drop MAX_PAGES_PER_VCPU

This patches are dependency for:
 - [PATCH v2 3/4] xen: move alloc/free_vcpu_struct() to common code

Also, as a part of this patch series another clean up is done which makes
{alloc,free}_domain_struct() static.

[1] https://lore.kernel.org/xen-devel/f8a9be3a-a0c6-496a-806f-40760dca5aee@suse.com/T/#m275dfcbdccef0461fa9a8acef072403f18091768

CI: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2230207975

---
Changes in v4:
 - Address the comments recieved for v3.
---
Changes in v3:
 - Come up with a different way to optimize struct vcpu for Arm.
 - Merge patches "[PATCH v2 2/4] xen/arm: drop MAX_PAGES_PER_VCPU]" and
   "[PATCH v2 4/4] xen/common: make {alloc,free}_domain_struct() static"
   together.
 - Clean up vcpu_vgic_free() a little bit.
---
Changes in v2:
 - Introduce new patches for Arm:
     - [PATCH v2 1/4] xen/arm: optimize the size of struct vcpu
     - [PATCH v2 2/4] xen/arm: drop MAX_PAGES_PER_VCPU
    to allocate one page for struct vcpu in common code for all the arch-es.
 - Introduce patch to clean up xen/domain.h a little bit:
     - [PATCH v2 4/4] xen/common: make {alloc,free}_domain_struct() static
 - Address the comments from v1:
     - [PATCH v2 3/4] xen: move alloc/free_vcpu_struct() to common code
---

Oleksii Kurochko (4):
  xen/arm: vcpu_vgic_free() updates
  xen/arm: optimize the size of struct vcpu
  xen: move alloc/free_vcpu_struct() to common code
  xen/common: make {alloc,free}_domain_struct() static

 xen/arch/arm/domain.c               | 32 ---------------
 xen/arch/arm/include/asm/new_vgic.h |  2 +-
 xen/arch/arm/include/asm/vgic.h     |  2 +-
 xen/arch/arm/vgic.c                 |  5 +--
 xen/arch/arm/vgic/vgic-init.c       |  9 ++++-
 xen/arch/ppc/stubs.c                | 10 -----
 xen/arch/riscv/stubs.c              | 10 -----
 xen/arch/x86/domain.c               | 24 -----------
 xen/arch/x86/include/asm/domain.h   | 12 ++++++
 xen/common/domain.c                 | 62 +++++++++++++++++++----------
 xen/include/xen/domain.h            |  8 ----
 11 files changed, 64 insertions(+), 112 deletions(-)

-- 
2.52.0

[PATCH v4 1/4] xen/arm: vcpu_vgic_free() updates

[Oleksii Kurochko]

Use XFREE() instead of xfree() so that vcpu_vgic_free() can be idempotent.
With XFREE(), vgic_vcpu->private_irqs is set to NULL, so calling
vcpu_vgic_free() a second time is not an issue.

Update the prototype of vcpu_vgic_free() to return void to satisfy MISRA
Rule 17.7, since the return value of vcpu_vgic_free() is not used by any
callers.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Change in v4:
 - Add Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>.
---
Change in v3:
 - New patch.
---
 xen/arch/arm/include/asm/vgic.h | 2 +-
 xen/arch/arm/vgic.c             | 5 ++---
 xen/arch/arm/vgic/vgic-init.c   | 4 +---
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/include/asm/vgic.h b/xen/arch/arm/include/asm/vgic.h
index 31b3d3e5ec5d..6f9ab1c98c1c 100644
--- a/xen/arch/arm/include/asm/vgic.h
+++ b/xen/arch/arm/include/asm/vgic.h
@@ -418,7 +418,7 @@ int domain_vgic_register(struct domain *d, unsigned int *mmio_count);
 int domain_vgic_init(struct domain *d, unsigned int nr_spis);
 void domain_vgic_free(struct domain *d);
 int vcpu_vgic_init(struct vcpu *v);
-int vcpu_vgic_free(struct vcpu *v);
+void vcpu_vgic_free(struct vcpu *v);
 
 void vgic_inject_irq(struct domain *d, struct vcpu *v, unsigned int virq,
                      bool level);
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 3ebdf9953f07..6647071ad4d7 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -390,10 +390,9 @@ int vcpu_vgic_init(struct vcpu *v)
     return 0;
 }
 
-int vcpu_vgic_free(struct vcpu *v)
+void vcpu_vgic_free(struct vcpu *v)
 {
-    xfree(v->arch.vgic.private_irqs);
-    return 0;
+    XFREE(v->arch.vgic.private_irqs);
 }
 
 struct vcpu *vgic_get_target_vcpu(struct vcpu *v, unsigned int virq)
diff --git a/xen/arch/arm/vgic/vgic-init.c b/xen/arch/arm/vgic/vgic-init.c
index f8d7d3a226d0..aef526f2e717 100644
--- a/xen/arch/arm/vgic/vgic-init.c
+++ b/xen/arch/arm/vgic/vgic-init.c
@@ -239,13 +239,11 @@ void domain_vgic_free(struct domain *d)
     dist->nr_spis = 0;
 }
 
-int vcpu_vgic_free(struct vcpu *v)
+void vcpu_vgic_free(struct vcpu *v)
 {
     struct vgic_cpu *vgic_cpu = &v->arch.vgic;
 
     INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
-
-    return 0;
 }
 
 /*
-- 
2.52.0

[PATCH v4 2/4] xen/arm: optimize the size of struct vcpu

[Oleksii Kurochko]

When CONFIG_NEW_VGIC=y and CONFIG_ARM_64=y, the size of struct vcpu
exceeds one page, which requires allocating two pages and led to the
introduction of MAX_PAGES_PER_VCPU.

To remove the need for MAX_PAGES_PER_VCPU in a follow-up patch, the vgic
member of NEW_VGIC's struct vgic_vcpu member private_irq is changed to a
pointer to struct vgic_irq.
As a result, the size of struct vcpu for Arm64 is reduced to 2176 bytes,
compared to 3840 bytes (without these changes and with CONFIG_ARM_64=y)
and 4736 bytes (without these changes and with both CONFIG_ARM_64=y and
CONFIG_NEW_VGIC=y).

Since the private_irqs member is now a pointer, vcpu_vgic_init() and
vcpu_vgic_free() are updated to allocate and free private_irqs instance.

Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Change in v4:
 - Add Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>.
---
Changes in v3:
 - Make private_irqs member as pointer to vgic_irq in struct vgic_cpu
   of new_vgic instead of vgic member of arch_vcpu.
---
Changes in v2:
 - New patch.
---
 xen/arch/arm/include/asm/new_vgic.h | 2 +-
 xen/arch/arm/vgic/vgic-init.c       | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/include/asm/new_vgic.h b/xen/arch/arm/include/asm/new_vgic.h
index 1e762138939f..6f7af0e02b2b 100644
--- a/xen/arch/arm/include/asm/new_vgic.h
+++ b/xen/arch/arm/include/asm/new_vgic.h
@@ -155,7 +155,7 @@ struct vgic_dist {
 };
 
 struct vgic_cpu {
-    struct vgic_irq private_irqs[VGIC_NR_PRIVATE_IRQS];
+    struct vgic_irq *private_irqs;
 
     struct list_head ap_list_head;
     spinlock_t ap_list_lock;    /* Protects the ap_list */
diff --git a/xen/arch/arm/vgic/vgic-init.c b/xen/arch/arm/vgic/vgic-init.c
index aef526f2e717..4eb49d922492 100644
--- a/xen/arch/arm/vgic/vgic-init.c
+++ b/xen/arch/arm/vgic/vgic-init.c
@@ -202,6 +202,11 @@ int vcpu_vgic_init(struct vcpu *v)
 {
     int ret = 0;
 
+    v->arch.vgic.private_irqs =
+        xzalloc_array(struct vgic_irq, VGIC_NR_PRIVATE_IRQS);
+    if ( !v->arch.vgic.private_irqs )
+        return -ENOMEM;
+
     vgic_vcpu_early_init(v);
 
     if ( gic_hw_version() == GIC_V2 )
@@ -244,6 +249,8 @@ void vcpu_vgic_free(struct vcpu *v)
     struct vgic_cpu *vgic_cpu = &v->arch.vgic;
 
     INIT_LIST_HEAD(&vgic_cpu->ap_list_head);
+
+    XFREE(v->arch.vgic.private_irqs);
 }
 
 /*
-- 
2.52.0

[PATCH v4 3/4] xen: move alloc/free_vcpu_struct() to common code

[Oleksii Kurochko]

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.

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.

Now that the size of struct vcpu for Arm64 is smaller than PAGE_SIZE,
MAX_PAGES_PER_VCPU is no longer needed and is removed.

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>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes in v4:
 - Move implementation of alloc_vcpu_struct() and free_vcpu_struct() ahead
   of vmtrace_free_buffer().
 - Add Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>.
 - Add Acked-by: Jan Beulich <jbeulich@suse.com>.
---
Changes in v3:
 - Make from function arch_vcpu_struct_memflags() a macros in asm/domain.h.
 - Drop forward declaration of arch_vcpu_struct_memflags() in asm/domain.h.
 - Update defintion of arch_vcpu_stuct_memflags() in alloc_vcpu_struct().
---
Changes in v2:
 - Rework alloc/free_vcpu_struct() to work with only one page.
 - Return back the comment about the restriction inside x86's
   arch_vcpu_struct_memflags().
 - Drop MAX_PAGES_PER_VCPU.
---
 xen/arch/arm/domain.c             | 32 -------------------------------
 xen/arch/ppc/stubs.c              | 10 ----------
 xen/arch/riscv/stubs.c            | 10 ----------
 xen/arch/x86/domain.c             | 24 -----------------------
 xen/arch/x86/include/asm/domain.h | 12 ++++++++++++
 xen/common/domain.c               | 20 +++++++++++++++++++
 xen/include/xen/domain.h          |  4 ----
 7 files changed, 32 insertions(+), 80 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 47973f99d935..507df807edb8 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/ppc/stubs.c b/xen/arch/ppc/stubs.c
index 9953ea1c6c08..f7f6e7ed97af 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 164fc091b28a..29bdb65afbdf 100644
--- a/xen/arch/riscv/stubs.c
+++ b/xen/arch/riscv/stubs.c
@@ -121,11 +121,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");
@@ -233,11 +228,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 7632d5e2d62d..c29a6b0decee 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -493,30 +493,6 @@ unsigned int arch_domain_struct_memflags(void)
     return MEMF_bits(bits);
 }
 
-struct vcpu *alloc_vcpu_struct(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);
-}
-
 /* Initialise various registers to their architectural INIT/RESET state. */
 void arch_vcpu_regs_init(struct vcpu *v)
 {
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index 16cd45cc32c0..effb23a23416 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -15,6 +15,18 @@
 unsigned int arch_domain_struct_memflags(void);
 #define arch_domain_struct_memflags arch_domain_struct_memflags
 
+/*
+ * 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).
+ */
+#define arch_vcpu_struct_memflags(d) ({                                 \
+    const struct domain *d_ = (d);                                      \
+                                                                        \
+    (is_hvm_domain(d_) && paging_mode_shadow(d_) ? MEMF_bits(32) : 0);  \
+})
+
 #define has_32bit_shinfo(d)    ((d)->arch.has_32bit_shinfo)
 
 /*
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 93c71bc766b0..655d9590f846 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -316,6 +316,26 @@ static void vcpu_info_reset(struct vcpu *v)
          : &dummy_vcpu_info);
 }
 
+static struct vcpu *alloc_vcpu_struct(const struct domain *d)
+{
+#ifndef arch_vcpu_struct_memflags
+# define arch_vcpu_struct_memflags(d) ((void)(d), 0)
+#endif
+    struct vcpu *v;
+
+    BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE);
+    v = alloc_xenheap_pages(0, arch_vcpu_struct_memflags(d));
+    if ( v )
+        clear_page(v);
+
+    return v;
+}
+
+static void free_vcpu_struct(struct vcpu *v)
+{
+    free_xenheap_page(v);
+}
+
 static void vmtrace_free_buffer(struct vcpu *v)
 {
     const struct domain *d = v->domain;
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 8aab05ae93c8..644f5ac3f293 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -70,10 +70,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

[PATCH v4 4/4] xen/common: make {alloc,free}_domain_struct() static

[Oleksii Kurochko]

As {alloc,free}_domain_struct() are used only within domain.c,
they can be declared static and their declarations removed
from xen/domain.h.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes in v4:
 - Move implementation of alloc_domain_struct() and free_domain_struct()
   ahead of alloc_vcpu_struct().
---
Changes in v3:
 - Move alloc_domain_struct() and free_domain_struct() to not have
   forward declaration.
 - Add Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>.
---
Changes in v2:
 - New patch.
---
 xen/common/domain.c      | 42 ++++++++++++++++++++--------------------
 xen/include/xen/domain.h |  4 ----
 2 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 655d9590f846..ed4b6175de0b 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -316,6 +316,27 @@ static void vcpu_info_reset(struct vcpu *v)
          : &dummy_vcpu_info);
 }
 
+static struct domain *alloc_domain_struct(void)
+{
+#ifndef arch_domain_struct_memflags
+# define arch_domain_struct_memflags() 0
+#endif
+
+    struct domain *d = alloc_xenheap_pages(0, arch_domain_struct_memflags());
+
+    BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
+
+    if ( d )
+        clear_page(d);
+
+    return d;
+}
+
+static void free_domain_struct(struct domain *d)
+{
+    free_xenheap_page(d);
+}
+
 static struct vcpu *alloc_vcpu_struct(const struct domain *d)
 {
 #ifndef arch_vcpu_struct_memflags
@@ -819,27 +840,6 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
     return arch_sanitise_domain_config(config);
 }
 
-struct domain *alloc_domain_struct(void)
-{
-#ifndef arch_domain_struct_memflags
-# define arch_domain_struct_memflags() 0
-#endif
-
-    struct domain *d = alloc_xenheap_pages(0, arch_domain_struct_memflags());
-
-    BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
-
-    if ( d )
-        clear_page(d);
-
-    return d;
-}
-
-void free_domain_struct(struct domain *d)
-{
-    free_xenheap_page(d);
-}
-
 struct domain *domain_create(domid_t domid,
                              struct xen_domctl_createdomain *config,
                              unsigned int flags)
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 644f5ac3f293..273717c31b3f 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -66,10 +66,6 @@ void domid_free(domid_t domid);
  * Arch-specifics.
  */
 
-/* Allocate/free a domain structure. */
-struct domain *alloc_domain_struct(void);
-void free_domain_struct(struct domain *d);
-
 /* Allocate/free a PIRQ structure. */
 #ifndef alloc_pirq_struct
 struct pirq *alloc_pirq_struct(struct domain *d);
-- 
2.52.0

Re: [PATCH v4 1/4] xen/arm: vcpu_vgic_free() updates

[Orzel, Michal]

On 23/12/2025 18:01, Oleksii Kurochko wrote:
> Use XFREE() instead of xfree() so that vcpu_vgic_free() can be idempotent.
> With XFREE(), vgic_vcpu->private_irqs is set to NULL, so calling
> vcpu_vgic_free() a second time is not an issue.
Usually it would be beneficial to back such statement with an example of a
situation where this is or will be the issue.

> 
> Update the prototype of vcpu_vgic_free() to return void to satisfy MISRA
> Rule 17.7, since the return value of vcpu_vgic_free() is not used by any
> callers.
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Re: [PATCH v4 2/4] xen/arm: optimize the size of struct vcpu

[Orzel, Michal]

On 23/12/2025 18:01, Oleksii Kurochko wrote:
> When CONFIG_NEW_VGIC=y and CONFIG_ARM_64=y, the size of struct vcpu
> exceeds one page, which requires allocating two pages and led to the
> introduction of MAX_PAGES_PER_VCPU.
> 
> To remove the need for MAX_PAGES_PER_VCPU in a follow-up patch, the vgic
> member of NEW_VGIC's struct vgic_vcpu member private_irq is changed to a
s/vgic_vcpu/vgic_cpu/
s/private_irq/private_irqs/

> pointer to struct vgic_irq.
> As a result, the size of struct vcpu for Arm64 is reduced to 2176 bytes,
> compared to 3840 bytes (without these changes and with CONFIG_ARM_64=y)
> and 4736 bytes (without these changes and with both CONFIG_ARM_64=y and
> CONFIG_NEW_VGIC=y).
You only touch new vGIC, so there should be no size reduction without it but the
paragraph reads as if the change affected both old and new vGIC. Also I would
mention that probably you provided the numbers based on a defconfig target.

> 
> Since the private_irqs member is now a pointer, vcpu_vgic_init() and
> vcpu_vgic_free() are updated to allocate and free private_irqs instance.
> 
> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Other than that:
Acked-by: Michal Orzel <michal.orzel@amd.com>

~Michal

Re: [PATCH v4 2/4] xen/arm: optimize the size of struct vcpu

[Orzel, Michal]

On 29/12/2025 12:08, Orzel, Michal wrote:
> 
> 
> On 23/12/2025 18:01, Oleksii Kurochko wrote:
>> When CONFIG_NEW_VGIC=y and CONFIG_ARM_64=y, the size of struct vcpu
>> exceeds one page, which requires allocating two pages and led to the
>> introduction of MAX_PAGES_PER_VCPU.
Also, I think it would be better to drop MAX_PAGES_PER_VCPU in this patch.

~Michal

>>
>> To remove the need for MAX_PAGES_PER_VCPU in a follow-up patch, the vgic
>> member of NEW_VGIC's struct vgic_vcpu member private_irq is changed to a
> s/vgic_vcpu/vgic_cpu/
> s/private_irq/private_irqs/
> 
>> pointer to struct vgic_irq.
>> As a result, the size of struct vcpu for Arm64 is reduced to 2176 bytes,
>> compared to 3840 bytes (without these changes and with CONFIG_ARM_64=y)
>> and 4736 bytes (without these changes and with both CONFIG_ARM_64=y and
>> CONFIG_NEW_VGIC=y).
> You only touch new vGIC, so there should be no size reduction without it but the
> paragraph reads as if the change affected both old and new vGIC. Also I would
> mention that probably you provided the numbers based on a defconfig target.
> 
>>
>> Since the private_irqs member is now a pointer, vcpu_vgic_init() and
>> vcpu_vgic_free() are updated to allocate and free private_irqs instance.
>>
>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Other than that:
> Acked-by: Michal Orzel <michal.orzel@amd.com>
> 
> ~Michal
> 
> 

Re: [PATCH v4 2/4] xen/arm: optimize the size of struct vcpu

[Andrew Cooper]

On 29/12/2025 11:10 am, Orzel, Michal wrote:
>
> On 29/12/2025 12:08, Orzel, Michal wrote:
>>
>> On 23/12/2025 18:01, Oleksii Kurochko wrote:
>>> When CONFIG_NEW_VGIC=y and CONFIG_ARM_64=y, the size of struct vcpu
>>> exceeds one page, which requires allocating two pages and led to the
>>> introduction of MAX_PAGES_PER_VCPU.
> Also, I think it would be better to drop MAX_PAGES_PER_VCPU in this patch.

Or at least shrink it to just 1, which would be minimal churn.

>
> ~Michal
>
>>> To remove the need for MAX_PAGES_PER_VCPU in a follow-up patch, the vgic
>>> member of NEW_VGIC's struct vgic_vcpu member private_irq is changed to a
>> s/vgic_vcpu/vgic_cpu/
>> s/private_irq/private_irqs/
>>
>>> pointer to struct vgic_irq.
>>> As a result, the size of struct vcpu for Arm64 is reduced to 2176 bytes,
>>> compared to 3840 bytes (without these changes and with CONFIG_ARM_64=y)
>>> and 4736 bytes (without these changes and with both CONFIG_ARM_64=y and
>>> CONFIG_NEW_VGIC=y).
>> You only touch new vGIC, so there should be no size reduction without it but the
>> paragraph reads as if the change affected both old and new vGIC. Also I would
>> mention that probably you provided the numbers based on a defconfig target.

I think that was stale from v1, where this patch was far larger and more
invasive.

~Andrew

Re: [PATCH v4 1/4] xen/arm: vcpu_vgic_free() updates

[Oleksii Kurochko]

On 12/29/25 11:50 AM, Orzel, Michal wrote:
>
> On 23/12/2025 18:01, Oleksii Kurochko wrote:
>> Use XFREE() instead of xfree() so that vcpu_vgic_free() can be idempotent.
>> With XFREE(), vgic_vcpu->private_irqs is set to NULL, so calling
>> vcpu_vgic_free() a second time is not an issue.
> Usually it would be beneficial to back such statement with an example of a
> situation where this is or will be the issue.

I think there is not a case now in current code base, but just common practice
to have such type of functions to be idempotent.

>
>> Update the prototype of vcpu_vgic_free() to return void to satisfy MISRA
>> Rule 17.7, since the return value of vcpu_vgic_free() is not used by any
>> callers.
>>
>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Michal Orzel <michal.orzel@amd.com>

Thanks.

~ Oleksii

Re: [PATCH v4 2/4] xen/arm: optimize the size of struct vcpu

[Oleksii Kurochko]

On 12/29/25 12:08 PM, Orzel, Michal wrote:
>
> On 23/12/2025 18:01, Oleksii Kurochko wrote:
>> When CONFIG_NEW_VGIC=y and CONFIG_ARM_64=y, the size of struct vcpu
>> exceeds one page, which requires allocating two pages and led to the
>> introduction of MAX_PAGES_PER_VCPU.
>>
>> To remove the need for MAX_PAGES_PER_VCPU in a follow-up patch, the vgic
>> member of NEW_VGIC's struct vgic_vcpu member private_irq is changed to a
> s/vgic_vcpu/vgic_cpu/
> s/private_irq/private_irqs/
>
>> pointer to struct vgic_irq.
>> As a result, the size of struct vcpu for Arm64 is reduced to 2176 bytes,
>> compared to 3840 bytes (without these changes and with CONFIG_ARM_64=y)
>> and 4736 bytes (without these changes and with both CONFIG_ARM_64=y and
>> CONFIG_NEW_VGIC=y).
> You only touch new vGIC, so there should be no size reduction without it but the
> paragraph reads as if the change affected both old and new vGIC. Also I would
> mention that probably you provided the numbers based on a defconfig target.

   Yes, all the numbers are provided based on defconfig target.
   I will update this paragraph in the following way to be more clear:
   As a result, the size of struct vcpu for Arm64 is reduced to 2176 bytes
   in the case when CONFIG_ARM_64=y and CONFIG_NEW_VGIC=y, compared to 3840
   bytes (without these changes and with CONFIG_ARM_64=y) and 4736 bytes
   (without these changes and with both CONFIG_ARM_64=y and CONFIG_NEW_VGIC=y).
   Note that all numbers are based on defconfig with the mentioned options
   enabled or disabled as specified.

>
>> Since the private_irqs member is now a pointer, vcpu_vgic_init() and
>> vcpu_vgic_free() are updated to allocate and free private_irqs instance.
>>
>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Other than that:
> Acked-by: Michal Orzel <michal.orzel@amd.com>

Thanks!

~ Oleksii

Re: [PATCH v4 2/4] xen/arm: optimize the size of struct vcpu

[Oleksii Kurochko]

On 12/29/25 12:10 PM, Orzel, Michal wrote:
>
> On 29/12/2025 12:08, Orzel, Michal wrote:
>>
>> On 23/12/2025 18:01, Oleksii Kurochko wrote:
>>> When CONFIG_NEW_VGIC=y and CONFIG_ARM_64=y, the size of struct vcpu
>>> exceeds one page, which requires allocating two pages and led to the
>>> introduction of MAX_PAGES_PER_VCPU.
> Also, I think it would be better to drop MAX_PAGES_PER_VCPU in this patch.

Then I'll update alloc_vcpu_struct() and free_vcpu_struct() to:
  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);
+    BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE);
+    v = alloc_xenheap_pages(0, 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);
-    }
+        clear_page(v);
  
      return v;
@@ -503,5 +488,5 @@ struct vcpu *alloc_vcpu_struct(const struct domain *d)
  void free_vcpu_struct(struct vcpu *v)
  {
-    free_xenheap_pages(v, get_order_from_bytes(sizeof(*v)));
+    free_xenheap_page(v);
  }

Thanks.

~ Oleksii

Re: [PATCH v4 2/4] xen/arm: optimize the size of struct vcpu

[Jan Beulich]

On 31.12.2025 09:27, Oleksii Kurochko wrote:
> On 12/29/25 12:10 PM, Orzel, Michal wrote:
>> On 29/12/2025 12:08, Orzel, Michal wrote:
>>> On 23/12/2025 18:01, Oleksii Kurochko wrote:
>>>> When CONFIG_NEW_VGIC=y and CONFIG_ARM_64=y, the size of struct vcpu
>>>> exceeds one page, which requires allocating two pages and led to the
>>>> introduction of MAX_PAGES_PER_VCPU.
>> Also, I think it would be better to drop MAX_PAGES_PER_VCPU in this patch.
> 
> Then I'll update alloc_vcpu_struct() and free_vcpu_struct() to:
>   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);
> +    BUILD_BUG_ON(sizeof(*v) > PAGE_SIZE);
> +    v = alloc_xenheap_pages(0, 0);

Just one nit here: As (iirc) previously indicated by Andrew, please
avoid open-coding of alloc_xenheap_page().

Jan