[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] RE: [PATCH v2 04/40] xen/arm: add an option to define Xen start address for Armv8-R
Hi Julien, > -----Original Message----- > From: Julien Grall <julien@xxxxxxx> > Sent: 2023年1月18日 17:44 > To: Wei Chen <Wei.Chen@xxxxxxx>; Penny Zheng <Penny.Zheng@xxxxxxx>; xen- > devel@xxxxxxxxxxxxxxxxxxxx > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Bertrand Marquis > <Bertrand.Marquis@xxxxxxx>; Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>; > Jiamei Xie <Jiamei.Xie@xxxxxxx> > 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 <julien@xxxxxxx> > >> Sent: 2023年1月18日 7:24 > >> To: Penny Zheng <Penny.Zheng@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx > >> Cc: Wei Chen <Wei.Chen@xxxxxxx>; Stefano Stabellini > >> <sstabellini@xxxxxxxxxx>; Bertrand Marquis <Bertrand.Marquis@xxxxxxx>; > >> Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>; Jiamei Xie > >> <Jiamei.Xie@xxxxxxx> > >> 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 <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 platforms to Xen, that means the > >>> existed Arm64 platforms should not be listed in Armv8-R platform > >>> list, so we add !ARM_V8R dependency for these platforms. > >>> > >>> Signed-off-by: Wei Chen <wei.chen@xxxxxxx> > >>> Signed-off-by: Jiamei.Xie <jiamei.xie@xxxxxxx> > >> > >> 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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |