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

[Julien Grall <julien@xxxxxxx>]

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

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