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

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

On 5/31/22 06:49, Bertrand Marquis wrote:
> Hi Daniel,
> 
>> On 31 May 2022, at 11:45, Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx> 
>> wrote:
>>
>> 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.
> 
> If this is wanted this is ok, just mention it in the commit message so that 
> we know it was on purpose.

Understood, I will update the commit message on this one and the others
where there is done.

>>
>>> 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.
> 
> Cheers
> Bertrand
> 
>>
>> v/r,
>> dps
> 

v/r,
dps