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

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 5/31/22 05:07, Bertrand Marquis wrote:
> Hi Daniel,

Greetings Bertrand.

>> 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.

I have been trying to clean up trailing white space when I see it
nearby. I can drop this one if that is desired.

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

Thank you, will add it in when I respin it for submission.

v/r,
dps