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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 11/26/25 9:35 PM, Grygorii Strashko wrote:

>       
>       
>       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? 
>     
Oh, I missed that it is inside PV header. It should be in asm/domain.h, thanks for noticing that!

~ Oleksii