Re: [v2 4/4] xen/arm: Allow the user to build Xen with UBSAN

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Julien,

> On 29 Jun 2023, at 22:11, Julien Grall <julien@xxxxxxx> 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 UBSAN 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>

I am ok with the drawback.

Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Cheers
Bertrand

> 
> ---
> 
> 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.
> 
>    Changes in v2:
>        - Fix typoes
>        - Add Michal's reviewed-by tag
>        - Add Henry's  reviewed-by tag
>        - Add a comment regarding the reserved size for Xen
> ---
> xen/arch/arm/Kconfig              |  1 +
> xen/arch/arm/include/asm/config.h | 22 +++++++++++++---------
> 2 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 61e581b8c2b0..06b5ff755c95 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -17,6 +17,7 @@ config ARM
> select HAS_PASSTHROUGH
> select HAS_PDX
> select HAS_PMAP
> + select HAS_UBSAN
> select IOMMU_FORCE_PT_SHARE
> 
> config ARCH_DEFCONFIG
> diff --git a/xen/arch/arm/include/asm/config.h 
> b/xen/arch/arm/include/asm/config.h
> index 6d246ab23c48..cc32802ad0e9 100644
> --- a/xen/arch/arm/include/asm/config.h
> +++ b/xen/arch/arm/include/asm/config.h
> @@ -74,10 +74,10 @@
> /*
>  * ARM32 layout:
>  *   0  -   2M   Unmapped
> - *   2M -   4M   Xen text, data, bss
> - *   4M -   6M   Fixmap: special-purpose 4K mapping slots
> - *   6M -  10M   Early boot mapping of FDT
> - *   10M - 12M   Livepatch vmap (if compiled in)
> + *   2M -  10M   Xen text, data, bss
> + *  10M -  12M   Fixmap: special-purpose 4K mapping slots
> + *  12M -  16M   Early boot mapping of FDT
> + *  16M -  18M   Livepatch vmap (if compiled in)
>  *
>  *  32M - 128M   Frametable: 32 bytes per page for 12GB of RAM
>  * 256M -   1G   VMAP: ioremap and early_ioremap use this virtual address
> @@ -94,10 +94,10 @@
>  * 0x0000020000000000 - 0x0000027fffffffff (512GB, L0 slot [4])
>  *  (Relative offsets)
>  *   0  -   2M   Unmapped
> - *   2M -   4M   Xen text, data, bss
> - *   4M -   6M   Fixmap: special-purpose 4K mapping slots
> - *   6M -  10M   Early boot mapping of FDT
> - *  10M -  12M   Livepatch vmap (if compiled in)
> + *   2M -  10M   Xen text, data, bss
> + *  10M -  12M   Fixmap: special-purpose 4K mapping slots
> + *  12M -  16M   Early boot mapping of FDT
> + *  16M -  18M   Livepatch vmap (if compiled in)
>  *
>  *   1G -   2G   VMAP: ioremap and early_ioremap
>  *
> @@ -124,7 +124,11 @@
> #define XEN_VIRT_START          (SLOT0(4) + _AT(vaddr_t, MB(2)))
> #endif
> 
> -#define XEN_VIRT_SIZE           _AT(vaddr_t, MB(2))
> +/*
> + * Reserve enough space so both UBSAN and GCOV can be enabled together
> + * plus some slack for future growth.
> + */
> +#define XEN_VIRT_SIZE           _AT(vaddr_t, MB(8))
> #define XEN_NR_ENTRIES(lvl)     (XEN_VIRT_SIZE / XEN_PT_LEVEL_SIZE(lvl))
> 
> #define FIXMAP_VIRT_START       (XEN_VIRT_START + XEN_VIRT_SIZE)
> -- 
> 2.40.1
> 
>