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

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

On Tue, Dec 05, 2023 at 04:14:57PM +0100, Jan Beulich wrote:
> On 05.12.2023 16:01, Roger Pau Monné wrote:
> > On Tue, Dec 05, 2023 at 01:42:42PM +0000, Andrew Cooper wrote:
> >> On 28/11/2023 10:03 am, Roger Pau Monne wrote:
> >>> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> >>> index f3abdf9cd111..f629157086d0 100644
> >>> --- a/xen/arch/x86/Makefile
> >>> +++ b/xen/arch/x86/Makefile
> >>> @@ -82,6 +82,8 @@ obj-$(CONFIG_COMPAT) += x86_64/platform_hypercall.o
> >>>  obj-y += sysctl.o
> >>>  endif
> >>>  
> >>> +CFLAGS-$(CONFIG_LIVEPATCH) += 
> >>> -falign-functions=$(CONFIG_CC_FUNCTION_ALIGNMENT)
> >>
> >> I'd really prefer not to express it like this.  For one, a major reason
> >> for using an alignment of 16b or more is simply performance.
> >>
> >> Also, it isn't "CC" when we get the asm macros working.
> >>
> >> Copy Linux more closely.  Then, you have LIVEPATCH select
> >> FUNCTION_ALIGNMENT_{8,16}B as appropriate.  And PERFORMANCE selects
> >> FUNCTION_ALIGNMENT_16B or perhaps 32B depending on uarch.
> > 
> > So just use CONFIG_FUNCTION_ALIGNMENT and drop the CC part of it?
> > That would indeed be fine.  We will also need to adjust
> > CC_SPLIT_SECTIONS to drop the CC_ prefix when we start using it in
> > assembly code.
> 
> Could we prune the CC infixes once everything is settled asm-code-wise?

That would also be fine by me.

> >> If we ever get around to having KCFI, then we need 16B irrespective of
> >> anything else.
> >>
> >>
> >>
> >> As for the subject, it's not really about size; the function size is
> >> still going to be small irrespective of the alignment.
> > 
> > What about wording it like:
> > 
> > x86/livepatch: set function alignment to ensure minimal space between 
> > functions
> 
> This still wouldn't be right, as there may be no padding at all between
> functions (if they're just the right size).

But no padding would still be fine given the text above, as then the
minimal space requirement is already meet?

> Maybe "minimal distance
> between function entry points"? Getting long-ish, though ...

Oh, I see.  You want to explicitly mention the distance is between
function entry points, as otherwise one way to read the subject would
be distance between function end and next function entry point?

It's indeed a bit long for my taste, but I don't mind adjusting if you
think the current wording could cause confusion.

Thanks, Roger.