Re: [Xen-devel] [PATCH v7 08/12] arm: add ALL, QEMU, Rcar3 and MPSoC configs

[Andrii Anisov <andrii_anisov@xxxxxxxx>]

Hello Stefano,


On 27.07.18 01:46, Stefano Stabellini wrote:
> On Tue, 24 Jul 2018, Andrii Anisov wrote:
> > On 07.07.18 02:13, Stefano Stabellini wrote:
> > > Add a "Platform Support" choice with four kconfig options: QEMU, RCAR3,
> > > MPSOC and ALL. They enable the required options for their hardware
> > > platform. ALL enables all available platforms and it's the default. It
> > > doesn't automatically select any of the related drivers, otherwise they
> > > cannot be disabled. ALL is implemented by selecting hidden options
> > > corresponding to QEMU, MPSOC and RCAR3.
> > >
> > > In the case of the MPSOC that has a platform file under
> > > arch/arm/platforms/, build the file if MPSOC.
> > >
> > > Signed-off-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > CC: artem_mygaiev@xxxxxxxx
> > > CC: volodymyr_babchuk@xxxxxxxx
> > >
> > > ---
> > > Changes in v5:
> > > - turn platform support into a choice
> > > - add ALL
> > >
> > > Changes in v4:
> > > - fix GICv3/GICV3
> > > - default y to all options
> > > - build xilinx-zynqmp if MPSOC
> > > ---
> > >    xen/arch/arm/Kconfig            |  2 ++
> > >    xen/arch/arm/platforms/Kconfig  | 55
> > > +++++++++++++++++++++++++++++++++++++++++
> > >    xen/arch/arm/platforms/Makefile |  2 +-
> > >    3 files changed, 58 insertions(+), 1 deletion(-)
> > >    create mode 100644 xen/arch/arm/platforms/Kconfig
> > >
> > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > > index 2b87111..75cacfb 100644
> > > --- a/xen/arch/arm/Kconfig
> > > +++ b/xen/arch/arm/Kconfig
> > > @@ -213,6 +213,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..07c5930
> > > --- /dev/null
> > > +++ b/xen/arch/arm/platforms/Kconfig
> > > @@ -0,0 +1,55 @@
> > > +choice
> > > +       prompt "Platform Support"
> > > +       default ALL
> > > +       ---help---
> > > +       Choose which hardware platform to enable in Xen.
> > > +
> > > +       If unsure, choose ALL.
> > > +
> > > +config ALL
> > I would suggest to separate it into ALL_ARM32 and ALL_ARM64. Then, in a
> > makefile use them for platforms instead of raw ARM32 and ARM64. This would
> > make such change really useful: disabling ALL_x will drop all odd platform
> > code.
> Hi Andrii,
>
> I don't understand the suggestion: ARM32 platforms cannot be enabled on
> ARM64 and vice versa.
Indeed.

>   So basically it is as if you always get only
> ALL_ARM32 or ALL_ARM64 depending on your target architecture.
>
> Am I missing something?
With this patch, deselecting "config ALL" does not remove all platform 
code from the build. It is because build of the most of that code 
depends directly on ARMxx.
In order to get a possibility to drop unneeded platform code, the 
platform code should be dependent on "config ALL". But here you do not 
want to mix 32bit and 64bit platforms, so you would need "config ALL_32" 
and "config ALL_64".
For sure, written above is meaningful only for the case if someone needs 
the possibility to drop odd platform code from the build.
> > > +       bool "All Platforms"
> > > +       select MPSOC_PLATFORM
> > > +       select QEMU_PLATFORM
> > > +       select RCAR3_PLATFORM
> > > +       ---help---
> > > +       Enable support for all available hardware platforms. It doesn't
> > > +       automatically select any of the related drivers.
> > > +
> > > +config QEMU
> > > +       bool "QEMU aarch virt machine support"
> > > +       depends on ARM_64
> > > +       select QEMU_PLATFORM
> > > +       select GICV3
> > > +       select HAS_PL011
> > > +       ---help---
> > > +       Enable all the required drivers for QEMU aarch64 virt emulated
> > > +       machine.
> > > +
> > > +config RCAR3
> > > +       bool "Renesas RCar3 support"
> > > +       depends on ARM_64
> > > +       select RCAR3_PLATFORM
> > > +       select HAS_SCIF
> > > +       ---help---
> > > +       Enable all the required drivers for Renesas RCar3
> > > +
> > > +config MPSOC
> > > +       bool "Xilinx Ultrascale+ MPSoC support"
> > > +       depends on ARM_64
> > > +       select MPSOC_PLATFORM
> > > +       select HAS_CADENCE_UART
> > > +       select ARM_SMMU
> > > +       ---help---
> > > +       Enable all the required drivers for Xilinx Ultrascale+ MPSoC
> > > +
> > > +endchoice
> > > +
> > > +config QEMU_PLATFORM
> > > +       bool
> > > +
> > > +config RCAR3_PLATFORM
> > > +       bool
> > > +
> > > +config MPSOC_PLATFORM
> > Shouldn't MPSOC_PLATFORM be dependent on ARM64?
> Yes, and it is, see "config MPSOC" few lines above.
Few lines above, only "config MPSOC" is dependent on ARM64. But 
MPSOC_PLATFORM is selected by "config ALL" at the beginning of the 
patch. And it will be selected for ARM32 as well.
> > > +       bool
> > > +
> > > diff --git a/xen/arch/arm/platforms/Makefile
> > > b/xen/arch/arm/platforms/Makefile
> > > index 80e555c..a79bdb9 100644
> > > --- a/xen/arch/arm/platforms/Makefile
> > > +++ b/xen/arch/arm/platforms/Makefile
> > > @@ -8,4 +8,4 @@ obj-$(CONFIG_ARM_64) += seattle.o
> > >    obj-y += sunxi.o
> > >    obj-$(CONFIG_ARM_64) += thunderx.o
> > >    obj-$(CONFIG_ARM_64) += xgene-storm.o
> > > -obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o
> > > +obj-$(CONFIG_MPSOC_PLATFORM)  += xilinx-zynqmp.o
--

*Andrii Anisov*



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