Re: [PATCH v2] xen/domain: introduce generic functions for domain struct allocation and freeing

[Grygorii Strashko <grygorii_strashko@xxxxxxxx>]

On 26.11.25 16:32, Oleksii Kurochko wrote:
> From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
>
> 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_alloc_domain_struct_bits() for x86-specific
> allocation requirements.
>
> No functional change.
>
> Suggested-By: Jan Beulich <jbeulich@xxxxxxxx>
> [Introduce an arch-specific function instead of using a weak function]
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
> Changes in v2:
> - CI test results: 
> https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/2180836457
> - Introduce an arch-specific function to handle differences between arch-es
>    in domain structure allocation requirements, instead of relying on a weak
>    function.
> - Add Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>.
> - Move free_domain_struct() to common code.
> - Update the commit message.
> ---
>   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/pv/domain.h |  3 +++
>   xen/common/domain.c                  | 23 +++++++++++++++++++++++
>   6 files changed, 28 insertions(+), 50 deletions(-)
[...]

> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 3a21e035f4..34a02f089e 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_alloc_domain_struct_bits(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 bits;
>   }
>   
>   struct vcpu *alloc_vcpu_struct(const struct domain *d)
> diff --git a/xen/arch/x86/include/asm/pv/domain.h 
> b/xen/arch/x86/include/asm/pv/domain.h
> index 582d004051..6e65bc22fc 100644
> --- a/xen/arch/x86/include/asm/pv/domain.h
> +++ b/xen/arch/x86/include/asm/pv/domain.h
> @@ -18,6 +18,9 @@ extern int8_t opt_pv32;
>   # define opt_pv32 false
>   #endif
>   
> +unsigned int arch_alloc_domain_struct_bits(void);
> +#define arch_alloc_domin_struct_bits arch_alloc_domain_struct_bits
Is it intentionally placed in PV header x86/include/asm/pv/domain.h?

--
Best regards,
-grygorii