Re: [PATCH early-RFC 1/5] xen/arm: Clean-up the memory layout

[Julien Grall <julien@xen.org>]

On 17/03/2022 15:23, Bertrand Marquis wrote:
> Hi Julien,

Hi Bertrand,

>> On 9 Mar 2022, at 11:20, Julien Grall <julien@xen.org> wrote:
>>
>> From: Julien Grall <jgrall@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 <jgrall@amazon.com>
> 
> Changes looks ok to me so:
> Reviewed-by: Bertrand Marquis <bertrand.marquis@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?

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

Cheers,

-- 
Julien Grall