[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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





On 30/05/2018 22:39, Stefano Stabellini wrote:
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.

The other solution is to introduce a new command (or script) that will select the platform at the same time as olddefconfig.

This would avoid to create a config per board and still keeping only one tiny 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.

This is not what I suggested for all. What I suggested is the option All will select all platforms/*.c file to build and does not select any drivers. The user will have to chose the drivers. You can see it as a "custom" option.

Also, by a list I meant:

config PLATFORM_RCAR3
    ...

choice
   prompt "Machine"
   default ....

config ALL
   select PLATFORM_RCAR3
   select PLATFORM_XILINX

config RCAR3
   prompt "RCAR 3 support"
   select PLATFORM_RCAR3
   select HAS_PL011
   select ...

endchoice.

The config PLATFORM_* would then be used in the Makefile
   xen-$(CONFIG_PLATFORM_XILINX) += xilinx.o
   ...

I can also understand that there might be issue with the "All" option hence why I suggested the "NONE" platform. This would select none of the PLATFORM_*.

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.