[v2,04/40] xen/arm: add an option to define Xen start address for Armv8-R

Commit Message

Penny Zheng Jan. 13, 2023, 5:28 a.m. UTC

From: Wei Chen <[e-mail]>

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 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]>
---
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.
---
 xen/arch/arm/Kconfig           |  8 ++++++++
 xen/arch/arm/platforms/Kconfig | 16 +++++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

Comments

Julien Grall Jan. 17, 2023, 11:24 p.m. UTC | #1

Hi Penny,

On 13/01/2023 05:28, Penny Zheng wrote:
> From: Wei Chen <[e-mail]>
> 
> 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 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]>

Your signed-off-by is missing.

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

I don't quite understand the last sentence. Are you saying that it is 
possible to have an ARMv8-R system with an MPU nor a page-table?

> ---
>   xen/arch/arm/Kconfig           |  8 ++++++++
>   xen/arch/arm/platforms/Kconfig | 16 +++++++++++++---
>   2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index ace7178c9a..c6b6b612d1 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -145,6 +145,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 ARM_V8R

It is still pretty unclear to me what would be the difference between 
HAS_MPU and ARM_V8R.

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

This seems unrelated to this patch.

> +
>   config NO_PLAT
>   	bool "No Platforms"
> +	depends on !ARM_V8R
>   	---help---
>   	Do not enable specific support for any platform.
>   

Cheers,

Wei Chen Jan. 18, 2023, 3 a.m. UTC | #2

Hi Julien,

> -----Original Message-----
> From: Julien Grall <[e-mail]>
> Sent: 2023年1月18日 7:24
> To: Penny Zheng <[e-mail]>; [e-mail]
> Cc: Wei Chen <[e-mail]>; Stefano Stabellini
> <[e-mail]>; Bertrand Marquis <[e-mail]>;
> Volodymyr Babchuk <[e-mail]>; Jiamei Xie
> <[e-mail]>
> Subject: Re: [PATCH v2 04/40] xen/arm: add an option to define Xen start
> address for Armv8-R
> 
> Hi Penny,
> 
> On 13/01/2023 05:28, Penny Zheng wrote:
> > From: Wei Chen <[e-mail]>
> >
> > 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 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]>
> 
> Your signed-off-by is missing.
> 
> > ---
> > 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.
> 
> I don't quite understand the last sentence. Are you saying that it is
> possible to have an ARMv8-R system with an MPU nor a page-table?
> 

Yes, from the Cortex-R82 page [1], you can see the MPU is optional in EL1
and EL2:
"Two optional and programmable MPUs controlled from EL1 and EL2 respectively."

Although it is unlikely that vendors using the Armv8-R IP will do so, it
is indeed an option. In the ID register, there are also related bits in
ID_AA64MMFR0_EL1 (MSA_frac) to indicate this.

> > ---
> >   xen/arch/arm/Kconfig           |  8 ++++++++
> >   xen/arch/arm/platforms/Kconfig | 16 +++++++++++++---
> >   2 files changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> > index ace7178c9a..c6b6b612d1 100644
> > --- a/xen/arch/arm/Kconfig
> > +++ b/xen/arch/arm/Kconfig
> > @@ -145,6 +145,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 ARM_V8R
> 
> It is still pretty unclear to me what would be the difference between
> HAS_MPU and ARM_V8R.
> 

If we don't want to support non-MPU supported Armv8-R, I think they are the
same. IMO, non-MPU supported Armv8-R is meaningless to Xen.

> > +	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..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
> 
> This seems unrelated to this patch.
> 

Can we add some descriptions in commit log for this change, or we
Should move it to a new patch? We had preferred to use separate
patches for this kind of changes, but we found the number of patches
would become more and more. This problem has been bothering us for
organizing patches.

[1] https://developer.arm.com/Processors/Cortex-R82

Cheers,
Wei Chen

> > +
> >   config NO_PLAT
> >   	bool "No Platforms"
> > +	depends on !ARM_V8R
> >   	---help---
> >   	Do not enable specific support for any platform.
> >
> 
> Cheers,
> 
> --
> Julien Grall

