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

[Julien Grall <julien@xxxxxxx>]

On 09/11/2022 04:55, Wei Chen wrote:
> Hi Julien,
Hi Wei,

> > -----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).
Why do you want to provide a default value? And how it is guaranteed 
that it will work for most of the users?
> 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.
Most of the current Xen is board agnostic except the UART. We push back 
the addition of new one because the address can be found in the firmware 
table and I wanted to avoid increase the number of option (there are 
dozens of platform out...).
> 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.
I am not sure I see the problem to force the user to enter the start 
address. My worry with per-platform default value is we end up to force 
each vendor to provide an header in order to boot Xen.
I think it would be better to provide a config tailored for that 
platform (whether it is part of Xen can be debatable). This would allow 
a user to try a release Xen on their platform with zero changes in the code.
> > > 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.
See above for this.

> If you meant Armv8R, that's because Armv8R does not equal to MPU.
I am not entirely sure to understand. Are you saying that an existing 
Xen can boot on Armv8R?
Cheers,

--
Julien Grall