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

[Michal Orzel <michal.orzel@xxxxxxx>]

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.

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."

Other than that:
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

~Michal