On 15/12/2023 11:18 am, Roger Pau Monne wrote: > The minimal function size requirements for livepatch are either 5 bytes (for
"for an x86 livepatch", seeing as we're touching multiple architectures worth of files. I know it's at the end of the sentence, but it wants to be earlier to be clearer. > jmp) or 9 bytes (for endbr + jmp) on x86, and always 4 bytes on Arm. Ensure > that distance between functions entry points is always at least of the minimal > required size for livepatch instruction replacement to be successful. > > Add an additional align directive to the linker script, in order to ensure > that > the next section placed after the .text.* (per-function sections) is also > aligned to the required boundary, so that the distance of the last function > entry point with the next symbol is also of minimal size. > > Note that it's possible for the compiler to end up using a higher function > alignment regardless of the passed value, so this change just make sure that > the minimum required for livepatch to work is present. Different compilers > handle the option differently, as clang will ignore -falign-functions value > if it's smaller than the one that would be set by the optimization level, > while > gcc seems to always honor the function alignment passed in -falign-functions. > In order to cope with this behavior and avoid that setting -falign-functions > results in an alignment inferior to what the optimization level would have > selected force x86 release builds to use a function alignment of 16 bytes. Yuck :( The same will be true for all other architectures too? What happens on ARM, which also picks up an explicit choice in livepatch builds? > > The compiler option -falign-functions is not available on at least clang 3.8, > so introduce a Kconfig check for it and make the livepatch option depend on > the > compiler supporting the option. > > The naming of the option(s) CONFIG_FUNCTION_ALIGNMENT is explicitly not > mentioning CC in preparation for the option also being used by assembly code. > > Signed-off-by: Roger Pau Monné <[email protected]> > --- > Changes since v3: > - Test for compiler option with -falign-functions. > - Make FUNCTION_ALIGNMENT depend on CC_HAS_FUNCTION_ALIGNMENT. > - Set 16byte function alignment for x86 release builds. > > Changes since v2: > - Add Arm side. > - Align end of section in the linker script to ensure enough padding for the > last function. > - Expand commit message and subject. > - Rework Kconfig options. > - Check that the compiler supports the option. > > Changes since v1: > - New in this version. > --- > xen/Kconfig | 19 +++++++++++++++++++ > xen/Makefile | 3 +++ > xen/arch/arm/livepatch.c | 2 ++ > xen/arch/arm/xen.lds.S | 4 ++++ > xen/arch/x86/Kconfig | 1 + > xen/arch/x86/livepatch.c | 4 ++++ > xen/arch/x86/xen.lds.S | 4 ++++ > xen/common/Kconfig | 5 ++++- > 8 files changed, 41 insertions(+), 1 deletion(-) xen$ git ls-files | grep xen.lds.S arch/arm/xen.lds.S arch/ppc/xen.lds.S arch/riscv/xen.lds.S arch/x86/xen.lds.S RISC-V and PPC have the same pattern that you're patching for x86 and ARM. > diff --git a/xen/Kconfig b/xen/Kconfig > index 134e6e68ad84..c2cc3fe165eb 100644 > --- a/xen/Kconfig > +++ b/xen/Kconfig > @@ -37,6 +37,25 @@ config CC_HAS_VISIBILITY_ATTRIBUTE > config CC_SPLIT_SECTIONS > bool > > +# Set function alignment. > +# > +# Allow setting on a boolean basis, and then convert such selection to an > +# integer for the build system and code to consume more easily. # Clang >= 6.0 > +config CC_HAS_FUNCTION_ALIGNMENT > + def_bool $(cc-option,-falign-functions) > +config FUNCTION_ALIGNMENT_4B > + bool > +config FUNCTION_ALIGNMENT_8B > + bool > +config FUNCTION_ALIGNMENT_16B > + bool > +config FUNCTION_ALIGNMENT > + int > + depends on CC_HAS_FUNCTION_ALIGNMENT > + default 16 if FUNCTION_ALIGNMENT_16B > + default 8 if FUNCTION_ALIGNMENT_8B > + default 4 if FUNCTION_ALIGNMENT_4B What value do we get here for RISCV/PPC? Do we need another override for them? ~Andrew
