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

[Jan Beulich <jbeulich@xxxxxxxx>]

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?

>> 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). Maybe "minimal distance
between function entry points"? Getting long-ish, though ...

Jan