Julien Grall Jan. 18, 2023, 9:44 a.m. UTC | #3

On 18/01/2023 03:00, Wei Chen wrote:
> Hi Julien,

Hi Wei,

>> -----Original Message-----
>> From: Julien Grall <[e-mail]>
>> Sent: 2023年1月18日 7:24
>> To: Penny Zheng <[e-mail]>; [e-mail]
>> Cc: Wei Chen <[e-mail]>; Stefano Stabellini
>> <[e-mail]>; Bertrand Marquis <[e-mail]>;
>> Volodymyr Babchuk <[e-mail]>; Jiamei Xie
>> <[e-mail]>
>> Subject: Re: [PATCH v2 04/40] xen/arm: add an option to define Xen start
>> address for Armv8-R
>>
>> Hi Penny,
>>
>> On 13/01/2023 05:28, Penny Zheng wrote:
>>> From: Wei Chen <[e-mail]>
>>>
>>> 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 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]>
>>
>> Your signed-off-by is missing.
>>
>>> ---
>>> 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.
>>
>> I don't quite understand the last sentence. Are you saying that it is
>> possible to have an ARMv8-R system with an MPU nor a page-table?
>>
> 
> Yes, from the Cortex-R82 page [1], you can see the MPU is optional in EL1
> and EL2:
> "Two optional and programmable MPUs controlled from EL1 and EL2 respectively."
Would this mean a vendor may provide their custom solution to protect 
the memory?

> 
> Although it is unlikely that vendors using the Armv8-R IP will do so, it
> is indeed an option. In the ID register, there are also related bits in
> ID_AA64MMFR0_EL1 (MSA_frac) to indicate this.
> 
>>> ---
>>>    xen/arch/arm/Kconfig           |  8 ++++++++
>>>    xen/arch/arm/platforms/Kconfig | 16 +++++++++++++---
>>>    2 files changed, 21 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>>> index ace7178c9a..c6b6b612d1 100644
>>> --- a/xen/arch/arm/Kconfig
>>> +++ b/xen/arch/arm/Kconfig
>>> @@ -145,6 +145,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 ARM_V8R
>>
>> It is still pretty unclear to me what would be the difference between
>> HAS_MPU and ARM_V8R.
>>
> 
> If we don't want to support non-MPU supported Armv8-R, I think they are the
> same. IMO, non-MPU supported Armv8-R is meaningless to Xen.
OOI, why do you think this is meaningless?

> 
>>> +	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..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
>>
>> This seems unrelated to this patch.
>>
> 
> Can we add some descriptions in commit log for this change, or we
> Should move it to a new patch? 

New patch please or introduce it in the patch where you need it.

We had preferred to use separate
> patches for this kind of changes, but we found the number of patches
> would become more and more. This problem has been bothering us for
> organizing patches.

I understand the concern of increasing the number of patches. However, 
this also needs to weight against the review.

In this case, it is very difficult for me to understand why we need to 
introduce FVP_BASER.

In fact, on the previous version, we discussed to not introduce any new 
platform specific config. So I am a bit surprised this is actually needed.

Cheers,

Wei Chen Jan. 18, 2023, 10:22 a.m. UTC | #4

Hi Julien,

