> On 28 Jun 2023, at 21:39, Julien Grall <jul...@xen.org> wrote: > > > > 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 <jgr...@amazon.com> >>>>> >>>>> 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 <jgr...@amazon.com> >>>>> >>>>> ---- >>>>> >>>>> 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.or...@amd.com> > > Thanks. > > Cheers, > > -- > Julien Grall
Hi Julien, Don’t know if it was pointed out before, but the commit message has a typo s/USBAN/UBSAN/ Cheers, Luca
