Re: [RFC v2 01/12] xen/arm: Clean-up the memory layout

[Julien Grall <julien@xxxxxxx>]

On 25/10/2022 11:36, Michal Orzel wrote:
> Hi,
>
> On 25/10/2022 12:31, Julien Grall wrote:
> > Caution: This message originated from an External Source. Use proper caution 
> > when opening attachments, clicking links, or responding.
> >
> >
> > On 25/10/2022 10:21, Michal Orzel wrote:
> > > Hi Julien,
> > Hi Michal,
> >
> >
> > > On 22/10/2022 17:04, Julien Grall 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.
> > > Maybe the following title would be better suited if you only want to touch the 
> > > common mappings?
> > > "xen/arm: Clean-up the common memory layout"
> > Ok.
> >
> > > > Take the opportunity to add missing *_SIZE for FIXMAP_VIRT_* and
> > > > XEN_VIRT_*.
> > > >
> > > > Take the opportunity to add missing *_SIZE for some mappings.
> > > I think this sentence can be removed as the previous one already covers it.
> > Yes. I will remove it.
> >
> > > Apart from that, wouldn't it be useful to do the cleanup for the arm32 macros
> > > as you are here (FRAMETABLE, VMAP, {XEN/DOM}HEAP) ?
> > Can you clarify? I looked at the macros and they look clean to me.
> As you choose to make use of MB() macro, I think it would be beneficial
> just from the consistency perspective to modify other places that use directly 
> hex values.
> We already do that for arm64 specific mappings, you are modifying the common 
> ones
> to adhere to this way, so as a natural consequence I would see the arm32 ones 
> to be modified as well.
You have a point. I will look to do it in this patch.

Cheers,

--
Julien Grall