RE: [PATCH v6 05/11] xen/arm: define Xen start address for FVP BaseR platform

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: 2022年11月7日 3:20
> To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: nd <nd@xxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Bertrand
> Marquis <Bertrand.Marquis@xxxxxxx>; Volodymyr Babchuk
> <Volodymyr_Babchuk@xxxxxxxx>; Jiamei Xie <Jiamei.Xie@xxxxxxx>
> Subject: Re: [PATCH v6 05/11] xen/arm: define Xen start address for FVP
> BaseR platform
> 
> 
> 
> On 04/11/2022 10:07, Wei Chen wrote:
> > On Armv8-A, Xen has a fixed virtual start address (link address
> > too) for all Armv8-A platforms. In an MMU based system, Xen can
> > map its loaded address to this virtual start address. So, on
> > Armv8-A platforms, the Xen start address does not need to be
> > configurable. But on Armv8-R platforms, there is no MMU to map
> > loaded address to a fixed virtual address and different platforms
> > will have very different address space layout. So Xen cannot use
> > a fixed physical address on MPU based system and need to have it
> > configurable.
> >
> > So in this patch, we reuse the existing arm/platforms to store
> > Armv8-R platforms' parameters. And `XEN_START_ADDRESS` is one
> > kind of FVP BaseR platform's parameters. So we define default
> > `XEN_START_ADDRESS` for FVP BaseR in its platform file.
> >
> > We also introduce one Kconfig option for users to override the
> > default Xen start address of selected platform, if they think
> > the default address doesn't suit their scenarios. For this
> > Kconfig option, we use an unaligned address "0xffffffff" as the
> > default value to indicate that users haven't used a customized
> > Xen start address.
> >
> > And as we introduced Armv8-R platforms to Xen, that means the
> > existed Arm64 platforms should not be listed in Armv8-R platform
> > list, so we add !ARM_V8R dependency for these platforms.
> >
> > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > Signed-off-by: Jiamei.Xie <jiamei.xie@xxxxxxx>
> > ---
> >   xen/arch/arm/Kconfig                           | 11 +++++++++++
> >   xen/arch/arm/include/asm/platforms/fvp_baser.h | 14 ++++++++++++++
> 
> I looked at the content of fvp_baser.h after this series is applied.
> There are a bit of boiler plate that I expect to be part for other
> platforms. In particular...
> 
> >   xen/arch/arm/platforms/Kconfig                 | 16 +++++++++++++---
> >   3 files changed, 38 insertions(+), 3 deletions(-)
> >   create mode 100644 xen/arch/arm/include/asm/platforms/fvp_baser.h
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index ad592367bd..ac276307d6 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -138,6 +138,17 @@ config TEE
> >       This option enables generic TEE mediators support. It allows
> guests
> >       to access real TEE via one of TEE mediators implemented in XEN.
> >
> > +config XEN_START_ADDRESS
> > +   hex "Xen start address: keep default to use platform defined
> address"
> > +   default 0xFFFFFFFF
> 
> ... this default value will need to be tested everywhere. At least for
> now, I think you can avoid the per platform header by using the Kconfig
> to select the proper address (see the config for selecting early printk
> address).
> 
> This will also avoids to use an invalid value here.
> 

We had considered to use Kconfig to define the start addresses of v8R64
platforms (prompt users to input the address). But we also want to provide
a default start address for each platform (Discussed in [1], header for
default value, Kconfig option for customized address).

We also had thought to use Kconfig to define a default start address
for each platform like what we had done for early printk in RFC[2].
But this method has been deprecated.

So if we don’t use header files, just use the Kconfig, we can't
provide a default start address for platforms, and have to force users
to enter the start address.

[1] https://lists.xenproject.org/archives/html/xen-devel/2022-05/msg00643.html
[2] 
https://gitlab.com/xen-project/fusa/xen-integration/-/blob/integration/mpu/xen/arch/arm/Kconfig.debug

