Re: [PATCH v5 5/5] arm/dom0less: introduce Kconfig for dom0less feature

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Luca,

On 14/11/2023 10:03, Luca Fancellu wrote:
> 
> 
> Introduce a Kconfig for the dom0less feature, enabled by default,
> to be able to choose if the feature should be compiled or not.
> 
> Provide static inline stubs when the option is disabled for the
> functions externally visible.
> 
> Use the new Kconfig to remove dom0less DT binding from the efi-boot.h
> code when the Kconfig is not enabled, do the same for
> allocate_bank_memory inside domain_build.c that currently is used
> only by dom0less-build.c module, but it's kept there provisioning
> its usage by dom0 code.
> 
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> ---
> Changes from v4:
>  - No changes
> Changes from v3:
>  - Update Kconfig
> Changes from v2:
>  - protect allocate_bank_memory with the new Kconfig
> ---
>  xen/arch/arm/Kconfig                      |  8 ++++++++
>  xen/arch/arm/Makefile                     |  2 +-
>  xen/arch/arm/domain_build.c               |  2 ++
>  xen/arch/arm/efi/efi-boot.h               |  4 ++++
>  xen/arch/arm/include/asm/dom0less-build.h | 12 ++++++++++++
>  xen/arch/arm/include/asm/domain_build.h   |  2 ++
>  xen/common/Kconfig                        |  2 +-
>  7 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 0045a3762d86..f321381a6e87 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -88,6 +88,14 @@ config GICV2
>           Driver for the ARM Generic Interrupt Controller v2.
>           If unsure, say Y
> 
> +config DOM0LESS_BOOT
> +       bool "Dom0less boot support" if EXPERT
> +       default y
> +       help
> +         Dom0less boot support enables Xen to create and start domU guests 
> during
> +         Xen boot without the need of a control domain (Dom0), which could be
> +         present anyway.
> +
>  config GICV3
>         bool "GICv3 driver"
>         depends on !NEW_VGIC
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 89ef0c9075b5..5daf8f10114d 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -15,7 +15,7 @@ obj-y += cpufeature.o
>  obj-y += decode.o
>  obj-y += device.o
>  obj-$(CONFIG_IOREQ_SERVER) += dm.o
> -obj-y += dom0less-build.init.o
> +obj-$(CONFIG_DOM0LESS_BOOT) += dom0less-build.init.o
>  obj-y += domain.o
>  obj-y += domain_build.init.o
>  obj-$(CONFIG_ARCH_MAP_DOMAIN_PAGE) += domain_page.o
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 7eb766da9c63..df66fb88d8ec 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -416,6 +416,7 @@ static void __init allocate_memory_11(struct domain *d,
>      }
>  }
> 
> +#ifdef CONFIG_DOM0LESS_BOOT
>  bool __init allocate_bank_memory(struct domain *d, struct kernel_info *kinfo,
>                                   gfn_t sgfn, paddr_t tot_size)
>  {
> @@ -477,6 +478,7 @@ bool __init allocate_bank_memory(struct domain *d, struct 
> kernel_info *kinfo,
> 
>      return true;
>  }
> +#endif
> 
>  /*
>   * When PCI passthrough is available we want to keep the
> diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
> index 1c3640bb65fd..689dc016d081 100644
> --- a/xen/arch/arm/efi/efi-boot.h
> +++ b/xen/arch/arm/efi/efi-boot.h
> @@ -802,6 +802,7 @@ static int __init handle_module_node(const 
> EFI_LOADED_IMAGE *loaded_image,
>      return 1;
>  }
> 
> +#ifdef CONFIG_DOM0LESS_BOOT
>  /*
>   * This function checks for boot modules under the domU guest domain node
>   * in the DT.
> @@ -849,6 +850,7 @@ static int __init handle_dom0less_domain_node(const 
> EFI_LOADED_IMAGE *loaded_ima
> 
>      return mb_modules_found;
>  }
> +#endif
> 
>  /*
>   * This function checks for xen domain nodes under the /chosen node for 
> possible
> @@ -876,6 +878,7 @@ static int __init efi_check_dt_boot(const 
> EFI_LOADED_IMAGE *loaded_image)
>      {
>          int ret;
> 
> +#ifdef CONFIG_DOM0LESS_BOOT
>          if ( !fdt_node_check_compatible(fdt_efi, node, "xen,domain") )
>          {
>              /* Found a node with compatible xen,domain; handle this node. */
> @@ -884,6 +887,7 @@ static int __init efi_check_dt_boot(const 
> EFI_LOADED_IMAGE *loaded_image)
>                  return ERROR_DT_MODULE_DOMU;
>          }
>          else
> +#endif
>          {
>              ret = handle_module_node(loaded_image, &dir_handle, node, 
> addr_len,
>                                       size_len, false);
> diff --git a/xen/arch/arm/include/asm/dom0less-build.h 
> b/xen/arch/arm/include/asm/dom0less-build.h
> index c5625925d940..db2bf821e7aa 100644
> --- a/xen/arch/arm/include/asm/dom0less-build.h
> +++ b/xen/arch/arm/include/asm/dom0less-build.h
> @@ -3,9 +3,21 @@
>  #ifndef __ASM_DOM0LESS_BUILD_H_
>  #define __ASM_DOM0LESS_BUILD_H_
> 
> +#ifdef CONFIG_DOM0LESS_BOOT
> +
>  void create_domUs(void);
>  bool is_dom0less_mode(void);
> 
> +#else  /* !CONFIG_DOM0LESS_BOOT */
one space between #else and /*

> +
> +static inline void create_domUs(void) {}
> +static inline bool is_dom0less_mode(void)
> +{
> +    return false;
> +}
> +
> +#endif /* CONFIG_DOM0LESS_BOOT */
> +
>  #endif /* __ASM_DOM0LESS_BUILD_H_ */
> 
>  /*
> diff --git a/xen/arch/arm/include/asm/domain_build.h 
> b/xen/arch/arm/include/asm/domain_build.h
> index da9e6025f37c..6df61fa1d587 100644
> --- a/xen/arch/arm/include/asm/domain_build.h
> +++ b/xen/arch/arm/include/asm/domain_build.h
> @@ -6,8 +6,10 @@
> 
>  typedef __be32 gic_interrupt_t[3];
> 
> +#ifdef CONFIG_DOM0LESS_BOOT
AFAICT, guarding prototypes is not needed and we opt to avoid it. I'd remove it.

The remarks can be handled on commit (depending on the conflict with Henry's 
series)
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

~Michal