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

[Michal Orzel <michal.orzel@xxxxxxx>]

On 28/06/2023 22:39, Julien Grall 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 <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).
Seems right.

~Michal