Re: [Xen-devel] [PATCH v3 09/10] arm: add QEMU, Rcar3 and MPSoC configs

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 29 May 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 23/05/18 01:25, Stefano Stabellini wrote:
> > Add a "Platform Support" menu with three umbrella kconfig options: QEMU,
> > RCAR3 and MPSOC. They enable the required options for their hardware
> > platform.
> > 
> > They are introduced for convience: the user will be able to simply open
> > the menu and enable the right config for her platform.
> > 
> > Signed-off-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > CC: artem_mygaiev@xxxxxxxx
> > CC: volodymyr_babchuk@xxxxxxxx
> > 
> > ---
> > Note that this approach has a limitation: it is not possible to "select
> > a range". In other words, using tiny.config NR_CPUS is set to 4. It is
> > not possible to increase it to 8 from config RCAR3.
> 
> What you can do is:
> 
> config NR_CPUS
>       range ...
>       default "8" if (RCAR3)
>         default "x" if (QEMU)
>       default 64
> 
> This would imply to move NR_CPUS in arch/{arm,x86}/Kconfig.
> 
> This solution is not very nice, but at least would provide a better experience
> to the user.

Unfortunately, make olddefconfig is executed automatically when make is
called, adding CONFIG_NR_CPUS=128. Thus, unless tiny.config has already
CONFIG_RCAR3 in it, the correct default won't be applied.

This suggestions only make sense if we introduce per-platform configs,
such as xen/arch/arm/configs/tiny-rcar3.config.


> > Suggestions are welcome.
> > ---
> >   xen/arch/arm/Kconfig           |  2 ++
> >   xen/arch/arm/platforms/Kconfig | 30 ++++++++++++++++++++++++++++++
> >   2 files changed, 32 insertions(+)
> >   create mode 100644 xen/arch/arm/platforms/Kconfig
> > 
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index a5a6943..b5ddd12 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -245,6 +245,8 @@ config ARM64_HARDEN_BRANCH_PREDICTOR
> >   config ARM32_HARDEN_BRANCH_PREDICTOR
> >       def_bool y if ARM_32 && HARDEN_BRANCH_PREDICTOR
> >   +source "arch/arm/platforms/Kconfig"
> > +
> >   source "common/Kconfig"
> >     source "drivers/Kconfig"
> > diff --git a/xen/arch/arm/platforms/Kconfig b/xen/arch/arm/platforms/Kconfig
> > new file mode 100644
> > index 0000000..0eafbef
> > --- /dev/null
> > +++ b/xen/arch/arm/platforms/Kconfig
> > @@ -0,0 +1,30 @@
> > +menu "Platform Support"
> > +
> > +config QEMU
> > +   bool "QEMU aarch virt machine support"
> > +   default n
> The default value is confusing here. The default .config will support QEMU but
> not select that.
> 
> While I don't yet buy the argument, some users will also want to remove
> platform specific code (Andrii suggest that). This would means by default
> support for a specific platform will not be in Xen.
>
> Furthermore, very likely, the end user will select either one board (e.g
> automotive) or all of them (e.g distribution).
> 
> So I think it would be better to do a choice list:
>       - All -> Board support for all board added. Drivers selected by the
> user
>       - MPSOC -> Select board support for Xilinx + appropriate drivers
>       - RCAR3 -> Select board support for RCAR3 + appropriate drivers
> 
> The tiny.config would select ALL. This could then be refined by selecting a
> specific platform.

The idea of an "ALL" configuration is interesting, however, all the
options we would select under "ALL" already default to "Y". Effectively,
if we remove the following lines from tiny.config:

# CONFIG_GICV3 is not set
# CONFIG_MEM_ACCESS is not set
# CONFIG_SBSA_VUART_CONSOLE is not set
# CONFIG_HAS_NS16550 is not set
# CONFIG_HAS_CADENCE_UART is not set
# CONFIG_HAS_MVEBU is not set
# CONFIG_HAS_PL011 is not set
# CONFIG_HAS_SCIF is not set
# CONFIG_ARM_SMMU is not set

then, it would be as if tiny.config had CONFIG_ALL=y, because after
running `make olddefconfig' it would get all these options set to "Y".

Given that make olddefconfig is always executed automatically by make, I
cannot even remove the "is not set" options above from tiny.config
because otherwise they will all be automatically enabled always unless I
change all the defaults from Y to N.

In fact, the main issue is that it is not possible to deselect Kconfig
options using the Kconfig infrastructure. So, if a user has a
config with CONFIG_ALL in it, then she executes `make menuconfig'
to select RCAR3 and reduce the config size, the menu won't actually be
able to deselect any other option automatically. This is very
unfortunate. For instance, if the config has CADENCE_UART, and the user
selects CONFIG_RCAR3 from the menu, the resulting config will still have
CADENCE_UART, unless she goes to remove it by hand.

Given all this, I don't know if it is worth introducing CONFIG_ALL. I
could add something like:

+config ALL
+       bool "Support for all platforms"
+       default y
+       select GICv3
+       select HAS_NS16550
+       select HAS_CADENCE_UART
+       select HAS_PL011
+       select HAS_EXYNOS4210
+       select HAS_MVEBU
+       select HAS_OMAP
+       select HAS_SCIF
+       select ARM_SMMU
+       ---help---
+       Enable support for all platforms. Triggers the build of a larger Xen
+       binary but with more drivers.
+
+       If unsure, say Y.

but my preference would be to avoid it because it just duplicates the
default Y/N settings elsewhere.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel