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

[Julien Grall <julien@xxxxxxx>]

On 31/05/2022 11:53, Daniel P. Smith wrote:
> On 5/31/22 05:25, Julien Grall wrote:
> > Hi,
> >
> > On 31/05/2022 03:41, Daniel P. Smith wrote:
> > > 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
> > OOI, any reason to limit the size?
> I modelled this entirely after NR_CPUS, which applied a limit
The limit for NR_CPUS makes sense because there are scalability issues 
after that (although 4095 seems quite high) and/or the HW impose a limit.
> , and it
> seemed reasonable to me at the time. I choose 64 since it was double
> currently what Arm had set for MAX_MODULES. As such, I have no hard
> reason for there to be a limit. It can easily be removed or adjusted to
> whatever the reviewers feel would be appropriate.
Ok. In which case I would drop the limit beause it also prevent a users 
to create more 64 dom0less domUs (actually a bit less because some 
modules are used by Xen). I don't think there are a strong reason to 
prevent that, right?
> > > +    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
> > There are only a handful number of use of MAX_MODULES in Arm. So I would
> > prefer if we replace all the use with CONFIG_NR_BOOTMODS.
> No problem, as I stated above, I mimicked what was done for NR_CPUS
> which did a similar #define for MAX_CPUS.
Thanks!

Cheers,

--
Julien Grall