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

[Julien Grall <julien@xxxxxxx>]

On 26/06/2023 08:29, Henry Wang wrote:
> Hi Julien,
Hi Henry,


> > -----Original Message-----
> > From: Julien Grall <julien@xxxxxxx>
> > Subject: [PATCH 8/9] xen/arm: Allow the user to build Xen with USBAN
> Typo in title: s/USBAN/UBSAN/ and...
>
> > 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
> ...here also.
>
> > 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>
> Both typos can be fixed on commit and I think I am good with the
> current approach, so:
>
> Reviewed-by: Henry Wang <Henry.Wang@xxxxxxx>
Thanks, I will fix both typos.

> I did a play with UBSAN enabled on FVP (arm64), in my setup,
> the result Xen file is 3.7MB:
>
> -rwxrwxr-x  1 xinwan02 xinwan02 3.7M Jun 26 14:47 xen
> lrwxrwxrwx  1 xinwan02 xinwan02    3 Jun 26 14:47 xen.efi -> xen
> -rwxrwxr-x  1 xinwan02 xinwan02  14M Jun 26 14:47 xen-syms
>
> and the Xen and Dom0 booted fine and I can login Dom0. So
> technically:
>
> Tested-by: Henry Wang <Henry.Wang@xxxxxxx>
>
> However, I noticed Xen will print below during the Dom0 boot:
> (XEN) 
> ================================================================================
> (XEN) UBSAN: Undefined behaviour in arch/arm/vgic.c:371:15
> (XEN) left shift of 1 by 31 places cannot be represented in type 'int'
> (XEN) Xen WARN at common/ubsan/ubsan.c:172
>
> Just want to make sure you also noticed this, otherwise maybe you
> can include another patch in the series to fix this? 
I haven't seen this one. Probably because you would need a setup where 
interrupts are around ID 1022.
Or I can do that
> in case you don't have enough bandwidth.
You have the setup to exercise the problem. So it would be best if you 
do it.
Cheers,

--
Julien Grall