Re: [PATCH 8/9] xen/arm: Allow the user to build Xen with USBAN

[Julien Grall <julien@xxxxxxx>]

On 27/06/2023 09:09, Michal Orzel wrote:
> Hi Julien,
>
> On 26/06/2023 14:56, Julien Grall wrote:
> > Hi,
> >
> > On 26/06/2023 12:56, Michal Orzel wrote:
> > > On 25/06/2023 22:49, Julien Grall wrote:
> > > > From: Julien Grall <jgrall@xxxxxxxxxx>
> > > >
> > > > UBSAN has been enabled a few years ago on x86 but was never
> > > > enabled on Arm because the final binary is bigger than 2MB (
> > > > the maximum we can currently handled).
> > > >
> > > > With the recent rework, it is now possible to grow Xen over 2MB.
> > > > So there is no more roadblock to enable Xen other than increasing
> > > > the reserved area.
> > > >
> > > > On my setup, for arm32, the final binaray was very close to 4MB.
> > > > Furthermore, one may want to enable USBAN and GCOV which would put
> > > > the binary well-over 4MB (both features require for some space).
> > > > Therefore, increase the size to 8MB which should us some margin.
> > > >
> > > > Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
> > > >
> > > > ----
> > > >
> > > > The drawback with this approach is that we are adding 6 new
> > > > page-table (3 for boot and 3 for runtime) that are statically
> > > > allocated. So the final Xen binary will be 24KB bigger when
> > > > neither UBSAN nor GCOV.
> > > >
> > > > If this is not considered acceptable, then we could make the
> > > > size of configurable in the Kconfig and decide it based on the
> > > > features enabled.
> > > Both of these features are enabled only in a debug build of Xen, so
> > > another option would be to increase Xen size only for a debug build.
> > At least UBSAN can be selected without DEBUG. For that you need to add
> > EXPERT.
> >
> > Anyway, from your comment, it is not clear to me whether you dislike the
> > existing approach (and why) or you are OK with the increase.
> Sorry, I was traveling and did not have time to complete review.
> I cannot see why increasing the size would be problematic so it is ok. That 
> said, it could be
> a good idea to write some comment above XEN_VIRT_SIZE, that this value is 
> somewhat exaggerated,
> so that we are on the safe side at the time of activating features like 
> UBSAN/GCOV.
Sure. I will add a comment in this patch. For ...

> Also, it would be nice to update comments in head.S of both arm32 and arm64 
> above
> GLOBAL(start) that were left stating:
> "All of text+data+bss must fit in 2MB, or the initial pagetable code below will need 
> adjustment."
... this one, I am thinking to drop the comments in patch #5. They don't 
make sense anymore as we can now in theory deal with Xen up to the size 
of a L2 mapping (1GB for 4KB).
> Other than that:
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
Thanks.

Cheers,

--
Julien Grall