> -----Original Message-----
> From: Julien Grall <[e-mail]>
> Sent: 2023年1月18日 17:44
> To: Wei Chen <[e-mail]>; Penny Zheng <[e-mail]>; xen-
> [e-mail]
> Cc: Stefano Stabellini <[e-mail]>; Bertrand Marquis
> <[e-mail]>; Volodymyr Babchuk <[e-mail]>;
> Jiamei Xie <[e-mail]>
> Subject: Re: [PATCH v2 04/40] xen/arm: add an option to define Xen start
> address for Armv8-R
> 
> 
> 
> On 18/01/2023 03:00, Wei Chen wrote:
> > Hi Julien,
> 
> Hi Wei,
> 
> >> -----Original Message-----
> >> From: Julien Grall <[e-mail]>
> >> Sent: 2023年1月18日 7:24
> >> To: Penny Zheng <[e-mail]>; [e-mail]
> >> Cc: Wei Chen <[e-mail]>; Stefano Stabellini
> >> <[e-mail]>; Bertrand Marquis <[e-mail]>;
> >> Volodymyr Babchuk <[e-mail]>; Jiamei Xie
> >> <[e-mail]>
> >> Subject: Re: [PATCH v2 04/40] xen/arm: add an option to define Xen
> start
> >> address for Armv8-R
> >>
> >> Hi Penny,
> >>
> >> On 13/01/2023 05:28, Penny Zheng wrote:
> >>> From: Wei Chen <[e-mail]>
> >>>
> >>> 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 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]>
> >>
> >> Your signed-off-by is missing.
> >>
> >>> ---
> >>> 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.
> >>
> >> I don't quite understand the last sentence. Are you saying that it is
> >> possible to have an ARMv8-R system with an MPU nor a page-table?
> >>
> >
> > Yes, from the Cortex-R82 page [1], you can see the MPU is optional in
> EL1
> > and EL2:
> > "Two optional and programmable MPUs controlled from EL1 and EL2
> respectively."
> Would this mean a vendor may provide their custom solution to protect
> the memory?
> 

Ah, you gave me a new idea, yes in the "ARM DDI 0600A.c G1.3.7" MSA_frac
of ID_AA64MMFR0_EL1 says:
0b0000 PMSAv8-64 not supported in any translation regime.
0b0000 is not permitted value.

So maybe you're right, on Armv8-R64, we always have MPU in EL1&EL2, the
optional is for MPU customization.

> >
> > Although it is unlikely that vendors using the Armv8-R IP will do so, it
> > is indeed an option. In the ID register, there are also related bits in
> > ID_AA64MMFR0_EL1 (MSA_frac) to indicate this.
> >
> >>> ---
> >>>    xen/arch/arm/Kconfig           |  8 ++++++++
> >>>    xen/arch/arm/platforms/Kconfig | 16 +++++++++++++---
> >>>    2 files changed, 21 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> >>> index ace7178c9a..c6b6b612d1 100644
> >>> --- a/xen/arch/arm/Kconfig
> >>> +++ b/xen/arch/arm/Kconfig
> >>> @@ -145,6 +145,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 ARM_V8R
> >>
> >> It is still pretty unclear to me what would be the difference between
> >> HAS_MPU and ARM_V8R.
> >>
> >
> > If we don't want to support non-MPU supported Armv8-R, I think they are
> the
> > same. IMO, non-MPU supported Armv8-R is meaningless to Xen.
> OOI, why do you think this is meaningless?

If there is Armv8-R board without EL2 MPU, how can we protect Xen? Of course,
if users don't care about security, Xen still can support it.

> 
> >
> >>> +	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..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
> >>
> >> This seems unrelated to this patch.
> >>
> >
> > Can we add some descriptions in commit log for this change, or we
> > Should move it to a new patch?
> 
> New patch please or introduce it in the patch where you need it.
> 
> We had preferred to use separate
> > patches for this kind of changes, but we found the number of patches
> > would become more and more. This problem has been bothering us for
> > organizing patches.
> 
> I understand the concern of increasing the number of patches. However,
> this also needs to weight against the review.
> 

Understand.

> In this case, it is very difficult for me to understand why we need to
> introduce FVP_BASER.
> 
> In fact, on the previous version, we discussed to not introduce any new
> platform specific config. So I am a bit surprised this is actually needed.
> 

No, this is no true, it's my mistake, I forgot to remove FVP_BASER from
this Kconfig. Actually, we do not need this one. We also don't need a
new patch for it.

Cheers,
Wei Chen

> Cheers,
> 
> --
> Julien Grall

Julien Grall Jan. 18, 2023, 10:59 a.m. UTC | #5

Hi,

On 18/01/2023 10:22, Wei Chen wrote:
>>> Although it is unlikely that vendors using the Armv8-R IP will do so, it
>>> is indeed an option. In the ID register, there are also related bits in
>>> ID_AA64MMFR0_EL1 (MSA_frac) to indicate this.
>>>
>>>>> ---
>>>>>     xen/arch/arm/Kconfig           |  8 ++++++++
>>>>>     xen/arch/arm/platforms/Kconfig | 16 +++++++++++++---
>>>>>     2 files changed, 21 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>>>>> index ace7178c9a..c6b6b612d1 100644
>>>>> --- a/xen/arch/arm/Kconfig
>>>>> +++ b/xen/arch/arm/Kconfig
>>>>> @@ -145,6 +145,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 ARM_V8R
>>>>
>>>> It is still pretty unclear to me what would be the difference between
>>>> HAS_MPU and ARM_V8R.
>>>>
>>>
>>> If we don't want to support non-MPU supported Armv8-R, I think they are
>> the
>>> same. IMO, non-MPU supported Armv8-R is meaningless to Xen.
>> OOI, why do you think this is meaningless?
> 
> If there is Armv8-R board without EL2 MPU, how can we protect Xen?

So what you call EL2 MPU is an MPU that is following the Arm 
specification. In theory, you could have a proprietary mechanism for that.

So the question is whether a system not following the Arm specification 
is allowed.

Cheers,

Wei Chen Jan. 18, 2023, 11:27 a.m. UTC | #6

Hi Julien,

> -----Original Message-----
> From: Xen-devel <[e-mail]> On Behalf Of
> Julien Grall
> Sent: 2023年1月18日 19:00
> To: Wei Chen <[e-mail]>; Penny Zheng <[e-mail]>; xen-
> [e-mail]
> Cc: Stefano Stabellini <[e-mail]>; Bertrand Marquis
> <[e-mail]>; Volodymyr Babchuk <[e-mail]>;
> Jiamei Xie <[e-mail]>
> Subject: Re: [PATCH v2 04/40] xen/arm: add an option to define Xen start
> address for Armv8-R
> 
> Hi,
> 
> On 18/01/2023 10:22, Wei Chen wrote:
> >>> Although it is unlikely that vendors using the Armv8-R IP will do so,
> it
> >>> is indeed an option. In the ID register, there are also related bits
> in
> >>> ID_AA64MMFR0_EL1 (MSA_frac) to indicate this.
> >>>
> >>>>> ---
> >>>>>     xen/arch/arm/Kconfig           |  8 ++++++++
> >>>>>     xen/arch/arm/platforms/Kconfig | 16 +++++++++++++---
> >>>>>     2 files changed, 21 insertions(+), 3 deletions(-)
> >>>>>
> >>>>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> >>>>> index ace7178c9a..c6b6b612d1 100644
> >>>>> --- a/xen/arch/arm/Kconfig
> >>>>> +++ b/xen/arch/arm/Kconfig
> >>>>> @@ -145,6 +145,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 ARM_V8R
> >>>>
> >>>> It is still pretty unclear to me what would be the difference between
> >>>> HAS_MPU and ARM_V8R.
> >>>>
> >>>
> >>> If we don't want to support non-MPU supported Armv8-R, I think they
> are
> >> the
> >>> same. IMO, non-MPU supported Armv8-R is meaningless to Xen.
> >> OOI, why do you think this is meaningless?
> >
> > If there is Armv8-R board without EL2 MPU, how can we protect Xen?
> 
> So what you call EL2 MPU is an MPU that is following the Arm
> specification. In theory, you could have a proprietary mechanism for that.
> 
> So the question is whether a system not following the Arm specification
> is allowed.
> 

I think no, the PMSA is an architectural feature, the spec contains CPU and MPU
interfaces. Vendors can have their own hardware implementation, but need follow
the Arm spec.

But I agree that, here we could change to "depends on HAS_MPU" which will make
It easier to used by other Arm Architecture or other architecture in the future.

Cheers,
Wei Chen

> Cheers,
> 
> --
> Julien Grall

Patch

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index ace7178c9a..c6b6b612d1 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -145,6 +145,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 ARM_V8R
+	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..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.