Hi Julien, > On 17 Mar 2022, at 20:32, Julien Grall <jul...@xen.org> wrote: > > > > On 17/03/2022 15:23, Bertrand Marquis wrote: >> Hi Julien, > > Hi Bertrand, > >>> On 9 Mar 2022, at 11:20, Julien Grall <jul...@xen.org> wrote: >>> >>> From: Julien Grall <jgr...@amazon.com> >>> >>> In a follow-up patch, the base address for the common mappings will >>> vary between arm32 and arm64. To avoid any duplication, define >>> every mapping in the common region from the previous one. >>> >>> Take the opportunity to add mising *_SIZE for some mappings. >>> >>> Signed-off-by: Julien Grall <jgr...@amazon.com> >> Changes looks ok to me so: >> Reviewed-by: Bertrand Marquis <bertrand.marq...@arm.com> >> Only one question after. >>> >>> --- >>> >>> After the next patch, the term "common" will sound strange because >>> the base address is different. Any better suggestion? >> MAPPING_VIRT_START ? > > For arm32, I am planning to reshuffle the layout so the runtime address is > always at the end of the layout. > > So I think MAPPING_VIRT_START may be as confusing. How about > SHARED_ARCH_VIRT_MAPPING?
> >> Or space maybe.. > > I am not sure I understand this suggestion. Can you clarify? VIRT_SPACE_START was in my mind at that time but that is also not good How about using BASE instead of start: MAPPING_COMMON_VIRT_BASE ? Anyway hard to find a nice name, so your solution with SHARED is ok for me unless someone has a better suggestion. > >>> --- >>> xen/arch/arm/include/asm/config.h | 24 +++++++++++++++++------- >>> 1 file changed, 17 insertions(+), 7 deletions(-) >>> >>> diff --git a/xen/arch/arm/include/asm/config.h >>> b/xen/arch/arm/include/asm/config.h >>> index aedb586c8d27..5db28a8dbd56 100644 >>> --- a/xen/arch/arm/include/asm/config.h >>> +++ b/xen/arch/arm/include/asm/config.h >>> @@ -107,16 +107,26 @@ >>> * Unused >>> */ >>> >>> -#define XEN_VIRT_START _AT(vaddr_t,0x00200000) >>> -#define FIXMAP_ADDR(n) (_AT(vaddr_t,0x00400000) + (n) * PAGE_SIZE) >>> +#define COMMON_VIRT_START _AT(vaddr_t, 0) >>> >>> -#define BOOT_FDT_VIRT_START _AT(vaddr_t,0x00600000) >>> -#define BOOT_FDT_SLOT_SIZE MB(4) >>> -#define BOOT_FDT_VIRT_END (BOOT_FDT_VIRT_START + BOOT_FDT_SLOT_SIZE) >>> +#define XEN_VIRT_START (COMMON_VIRT_START + MB(2)) >>> +#define XEN_SLOT_SIZE MB(2) >> I know this is not modified by your patch, but any idea why SLOT is used >> here ? >> XEN_VIRT_SIZE would sound a bit more logic. > > No idea. I can add a patch to rename it. I think it would be a good idea, we already have a lot of terms in here and SLOT is just adding to the confusion I find. Thanks Bertrand
