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

Commit Message

Wei Chen Nov. 4, 2022, 10:07 a.m. UTC

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 <[e-mail]>
Signed-off-by: Jiamei.Xie <[e-mail]>
---
 xen/arch/arm/Kconfig                           | 11 +++++++++++
 xen/arch/arm/include/asm/platforms/fvp_baser.h | 14 ++++++++++++++
 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

Comments

Julien Grall Nov. 6, 2022, 7:19 p.m. UTC | #1

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 <[e-mail]>
> Signed-off-by: Jiamei.Xie <[e-mail]>
> ---
>   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.

> +	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?

>   	---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,

Wei Chen Nov. 9, 2022, 4:55 a.m. UTC | #2

Hi Julien,

> -----Original Message-----
> From: Julien Grall <[e-mail]>
> Sent: 2022年11月7日 3:20
> To: Wei Chen <[e-mail]>; [e-mail]
> Cc: nd <[e-mail]>; Stefano Stabellini <[e-mail]>; Bertrand
> Marquis <[e-mail]>; Volodymyr Babchuk
> <[e-mail]>; Jiamei Xie <[e-mail]>
> 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 <[e-mail]>
> > Signed-off-by: Jiamei.Xie <[e-mail]>
> > ---
> >   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

Julien Grall Nov. 9, 2022, 6:24 p.m. UTC | #3

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

Hi Wei,

> 
>> -----Original Message-----
>> From: Julien Grall <[e-mail]>
>> Sent: 2022年11月7日 3:20
>> To: Wei Chen <[e-mail]>; [e-mail]
>> Cc: nd <[e-mail]>; Stefano Stabellini <[e-mail]>; Bertrand
>> Marquis <[e-mail]>; Volodymyr Babchuk
>> <[e-mail]>; Jiamei Xie <[e-mail]>
>> 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 <[e-mail]>
>>> Signed-off-by: Jiamei.Xie <[e-mail]>
>>> ---
>>>    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,

Stefano Stabellini Nov. 10, 2022, 10:12 p.m. UTC | #4

On Wed, 9 Nov 2022, Julien Grall wrote:
> > > -----Original Message-----
> > > From: Julien Grall <[e-mail]>
> > > Sent: 2022年11月7日 3:20
> > > To: Wei Chen <[e-mail]>; [e-mail]
> > > Cc: nd <[e-mail]>; Stefano Stabellini <[e-mail]>; Bertrand
> > > Marquis <[e-mail]>; Volodymyr Babchuk
> > > <[e-mail]>; Jiamei Xie <[e-mail]>
> > > 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 <[e-mail]>
> > > > Signed-off-by: Jiamei.Xie <[e-mail]>
> > > > ---
> > > >    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.

I agree with Julien, especially on this last point.

Of course we need a default configuration for a given platform, we don't
want every user of the same platform to have to go and look at the
manual to find the right address to use.

The question is where to put the per-platform default value. The kconfig
"default" keyword is not great for that and it is not realistic to have
a single address that works everywhere.

Instead, we could have a prepopulated kconfig under
xen/arch/arm/configs, or something under ImageBuilder, or maybe expand
the existing "Platform Support" kconfig menu.

If this was just XEN_START_ADDRESS, I would suggest to keep it in
xen.git somewhere. But given that there are a few addresses and sizes to
provide/calculate for Xen on MPU to work, using ImageBuilder could be a
good idea.

Wei Chen Nov. 11, 2022, 10:13 a.m. UTC | #5

Hi Stefano, Julien,

> -----Original Message-----
> From: Xen-devel <[e-mail]> On Behalf Of
> Stefano Stabellini
> Sent: 2022年11月11日 6:13
> To: Julien Grall <[e-mail]>
> Cc: Wei Chen <[e-mail]>; [e-mail]; nd
> <[e-mail]>; Stefano Stabellini <[e-mail]>; Bertrand
> Marquis <[e-mail]>; Volodymyr Babchuk
> <[e-mail]>; Jiamei Xie <[e-mail]>
> Subject: Re: [PATCH v6 05/11] xen/arm: define Xen start address for FVP
> BaseR platform
> 
> On Wed, 9 Nov 2022, Julien Grall wrote:
> > > > -----Original Message-----
> > > > From: Julien Grall <[e-mail]>
> > > > Sent: 2022年11月7日 3:20
> > > > To: Wei Chen <[e-mail]>; [e-mail]
> > > > Cc: nd <[e-mail]>; Stefano Stabellini <[e-mail]>;
> Bertrand
> > > > Marquis <[e-mail]>; Volodymyr Babchuk
> > > > <[e-mail]>; Jiamei Xie <[e-mail]>
> > > > 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 <[e-mail]>
> > > > > Signed-off-by: Jiamei.Xie <[e-mail]>
> > > > > ---
> > > > >    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.
> 
> I agree with Julien, especially on this last point.
> 
> Of course we need a default configuration for a given platform, we don't
> want every user of the same platform to have to go and look at the
> manual to find the right address to use.
> 
> The question is where to put the per-platform default value. The kconfig
> "default" keyword is not great for that and it is not realistic to have
> a single address that works everywhere.
> 
> Instead, we could have a prepopulated kconfig under
> xen/arch/arm/configs, or something under ImageBuilder, or maybe expand

Do you mean we can keep a config like armv8r_fvp_baser_config in
xen/arch/arm/configs for users to generate a default config?
If yes I think this method might be better for now. And about ImageBuilder
solution we can do it after MPU support be merged?

Cheers,
Wei Chen

> the existing "Platform Support" kconfig menu.
> 
> If this was just XEN_START_ADDRESS, I would suggest to keep it in
> xen.git somewhere. But given that there are a few addresses and sizes to
> provide/calculate for Xen on MPU to work, using ImageBuilder could be a
> good idea.

Stefano Stabellini Nov. 11, 2022, 8:15 p.m. UTC | #6

On Fri, 11 Nov 2022, Wei Chen wrote:
> Hi Stefano, Julien,
> 
> > -----Original Message-----
> > From: Xen-devel <[e-mail]> On Behalf Of
> > Stefano Stabellini
> > Sent: 2022年11月11日 6:13
> > To: Julien Grall <[e-mail]>
> > Cc: Wei Chen <[e-mail]>; [e-mail]; nd
> > <[e-mail]>; Stefano Stabellini <[e-mail]>; Bertrand
> > Marquis <[e-mail]>; Volodymyr Babchuk
> > <[e-mail]>; Jiamei Xie <[e-mail]>
> > Subject: Re: [PATCH v6 05/11] xen/arm: define Xen start address for FVP
> > BaseR platform
> > 
> > On Wed, 9 Nov 2022, Julien Grall wrote:
> > > > > -----Original Message-----
> > > > > From: Julien Grall <[e-mail]>
> > > > > Sent: 2022年11月7日 3:20
> > > > > To: Wei Chen <[e-mail]>; [e-mail]
> > > > > Cc: nd <[e-mail]>; Stefano Stabellini <[e-mail]>;
> > Bertrand
> > > > > Marquis <[e-mail]>; Volodymyr Babchuk
> > > > > <[e-mail]>; Jiamei Xie <[e-mail]>
> > > > > 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 <[e-mail]>
> > > > > > Signed-off-by: Jiamei.Xie <[e-mail]>
> > > > > > ---
> > > > > >    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.
> > 
> > I agree with Julien, especially on this last point.
> > 
> > Of course we need a default configuration for a given platform, we don't
> > want every user of the same platform to have to go and look at the
> > manual to find the right address to use.
> > 
> > The question is where to put the per-platform default value. The kconfig
> > "default" keyword is not great for that and it is not realistic to have
> > a single address that works everywhere.
> > 
> > Instead, we could have a prepopulated kconfig under
> > xen/arch/arm/configs, or something under ImageBuilder, or maybe expand
> 
> Do you mean we can keep a config like armv8r_fvp_baser_config in
> xen/arch/arm/configs for users to generate a default config?

Yes


> If yes I think this method might be better for now. And about ImageBuilder
> solution we can do it after MPU support be merged?

That's fine

Ayan Kumar Halder Nov. 14, 2022, 6:52 p.m. UTC | #7

Hi Wei,

On 04/11/2022 10:07, Wei Chen 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.
>
>
> 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 <[e-mail]>
> Signed-off-by: Jiamei.Xie <[e-mail]>
> ---
>   xen/arch/arm/Kconfig                           | 11 +++++++++++
>   xen/arch/arm/include/asm/platforms/fvp_baser.h | 14 ++++++++++++++
>   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
> +       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

I could not spot the patch which introduced ARM_V8R.

Can you rename this to ARM64_V8R ? The reason being the underlying code 
is specific to R82 ie 64 bit V8R.

- Ayan

>          ---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.
>
> --
> 2.25.1
>
>

Wei Chen Nov. 15, 2022, 5:42 a.m. UTC | #8

Hi Ayan,

> > +
> > +#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
> 
> I could not spot the patch which introduced ARM_V8R.
> 

That patch is not in this part, it will be the last one of MPU support
patch series. You can find it gitlab branch's full series.

> Can you rename this to ARM64_V8R ? The reason being the underlying code
> is specific to R82 ie 64 bit V8R.
> 

I renamed ARM64_V8R (in RFC patch) to ARM_V8R is because "Arm64 v8r" is
not an official Arm architecture name. The Arm official name is Armv8-R
AArch32/AArch64. And currently, MPU will only be selected by Arm64, so
current MPU code can only work in AArch64 state. When you're trying to
enable Armv8-R AArch32 like R52, you can remove this limitation, and use
CONFIG_ARM64 or CONFIG_ARM32 to distinguish code between r82 and r52 code.

Cheers,
Wei Chen

> - Ayan
> 
> >          ---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.
> >
> > --
> > 2.25.1
> >
> >

Wei Chen Dec. 5, 2022, 10:17 a.m. UTC | #9

Hi Julien,

I thought I had replied to your email, but when I am checking to see if 
I have addressed all your comments, I didn't find my reply on the 
mailing list. Maybe there is something wrong with my mail system, I 
didn't find it sooner. Sorry about it. Hope my reply is not too late.


On 2022/11/10 2:24, Julien Grall wrote:
> 
> 
> On 09/11/2022 04:55, Wei Chen wrote:
>> Hi Julien,
> 
> Hi Wei,
> 
>>>
>>
>> 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.
> 

Above comments have been answered in my reply to you and Stefano.

>>>> 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 we move board specific information to tailored config file, I think
we don't need FVP_BASER.

>>
>> 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?
> 

No, I didn't mean that. I just think we can't use only one MPU or one
ARM_V8R to cover all our changes in this series. For example, some
changes like new system registers are brought by Armv8R not the MPU.

Cheers,
Wei Chen




> Cheers,
>

Julien Grall Dec. 5, 2022, 11:02 a.m. UTC | #10

Hi,

On 05/12/2022 10:17, Wei Chen wrote:
> On 2022/11/10 2:24, Julien Grall wrote:
> 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 we move board specific information to tailored config file, I think
> we don't need FVP_BASER.
> 
>>>
>>> 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?
>>
> 
> No, I didn't mean that. I just think we can't use only one MPU or one
> ARM_V8R to cover all our changes in this series. For example, some
> changes like new system registers are brought by Armv8R not the MPU.

I understand the theory. But in practice this needs to be a balance 
between finer grain and too much Kconfig.

 From this series alone, it doesn't seem to be make sense to introduce 
the two. Furthermore, I am not entirely sure you will be able to make 
the MPU work without enable the ARMv8-R Kconfig.

Cheers,

Patch

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
+	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
 	---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.