Re: [PATCH v3 03/52] xen/arm: add an option to define Xen start address for Armv8-R

[Penny Zheng <penny.zheng@xxxxxxx>]

On 2023/7/4 19:47, Julien Grall wrote:
> On 04/07/2023 11:36, Ayan Kumar Halder wrote:
> > Hi Penny,
> Hi Ayan,
>
> > On 26/06/2023 04:33, Penny Zheng wrote:
> > > CAUTION: This message has originated from an External Source. Please 
> > > use proper judgment and caution when opening attachments, clicking 
> > > links, or responding to this email.
> > >
> > > From: Wei Chen <wei.chen@xxxxxxx>
> > >
> > > 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.
> > >
> > > In this patch we introduce one Kconfig option for users to define
> > > the default Xen start address for Armv8-R. Users can enter the
> > > address in config time, or select the tailored platform config
> > > file from arch/arm/configs.
> > >
> > > And as we introduced Armv8-R to Xen, that means the existed Arm64
> > > MMU based platforms should not be listed in Armv8-R platform
> > > list, so we add !HAS_MPU dependency for these platforms.
> > >
> > > Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> > > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> > > ---
> > > v1 -> v2:
> > > 1. Remove the platform header fvp_baser.h.
> > > 2. Remove the default start address for fvp_baser64.
> > > 3. Remove the description of default address from commit log.
> > > 4. Change HAS_MPU to ARM_V8R for Xen start address dependency.
> > >     No matter Arm-v8r board has MPU or not, it always need to
> > >     specify the start address.
> > > ---
> > > v3:
> > > 1. Remove unrelated change of "CONFIG_FVP_BASER"
> > > 2. Change ARM_V8R to HAS_MPU for Xen start address dependency
> > > ---
> > >   xen/arch/arm/Kconfig           | 8 ++++++++
> > >   xen/arch/arm/platforms/Kconfig | 8 +++++---
> > >   2 files changed, 13 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > > index 70fdc2ba63..ff17345cdb 100644
> > > --- a/xen/arch/arm/Kconfig
> > > +++ b/xen/arch/arm/Kconfig
> > > @@ -181,6 +181,14 @@ 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 0
> > > +       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.
> > > +
> > >   source "arch/arm/tee/Kconfig"
> > >
> > >   config STATIC_SHM
> > > diff --git a/xen/arch/arm/platforms/Kconfig 
> > > b/xen/arch/arm/platforms/Kconfig
> > > index c93a6b2756..75af48b5f9 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 NO_PLAT if HAS_MPU
> > I am a bit concerned about this as we will be introducing R52 specific 
> > platform in xen/arch/arm/platforms/
> > (For eg 
> > https://github.com/Xilinx/xen/blob/xlnx_rebase_4.17/xen/arch/arm_mpu/platforms/amd-versal-net.c )
> > Thus, we will have to remove this line at that time.
> >
> > Can you remove this line, please if it does not cause any issue ?
>  From my understanding of the discussion at Xen Summit, most of the 
> platform specific code would be moved to something similar to bootwrapper.
> So do you still actually need to have code in Xen for setting up the timer?
>
> Cheers,