Re: [PATCH v2 1/5] x86/livepatch: set function alignment to ensure minimal function size

["Roger Pau Monné" <roger.pau@citrix.com>]

On Fri, Dec 01, 2023 at 10:41:45AM +0100, Jan Beulich wrote:
> On 01.12.2023 09:50, Roger Pau Monné wrote:
> > On Fri, Dec 01, 2023 at 07:53:29AM +0100, Jan Beulich wrote:
> >> On 30.11.2023 18:37, Roger Pau Monné wrote:
> >>> On Thu, Nov 30, 2023 at 05:55:07PM +0100, Jan Beulich wrote:
> >>>> On 28.11.2023 11:03, Roger Pau Monne wrote:
> >>>>> The minimal function size requirements for livepatch are either 5 bytes (for
> >>>>> jmp) or 9 bytes (for endbr + jmp).  Ensure that functions are always at least
> >>>>> that size by requesting the compiled to align the functions to 8 or 16 bytes,
> >>>>> depending on whether Xen is build with IBT support.
> >>>>
> >>>> How is alignment going to enforce minimum function size? If a function is
> >>>> last in a section, there may not be any padding added (ahead of linking at
> >>>> least). The trailing padding also isn't part of the function.
> >>>
> >>> If each function lives in it's own section (by using
> >>> -ffunction-sections), and each section is aligned, then I think we can
> >>> guarantee that there will always be enough padding space?
> >>>
> >>> Even the last function/section on the .text block would still be
> >>> aligned, and as long as the function alignment <= SECTION_ALIGN
> >>> there will be enough padding left.  I should add some build time
> >>> assert that CONFIG_CC_FUNCTION_ALIGNMENT <= SECTION_ALIGN.
> >>
> >> I'm not sure of there being a requirement for a section to be padded to
> >> its alignment. If the following section has smaller alignment, it could
> >> be made start earlier. Of course our linker scripts might guarantee
> >> this ...
> > 
> > I do think so, given our linker script arrangements for the .text
> > section:
> > 
> > DECL_SECTION(.text) {
> >     [...]
> > } PHDR(text) = 0x9090
> > 
> > . = ALIGN(SECTION_ALIGN);
> > 
> > The end of the text section is aligned to SECTION_ALIGN, so as long as
> > SECTION_ALIGN >= CONFIG_CC_FUNCTION_ALIGNMENT the alignment should
> > guarantee a minimal function size.
> > 
> > Do you think it would be clearer if I add the following paragraph:
> > 
> > "Given the Xen linker script arrangement of the .text section, we can
> > ensure that when all functions are aligned to the given boundary the
> > function size will always be a multiple of such alignment, even for
> > the last function in .text, as the linker script aligns the end of the
> > section to SECTION_ALIGN."
> 
> I think this would be useful to have there. Beyond that, assembly code
> also needs considering btw.

Assembly will get dealt with once we start to also have separate
sections for each assembly function.  We cannot patch assembly code at
the moment anyway, due to lack of debug symbols.

Thanks, Roger.