Re: [PATCH v4 1/4] x86/livepatch: align functions to ensure minimal distance between entry points

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Dec 20, 2023 at 10:03:51AM +0100, Jan Beulich wrote:
> On 20.12.2023 09:32, Roger Pau Monné wrote:
> > On Tue, Dec 19, 2023 at 07:46:11PM +0000, Andrew Cooper wrote:
> >> 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?
> > 
> > I would expect that for gcc I guess.
> > 
> >> What happens on ARM, which also picks up an explicit choice in livepatch
> >> builds?
> > 
> > Arm AFAICT seems to use a 4 byte function alignment with -O2 (both gcc
> > and clang), so that matches what we need to enforce for livepatch.  If
> > we ever need a higher alignment for livepatch reasons it would be a
> > multiple of the minimum one set by the compiler, so that should be
> > fine.
> 
> Thinking of it: The forcing of 16-byte alignment in release builds of x86
> is based on observations with certain compiler versions, iirc. What if
> future versions decide to go lower/higher for, perhaps, very good reasons?
> We don't really mean to override the compiler's choice, so maybe further
> probing is actually necessary?

We do override the default (on gcc) when using livepatch anyway, so we
might as well be consistent and attempt to provide a value that's
reasonable?

Linux currently uses 16-byte also on x86 and set in Kconfig, and all
compiler versions I've tested use 16-bytes with -O2, so I think it's
unlikely to change overnight (and a lot of software will still use 16
anyway).

If a more suitable value is needed in the future we can always adjust,
that's the whole point of having it in Kconfig.  We can also select a
better value even in compilers that might not know about it.

Thanks, Roger.