| Message ID | 20230629201129.12934-5-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: Enable USBAN support | expand |
Hi Julien, > -----Original Message----- > Subject: [v2 4/4] xen/arm: Allow the user to build Xen with UBSAN > > From: Julien Grall <jgrall@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 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@amazon.com> Reviewed-by: Henry Wang <Henry.Wang@arm.com> Kind regards, Henry
On 29/06/2023 22:11, Julien Grall wrote: > > > From: Julien Grall <jgrall@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 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@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. > > Changes in v2: > - Fix typoes > - Add Michal's reviewed-by tag I cannot see one, so: Reviewed-by: Michal Orzel <michal.orzel@amd.com> ~Michal
Hi Julien, > On 29 Jun 2023, at 22:11, Julien Grall <julien@xen.org> wrote: > > From: Julien Grall <jgrall@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 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@amazon.com> I am ok with the drawback. Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> 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 > >
Hi Michal, On 30/06/2023 07:58, Michal Orzel wrote: > > > On 29/06/2023 22:11, Julien Grall wrote: >> >> >> From: Julien Grall <jgrall@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 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@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. >> >> Changes in v2: >> - Fix typoes >> - Add Michal's reviewed-by tag > I cannot see one, so: Hmmmm, you gave it on v1 but I can't remember why I didn't carry it. Anyway... > Reviewed-by: Michal Orzel <michal.orzel@amd.com> Thanks! Cheers,
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)