From: Roger Pau Monne <roger.pau@citrix.com>

x86 has specific requirements about the allocation of the domain structure,
but that's not the case for ARM or likely other architectures.

Introduce a generic weak function that can be overwritten with an
architecture specific implementation if required.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes:
 - Add Reviewed-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>.
 - Add empty line before last return in alloc_domain_struct().
---
 xen/arch/arm/domain.c  | 12 ------------
 xen/arch/ppc/stubs.c   |  5 -----
 xen/arch/riscv/stubs.c |  5 -----
 xen/common/domain.c    | 14 ++++++++++++++
 4 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 45aeb8bddc..29588f869c 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -479,18 +479,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);
diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
index 671e71aa0a..d999d22718 100644
--- a/xen/arch/ppc/stubs.c
+++ b/xen/arch/ppc/stubs.c
@@ -294,11 +294,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 e396b67cd3..155e5a7f58 100644
--- a/xen/arch/riscv/stubs.c
+++ b/xen/arch/riscv/stubs.c
@@ -268,11 +268,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/common/domain.c b/xen/common/domain.c
index e566a18747..c134868e95 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -785,6 +785,20 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
     return arch_sanitise_domain_config(config);
 }
 
+struct domain *__weak alloc_domain_struct(void)
+{
+    struct domain *d = alloc_xenheap_pages(0, 0);
+
+    BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
+
+    if ( !d )
+        return NULL;
+
+    clear_page(d);
+
+    return d;
+}
+
 struct domain *domain_create(domid_t domid,
                              struct xen_domctl_createdomain *config,
                              unsigned int flags)
-- 
2.49.0

On 13/06/2025 4:56 pm, Oleksii Kurochko wrote:
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index e566a18747..c134868e95 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -785,6 +785,20 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>      return arch_sanitise_domain_config(config);
>  }
>  
> +struct domain *__weak alloc_domain_struct(void)
> +{
> +    struct domain *d = alloc_xenheap_pages(0, 0);

I know you're just moving what ARM has, but this is spelt
alloc_xenheap_page();

> +
> +    BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
> +
> +    if ( !d )
> +        return NULL;
> +
> +    clear_page(d);
> +
> +    return d;
> +}
> +

This looks fine, but you must do the same with free_domain_struct() at
the same time to keep the pair symmetric.  That in turn gets you an even
bigger negative diffstat, and x86 doesn't even need to override the
common version.

vCPU really wants doing at the same time, although you're going to run
into problems on ARM there with the BUILD_BUG_ON().

However, I suspect we want to be building with -ffunction-sections
generally active, or IIRC we'll still have the weak copy present in the
final image.

~Andrew

On 13.06.2025 20:53, Andrew Cooper wrote:
> On 13/06/2025 4:56 pm, Oleksii Kurochko wrote:
>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>> index e566a18747..c134868e95 100644
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -785,6 +785,20 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>>      return arch_sanitise_domain_config(config);
>>  }
>>  
>> +struct domain *__weak alloc_domain_struct(void)
>> +{
>> +    struct domain *d = alloc_xenheap_pages(0, 0);
> 
> I know you're just moving what ARM has, but this is spelt
> alloc_xenheap_page();
> 
>> +
>> +    BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
>> +
>> +    if ( !d )
>> +        return NULL;
>> +
>> +    clear_page(d);
>> +
>> +    return d;
>> +}
>> +
> 
> This looks fine, but you must do the same with free_domain_struct() at
> the same time to keep the pair symmetric.  That in turn gets you an even
> bigger negative diffstat, and x86 doesn't even need to override the
> common version.
> 
> vCPU really wants doing at the same time, although you're going to run
> into problems on ARM there with the BUILD_BUG_ON().
> 
> However, I suspect we want to be building with -ffunction-sections
> generally active, or IIRC we'll still have the weak copy present in the
> final image.

Instead of introducing a weak function in common code, why don't we move
the function there altogether, adding merely an arch hook to control the
flags to be passed into alloc_xenheap_pages()?

Jan