> > +   depends on HAS_MPU
> > +   help
> > +     This option allows to set the customized address at which Xen will
> be
> > +     linked on MPU systems. This address must be aligned to a page size.
> > +     Use 0xFFFFFFFF as the default value to indicate that user hasn't
> > +     customized this address, and Xen use use the default value that
> has
> > +     been defined in platform files.
> > +
> >   source "arch/arm/tee/Kconfig"
> >
> >   config STATIC_SHM
> > diff --git a/xen/arch/arm/include/asm/platforms/fvp_baser.h
> b/xen/arch/arm/include/asm/platforms/fvp_baser.h
> > new file mode 100644
> > index 0000000000..9450a411a9
> > --- /dev/null
> > +++ b/xen/arch/arm/include/asm/platforms/fvp_baser.h
> > @@ -0,0 +1,14 @@
> > +#ifndef __ASM_ARM_PLATFORMS_FVP_BASER_H__
> > +#define __ASM_ARM_PLATFORMS_FVP_BASER_H__
> > +
> > +/*
> > + * 0xFFFFFFFF indicates users haven't customized XEN_START_ADDRESS,
> > + * we will use platform defined default address.
> > + */
> > +#if CONFIG_XEN_START_ADDRESS == 0xFFFFFFFF
> > +#define XEN_START_ADDRESS 0x200000
> > +#else
> > +#define XEN_START_ADDRESS CONFIG_XEN_START_ADDRESS
> > +#endif
> > +
> > +#endif /* __ASM_ARM_PLATFORMS_FVP_BASER_H__ */
> > diff --git a/xen/arch/arm/platforms/Kconfig
> b/xen/arch/arm/platforms/Kconfig
> > index c93a6b2756..0904793a0b 100644
> > --- a/xen/arch/arm/platforms/Kconfig
> > +++ b/xen/arch/arm/platforms/Kconfig
> > @@ -1,6 +1,7 @@
> >   choice
> >     prompt "Platform Support"
> >     default ALL_PLAT
> > +   default FVP_BASER if ARM_V8R
> 
> Is there any reason to create a new Kconfig rather than using MPU?
> 

Did you mean FVP_BASER? If yes, we want to give each board a MACRO
to indicate its specific configurations. In current series, this MACRO
only be used for board specific start address.

If you meant Armv8R, that's because Armv8R does not equal to MPU.
We only allow Armv8R code to detect MPU features. MPU is configurable
In EL2.

Cheers,
Wei Chen

> >     ---help---
> >     Choose which hardware platform to enable in Xen.
> >
> > @@ -8,13 +9,14 @@ choice
> >
> >   config ALL_PLAT
> >     bool "All Platforms"
> > +   depends on !ARM_V8R
> >     ---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
> > +   depends on ARM_64 && !ARM_V8R
> >     select GICV3
> >     select HAS_PL011
> >     ---help---
> > @@ -23,7 +25,7 @@ config QEMU
> >
> >   config RCAR3
> >     bool "Renesas RCar3 support"
> > -   depends on ARM_64
> > +   depends on ARM_64 && !ARM_V8R
> >     select HAS_SCIF
> >     select IPMMU_VMSA
> >     ---help---
> > @@ -31,14 +33,22 @@ config RCAR3
> >
> >   config MPSOC
> >     bool "Xilinx Ultrascale+ MPSoC support"
> > -   depends on ARM_64
> > +   depends on ARM_64 && !ARM_V8R
> >     select HAS_CADENCE_UART
> >     select ARM_SMMU
> >     ---help---
> >     Enable all the required drivers for Xilinx Ultrascale+ MPSoC
> >
> > +config FVP_BASER
> > +   bool "Fixed Virtual Platform BaseR support"
> > +   depends on ARM_V8R
> > +   help
> > +     Enable platform specific configurations for Fixed Virtual
> > +     Platform BaseR
> > +
> >   config NO_PLAT
> >     bool "No Platforms"
> > +   depends on !ARM_V8R
> >     ---help---
> >     Do not enable specific support for any platform.
> >
> 
> Cheers,
> 
> --
> Julien Grall