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

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 11/26/25 4:19 PM, Jan Beulich wrote:

> On 26.11.2025 15:32, Oleksii Kurochko wrote:
> > --- 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
> >       
> There was an 'a' lost in the identifier.
>
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -799,6 +799,29 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
> >      return arch_sanitise_domain_config(config);
> >  }
> >  
> > +struct domain *alloc_domain_struct(void)
> > +{
> > +    struct domain *d;
> > +    unsigned int bits = 0;
> > +
> > +#ifdef arch_alloc_domin_struct_bits
> > +    bits = arch_alloc_domin_struct_bits();
> > +#endif
> >       
> Maybe
>
> #ifndef arch_alloc_domain_struct_bits
> # define arch_alloc_domain_struct_bits() 0
> #endif
>
> ahead of the use and then simply
>
>     unsigned int bits = arch_alloc_domain_struct_bits();
>
> ?
>
> > +    BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
> > +
> > +    d = alloc_xenheap_pages(0, MEMF_bits(bits));
> >       
> I'd go a little farther and allow the arch to specify all the memflags that
> are wanted. Then the hook name would also be less ambiguous, as "bits" can
> mean many things. Perhaps arch_alloc_domain_struct_memflags() or, since
> "memflags" kind of implies allocation, arch_domain_struct_memflags()?
>     
I also thought about returning memflags from the hook instead of bits. Lets
then return memflags.

I will apply all other comments too.

Thanks.

~ Oleksii