xen/arch/arm/domain.c | 17 ----------------- xen/arch/ppc/stubs.c | 10 ---------- xen/arch/riscv/stubs.c | 10 ---------- xen/arch/x86/domain.c | 15 ++------------- xen/arch/x86/include/asm/domain.h | 3 +++ xen/common/domain.c | 22 ++++++++++++++++++++++ 6 files changed, 27 insertions(+), 50 deletions(-)
From: Roger Pau Monne <roger.pau@citrix.com>
Move x86's free_domain_struct() to common code since it is shared between
architectures.
Move the x86 version of alloc_domain_struct() to common code as most of the
logic is architecture-independent. To handle the remaining architectural
differences, introduce arch_domain_struct_memflags() for x86-specific
allocation requirements.
No functional change.
Suggested-by: Jan Beulich <jbeulich@suse.com>
[Introduce an arch-specific function instead of using a weak function]
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v3:
- s/arch_alloc_domain_struct_bits/arch_domain_struct_memflags.
- Make x86's arch_domain_struct_memflags() to return MEMF_bits(bits) instead
of bits.
- Move "#define arch_domain_struct_memflags arch_domain_struct_memflags"
and declaration of arch_domain_struct_memflags() from x86/asm/pv/domain.h
to x86/domain.h.
- Update alloc_domain_struct() to work with arch_domain_struct_memflags().
- s/Suggested-By/Suggested-by.
- CI tests: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2182821111
---
Changes in v2:
- Introduce an arch-specific function to handle differences between arch-es
in domain structure allocation requirements, instead of relying on a weak
function.
- Move free_domain_struct() to common code.
- Add Suggested-by: Jan Beulich <jbeulich@suse.com>.
- Update the commit message.
- Link to original patch:
https://lore.kernel.org/xen-devel/c08595dd7940b44a1392e16d4a2035b95b5c580b.1749829230.git.oleksii.kurochko@gmail.com/
---
xen/arch/arm/domain.c | 17 -----------------
xen/arch/ppc/stubs.c | 10 ----------
xen/arch/riscv/stubs.c | 10 ----------
xen/arch/x86/domain.c | 15 ++-------------
xen/arch/x86/include/asm/domain.h | 3 +++
xen/common/domain.c | 22 ++++++++++++++++++++++
6 files changed, 27 insertions(+), 50 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index ab78444335..3e3a1fb9f5 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -480,23 +480,6 @@ void startup_cpu_idle_loop(void)
reset_stack_and_jump(idle_loop);
}
-struct domain *alloc_domain_struct(void)
-{
- struct domain *d;
- BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
- d = alloc_xenheap_pages(0, 0);
- if ( d == NULL )
- return NULL;
-
- clear_page(d);
- return d;
-}
-
-void free_domain_struct(struct domain *d)
-{
- free_xenheap_page(d);
-}
-
void dump_pageframe_info(struct domain *d)
{
diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
index 75ebcae5e2..9953ea1c6c 100644
--- a/xen/arch/ppc/stubs.c
+++ b/xen/arch/ppc/stubs.c
@@ -147,11 +147,6 @@ void startup_cpu_idle_loop(void)
BUG_ON("unimplemented");
}
-void free_domain_struct(struct domain *d)
-{
- BUG_ON("unimplemented");
-}
-
void dump_pageframe_info(struct domain *d)
{
BUG_ON("unimplemented");
@@ -269,11 +264,6 @@ void vcpu_kick(struct vcpu *v)
BUG_ON("unimplemented");
}
-struct domain *alloc_domain_struct(void)
-{
- BUG_ON("unimplemented");
-}
-
struct vcpu *alloc_vcpu_struct(const struct domain *d)
{
BUG_ON("unimplemented");
diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
index 340ed3cd6c..fe7d85ee1d 100644
--- a/xen/arch/riscv/stubs.c
+++ b/xen/arch/riscv/stubs.c
@@ -121,11 +121,6 @@ void startup_cpu_idle_loop(void)
BUG_ON("unimplemented");
}
-void free_domain_struct(struct domain *d)
-{
- BUG_ON("unimplemented");
-}
-
void dump_pageframe_info(struct domain *d)
{
BUG_ON("unimplemented");
@@ -243,11 +238,6 @@ void vcpu_kick(struct vcpu *v)
BUG_ON("unimplemented");
}
-struct domain *alloc_domain_struct(void)
-{
- BUG_ON("unimplemented");
-}
-
struct vcpu *alloc_vcpu_struct(const struct domain *d)
{
BUG_ON("unimplemented");
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 3a21e035f4..42643c8813 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -463,10 +463,8 @@ void domain_cpu_policy_changed(struct domain *d)
}
}
-struct domain *alloc_domain_struct(void)
+unsigned int arch_domain_struct_memflags(void)
{
- struct domain *d;
-
/*
* Without CONFIG_BIGMEM, we pack the PDX of the domain structure into
* a 32-bit field within the page_info structure. Hence the MEMF_bits()
@@ -492,16 +490,7 @@ struct domain *alloc_domain_struct(void)
- 1;
#endif
- BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
- d = alloc_xenheap_pages(0, MEMF_bits(bits));
- if ( d != NULL )
- clear_page(d);
- return d;
-}
-
-void free_domain_struct(struct domain *d)
-{
- free_xenheap_page(d);
+ return MEMF_bits(bits);
}
struct vcpu *alloc_vcpu_struct(const struct domain *d)
diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
index 5df8c78253..386ec61745 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -12,6 +12,9 @@
#include <public/vcpu.h>
#include <public/hvm/hvm_info_table.h>
+unsigned int arch_domain_struct_memflags(void);
+#define arch_domain_struct_memflags arch_domain_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 3b6e9471c4..2e8d74cbd9 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -799,6 +799,28 @@ 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;
+
+ BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
+
+ d = alloc_xenheap_pages(0, arch_domain_struct_memflags());
+ if ( d != NULL )
+ 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)
--
2.52.0
Hi
Sorry for the late comment.
On 27.11.25 18:26, Oleksii Kurochko wrote:
> From: Roger Pau Monne <roger.pau@citrix.com>
>
> Move x86's free_domain_struct() to common code since it is shared between
> architectures.
>
> Move the x86 version of alloc_domain_struct() to common code as most of the
> logic is architecture-independent. To handle the remaining architectural
> differences, introduce arch_domain_struct_memflags() for x86-specific
> allocation requirements.
>
> No functional change.
>
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> [Introduce an arch-specific function instead of using a weak function]
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> ---
> Changes in v3:
> - s/arch_alloc_domain_struct_bits/arch_domain_struct_memflags.
> - Make x86's arch_domain_struct_memflags() to return MEMF_bits(bits) instead
> of bits.
> - Move "#define arch_domain_struct_memflags arch_domain_struct_memflags"
> and declaration of arch_domain_struct_memflags() from x86/asm/pv/domain.h
> to x86/domain.h.
> - Update alloc_domain_struct() to work with arch_domain_struct_memflags().
> - s/Suggested-By/Suggested-by.
> - CI tests: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2182821111
> ---
> Changes in v2:
> - Introduce an arch-specific function to handle differences between arch-es
> in domain structure allocation requirements, instead of relying on a weak
> function.
> - Move free_domain_struct() to common code.
> - Add Suggested-by: Jan Beulich <jbeulich@suse.com>.
> - Update the commit message.
> - Link to original patch:
> https://lore.kernel.org/xen-devel/c08595dd7940b44a1392e16d4a2035b95b5c580b.1749829230.git.oleksii.kurochko@gmail.com/
> ---
> xen/arch/arm/domain.c | 17 -----------------
> xen/arch/ppc/stubs.c | 10 ----------
> xen/arch/riscv/stubs.c | 10 ----------
> xen/arch/x86/domain.c | 15 ++-------------
> xen/arch/x86/include/asm/domain.h | 3 +++
> xen/common/domain.c | 22 ++++++++++++++++++++++
> 6 files changed, 27 insertions(+), 50 deletions(-)
>
[...]
>
> struct vcpu *alloc_vcpu_struct(const struct domain *d)
> diff --git a/xen/arch/x86/include/asm/domain.h b/xen/arch/x86/include/asm/domain.h
> index 5df8c78253..386ec61745 100644
> --- a/xen/arch/x86/include/asm/domain.h
> +++ b/xen/arch/x86/include/asm/domain.h
> @@ -12,6 +12,9 @@
> #include <public/vcpu.h>
> #include <public/hvm/hvm_info_table.h>
>
> +unsigned int arch_domain_struct_memflags(void);
> +#define arch_domain_struct_memflags arch_domain_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 3b6e9471c4..2e8d74cbd9 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -799,6 +799,28 @@ 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
Is it really the right way to
hide part of common interface in common code instead of header?
> +
> + struct domain *d;
> +
> + BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
> +
> + d = alloc_xenheap_pages(0, arch_domain_struct_memflags());
> + if ( d != NULL )
> + 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)
--
Best regards,
-grygorii
On 28.11.2025 13:11, Grygorii Strashko wrote:
> On 27.11.25 18:26, Oleksii Kurochko wrote:
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -799,6 +799,28 @@ 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
>
> Is it really the right way to
> hide part of common interface in common code instead of header?
If it had multiple uses in different files, surely it should be put in a header.
In this case though, with (even long term) there being only a single use site, I
don't see why we should even bother figuring out which header would be an
appropriate place for it to live in.
Jan
On 28.11.25 15:19, Jan Beulich wrote:
> On 28.11.2025 13:11, Grygorii Strashko wrote:
>> On 27.11.25 18:26, Oleksii Kurochko wrote:
>>> --- a/xen/common/domain.c
>>> +++ b/xen/common/domain.c
>>> @@ -799,6 +799,28 @@ 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
>>
>> Is it really the right way to
>> hide part of common interface in common code instead of header?
>
> If it had multiple uses in different files, surely it should be put in a header.
> In this case though, with (even long term) there being only a single use site, I
> don't see why we should even bother figuring out which header would be an
> appropriate place for it to live in.
Thank you for clarification.
--
Best regards,
-grygorii
On 27.11.2025 17:26, Oleksii Kurochko wrote:
> From: Roger Pau Monne <roger.pau@citrix.com>
>
> Move x86's free_domain_struct() to common code since it is shared between
> architectures.
>
> Move the x86 version of alloc_domain_struct() to common code as most of the
> logic is architecture-independent. To handle the remaining architectural
> differences, introduce arch_domain_struct_memflags() for x86-specific
> allocation requirements.
>
> No functional change.
>
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> [Introduce an arch-specific function instead of using a weak function]
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
ideally with ...
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -799,6 +799,28 @@ 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;
> +
> + BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
> +
> + d = alloc_xenheap_pages(0, arch_domain_struct_memflags());
... this now becoming the initializer of the variable.
> + if ( d != NULL )
Personally I'd also prefer if the "!= NULL" was dropped at this occasion.
Jan
© 2016 - 2025 Red Hat, Inc.