Re: [PATCH v6 3/3] xen/livepatch: align functions to ensure minimal distance between entry points

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

On Tue, Feb 13, 2024 at 04:58:38PM +0100, Jan Beulich wrote:
> On 07.02.2024 15:55, Roger Pau Monne wrote:
> > The minimal function size requirements for an x86 livepatch are either 5 
> > bytes
> > (for jmp) or 9 bytes (for endbr + jmp), 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 scripts, 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.
> 
> Perhaps "... minimal required size"?

Yes.

> > --- a/xen/common/Kconfig
> > +++ b/xen/common/Kconfig
> > @@ -395,8 +395,11 @@ config CRYPTO
> >  config LIVEPATCH
> >     bool "Live patching support"
> >     default X86
> > -   depends on "$(XEN_HAS_BUILD_ID)" = "y"
> > +   depends on "$(XEN_HAS_BUILD_ID)" = "y" && CC_HAS_FUNCTION_ALIGNMENT
> >     select CC_SPLIT_SECTIONS
> > +   select FUNCTION_ALIGNMENT_16B if XEN_IBT
> > +   select FUNCTION_ALIGNMENT_8B  if X86
> > +   select FUNCTION_ALIGNMENT_4B  if ARM
> 
> This isn't strictly needed, is it? Would be nice to avoid re-selection
> of what the default for an arch is anyway, as otherwise this will start
> looking clumsy when a couple more architectures are added.

My worry was that the default per-arch could change, ie: for example
x86 moving from 16 to 8 and then it would hamper livepatch support if
IBT is also enabled.  I however think it's very unlikely to reduce the
default alignment, and in any case we would hit a build time assert if
that ever happens.

So yes, I'm fine with dropping those.

> Preferably
> with that dropped (or it being clarified why it's still desirable to
> have):
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks, Roger.