Re: [RFC PATCH 1/4] kconfig: allow configuration of maximum modules

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Daniel,

> On 31 May 2022, at 03:41, Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx> 
> wrote:
> 
> For x86 the number of allowable multiboot modules varies between the different
> entry points, non-efi boot, pvh boot, and efi boot. In the case of both Arm 
> and
> x86 this value is fixed to values based on generalized assumptions. With
> hyperlaunch for x86 and dom0less on Arm, use of static sizes results in large
> allocations compiled into the hypervisor that will go unused by many use 
> cases.
> 
> This commit introduces a Kconfig variable that is set with sane defaults based
> on configuration selection. This variable is in turned used as the array size
> for the cases where a static allocated array of boot modules is declared.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> ---
> xen/arch/Kconfig                  | 12 ++++++++++++
> xen/arch/arm/include/asm/setup.h  |  5 +++--
> xen/arch/x86/efi/efi-boot.h       |  2 +-
> xen/arch/x86/guest/xen/pvh-boot.c |  2 +-
> xen/arch/x86/setup.c              |  4 ++--
> 5 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig
> index f16eb0df43..57b14e22c9 100644
> --- a/xen/arch/Kconfig
> +++ b/xen/arch/Kconfig
> @@ -17,3 +17,15 @@ config NR_CPUS
>         For CPU cores which support Simultaneous Multi-Threading or similar
>         technologies, this the number of logical threads which Xen will
>         support.
> +
> +config NR_BOOTMODS
> +     int "Maximum number of boot modules that a loader can pass"
> +     range 1 64
> +     default "8" if X86
> +     default "32" if ARM
> +     help
> +       Controls the build-time size of various arrays allocated for
> +       parsing the boot modules passed by a loader when starting Xen.
> +
> +       This is of particular interest when using Xen's hypervisor domain
> +       capabilities such as dom0less.
> diff --git a/xen/arch/arm/include/asm/setup.h 
> b/xen/arch/arm/include/asm/setup.h
> index 2bb01ecfa8..312a3e4209 100644
> --- a/xen/arch/arm/include/asm/setup.h
> +++ b/xen/arch/arm/include/asm/setup.h
> @@ -10,7 +10,8 @@
> 
> #define NR_MEM_BANKS 256
> 
> -#define MAX_MODULES 32 /* Current maximum useful modules */
> +/* Current maximum useful modules */
> +#define MAX_MODULES CONFIG_NR_BOOTMODS
> 
> typedef enum {
>     BOOTMOD_XEN,
> @@ -38,7 +39,7 @@ struct meminfo {
>  * The domU flag is set for kernels and ramdisks of "xen,domain" nodes.
>  * The purpose of the domU flag is to avoid getting confused in
>  * kernel_probe, where we try to guess which is the dom0 kernel and
> - * initrd to be compatible with all versions of the multiboot spec. 
> + * initrd to be compatible with all versions of the multiboot spec.

This seems to be a spurious change.

With that fixed, for the arm part:
Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Cheers
Bertrand