Re: [PATCH v1 6/9] asm-generic: move some parts of Arm's domain_build.h to asm-generic header

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.01.2025 12:13, Oleksii Kurochko wrote:
> Nothing changed. Only some functions declaration are moved to asm-generic
> header as they are expected to be used by common code of domain builing or
> dom0less.
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
>  xen/arch/arm/include/asm/domain_build.h | 19 ++----------
>  xen/include/asm-generic/domain-build.h  | 41 +++++++++++++++++++++++++
>  2 files changed, 43 insertions(+), 17 deletions(-)
>  create mode 100644 xen/include/asm-generic/domain-build.h

Again I question this movement under this name. "Domain building" is a pretty
generic thing, yes, but what you move would e.g. be entirely inapplicable on
x86 (as it is now). For example ...

> --- /dev/null
> +++ b/xen/include/asm-generic/domain-build.h
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __ASM_GENERIC_DOMAIN_BUILD_H__
> +#define __ASM_GENERIC_DOMAIN_BUILD_H__
> +
> +#include <xen/types.h>
> +
> +struct domain;
> +struct page_info;
> +struct kernel_info;
> +struct membanks;
> +
> +typedef bool (*alloc_domheap_mem_cb)(struct domain *d, struct page_info *pg,
> +                                     unsigned int order, void *extra);
> +bool allocate_domheap_memory(struct domain *d, paddr_t tot_size,
> +                             alloc_domheap_mem_cb cb, void *extra);
> +
> +bool allocate_bank_memory(struct kernel_info *kinfo, gfn_t sgfn,
> +                          paddr_t tot_size);

... the term "bank" seems pretty closely tied to DT. Other stuff ...

> +void allocate_memory(struct domain *d, struct kernel_info *kinfo);
> +int construct_domain(struct domain *d, struct kernel_info *kinfo);
> +int make_chosen_node(const struct kernel_info *kinfo);
> +int make_cpus_node(const struct domain *d, void *fdt);
> +int make_hypervisor_node(struct domain *d, const struct kernel_info *kinfo,
> +                         int addrcells, int sizecells);
> +int make_memory_node(const struct kernel_info *kinfo, int addrcells,
> +                     int sizecells, const struct membanks *mem);
> +int make_timer_node(const struct kernel_info *kinfo);

... here also falls in this category. Stuff like this may well live
under asm-generic/, but the file name chosen then needs to reflect
constraints.

> +unsigned int get_allocation_size(paddr_t size);
> +
> +

Nit: As before - no double blank lines please.

Jan