diff mbox series

[v5] x86/livepatch: align functions to ensure minimal distance between entry points

Message ID 20240122110244.14091-1-roger.pau@citrix.com (mailing list archive)
State New, archived
Headers show
Series [v5] x86/livepatch: align functions to ensure minimal distance between entry points | expand

Commit Message

Roger Pau Monné Jan. 22, 2024, 11:02 a.m. UTC
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 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.
For Arm the default compiler selection of function alignment matches the
requirements of livepatch, which are 4 bytes.

The compiler option -falign-functions is not available on at least clang 3.8,
so introduce a Kconfig check for it and make the livepatch option depend on the
compiler supporting the option.

The naming of the option(s) CONFIG_FUNCTION_ALIGNMENT is explicitly not
mentioning CC in preparation for the option also being used by assembly code.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v4:
 - Split from the rest of the livepatch testing series.
 - Reword and expand a bit the commit message.
 - Add a comment about falign-functions clang version requirement.

Changes since v3:
 - Test for compiler option with -falign-functions.
 - Make FUNCTION_ALIGNMENT depend on CC_HAS_FUNCTION_ALIGNMENT.
 - Set 16byte function alignment for x86 release builds.

Changes since v2:
 - Add Arm side.
 - Align end of section in the linker script to ensure enough padding for the
   last function.
 - Expand commit message and subject.
 - Rework Kconfig options.
 - Check that the compiler supports the option.

Changes since v1:
 - New in this version.
---
 xen/Kconfig              | 21 +++++++++++++++++++++
 xen/Makefile             |  3 +++
 xen/arch/arm/livepatch.c |  2 ++
 xen/arch/arm/xen.lds.S   |  4 ++++
 xen/arch/x86/Kconfig     |  1 +
 xen/arch/x86/livepatch.c |  4 ++++
 xen/arch/x86/xen.lds.S   |  4 ++++
 xen/common/Kconfig       |  5 ++++-
 8 files changed, 43 insertions(+), 1 deletion(-)

Comments

Jan Beulich Jan. 22, 2024, 11:21 a.m. UTC | #1
On 22.01.2024 12:02, 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 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.

Nit: A comma after "selected" may help readability.

> --- a/xen/Kconfig
> +++ b/xen/Kconfig
> @@ -37,6 +37,27 @@ config CC_HAS_VISIBILITY_ATTRIBUTE
>  config CC_SPLIT_SECTIONS
>  	bool
>  
> +# Set function alignment.
> +#
> +# Allow setting on a boolean basis, and then convert such selection to an
> +# integer for the build system and code to consume more easily.
> +#
> +# Requires clang >= 7.0.0
> +config CC_HAS_FUNCTION_ALIGNMENT
> +	def_bool $(cc-option,-falign-functions)

Nit: Maybe better have a blank line here?

> +config FUNCTION_ALIGNMENT_4B
> +	bool
> +config FUNCTION_ALIGNMENT_8B
> +	bool
> +config FUNCTION_ALIGNMENT_16B
> +	bool
> +config FUNCTION_ALIGNMENT
> +	int
> +	depends on CC_HAS_FUNCTION_ALIGNMENT
> +	default 16 if FUNCTION_ALIGNMENT_16B
> +	default  8 if  FUNCTION_ALIGNMENT_8B
> +	default  4 if  FUNCTION_ALIGNMENT_4B
> +
>  source "arch/$(SRCARCH)/Kconfig"
>  
>  config DEFCONFIG_LIST
>[...]
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -99,6 +99,10 @@ SECTIONS
>         *(.text)
>  #ifdef CONFIG_CC_SPLIT_SECTIONS
>         *(.text.*)
> +#endif
> +#ifdef CONFIG_FUNCTION_ALIGNMENT
> +       /* Ensure enough distance with the next placed section. */
> +       . = ALIGN(CONFIG_FUNCTION_ALIGNMENT);
>  #endif
>         *(.text.__x86_indirect_thunk_*)

I continue to fail to see how an alignment directive can guarantee minimum
distance. In the worst case such a directive inserts nothing at all. IOW
at the very least there's a non-spelled-out assumption here about the last
item in the earlier section having suitable alignment and thus, if small
in size, being suitably padded. Personally I don't think merely spelling
out such a requirement would help - it would end up being a trap for
someone to fall into.

I'm further curious why .text.__x86_indirect_thunk_* is left past the
inserted alignment. While pretty unlikely, isn't it in principle possible
for the thunks there to also need patching? Aren't we instead requiring
then that assembly functions (and thunks) all be suitably aligned as well?

Jan
Roger Pau Monné Jan. 22, 2024, 5:27 p.m. UTC | #2
On Mon, Jan 22, 2024 at 12:21:47PM +0100, Jan Beulich wrote:
> On 22.01.2024 12:02, 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 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.
> 
> Nit: A comma after "selected" may help readability.
> 
> > --- a/xen/Kconfig
> > +++ b/xen/Kconfig
> > @@ -37,6 +37,27 @@ config CC_HAS_VISIBILITY_ATTRIBUTE
> >  config CC_SPLIT_SECTIONS
> >  	bool
> >  
> > +# Set function alignment.
> > +#
> > +# Allow setting on a boolean basis, and then convert such selection to an
> > +# integer for the build system and code to consume more easily.
> > +#
> > +# Requires clang >= 7.0.0
> > +config CC_HAS_FUNCTION_ALIGNMENT
> > +	def_bool $(cc-option,-falign-functions)
> 
> Nit: Maybe better have a blank line here?
> 
> > +config FUNCTION_ALIGNMENT_4B
> > +	bool
> > +config FUNCTION_ALIGNMENT_8B
> > +	bool
> > +config FUNCTION_ALIGNMENT_16B
> > +	bool
> > +config FUNCTION_ALIGNMENT
> > +	int
> > +	depends on CC_HAS_FUNCTION_ALIGNMENT
> > +	default 16 if FUNCTION_ALIGNMENT_16B
> > +	default  8 if  FUNCTION_ALIGNMENT_8B
> > +	default  4 if  FUNCTION_ALIGNMENT_4B
> > +
> >  source "arch/$(SRCARCH)/Kconfig"
> >  
> >  config DEFCONFIG_LIST
> >[...]
> > --- a/xen/arch/x86/xen.lds.S
> > +++ b/xen/arch/x86/xen.lds.S
> > @@ -99,6 +99,10 @@ SECTIONS
> >         *(.text)
> >  #ifdef CONFIG_CC_SPLIT_SECTIONS
> >         *(.text.*)
> > +#endif
> > +#ifdef CONFIG_FUNCTION_ALIGNMENT
> > +       /* Ensure enough distance with the next placed section. */
> > +       . = ALIGN(CONFIG_FUNCTION_ALIGNMENT);
> >  #endif
> >         *(.text.__x86_indirect_thunk_*)
> 
> I continue to fail to see how an alignment directive can guarantee minimum
> distance. In the worst case such a directive inserts nothing at all.

I'm confused, you did provide a RB for this in v4:

https://lore.kernel.org/xen-devel/4cad003f-dda0-4e22-a770-5a5ff56f4d35@suse.com/

Which is basically the same code with a few comments and wording
adjustments.

> IOW
> at the very least there's a non-spelled-out assumption here about the last
> item in the earlier section having suitable alignment and thus, if small
> in size, being suitably padded.

Please bear with me, but I'm afraid I don't understand your concerns.

For livepatch build tools (which is the only consumer of such
alignments) we already have the requirement that a function in order
to be suitable for being live patched must reside in it's own
section.

We do want to aim for functions (even assembly ones) to live in their
own sections in order to be live patched, and to be properly aligned.
However it's also fine for functions to use a different (smaller)
alignment, the livepatch build tools will detect this and use the
alignment reported.

While we want to get to a point where everything that we care to patch
lives in it's own section, and is properly padded to ensure minimal
required space, I don't see why the proposed approach here should be
blocked, as it's a step in the right direction of achieving the
goal.

Granted, there's still assembly code that won't be suitably padded,
but the livepatch build tools won't assume it to be padded.  After
your series to enable assembly annotations we can also make sure the
assembly annotated functions live in separate sections and are
suitably aligned.

> Personally I don't think merely spelling
> out such a requirement would help - it would end up being a trap for
> someone to fall into.

> I'm further curious why .text.__x86_indirect_thunk_* is left past the
> inserted alignment. While pretty unlikely, isn't it in principle possible
> for the thunks there to also need patching? Aren't we instead requiring
> then that assembly functions (and thunks) all be suitably aligned as well?

Those are defined in assembly, so requires CONFIG_FUNCTION_ALIGNMENT
to also be applied to the function entry points in assembly files.

Thanks, Roger.
Jan Beulich Jan. 23, 2024, 7:53 a.m. UTC | #3
On 22.01.2024 18:27, Roger Pau Monné wrote:
> On Mon, Jan 22, 2024 at 12:21:47PM +0100, Jan Beulich wrote:
>> On 22.01.2024 12:02, Roger Pau Monne wrote:
>>> --- a/xen/arch/x86/xen.lds.S
>>> +++ b/xen/arch/x86/xen.lds.S
>>> @@ -99,6 +99,10 @@ SECTIONS
>>>         *(.text)
>>>  #ifdef CONFIG_CC_SPLIT_SECTIONS
>>>         *(.text.*)
>>> +#endif
>>> +#ifdef CONFIG_FUNCTION_ALIGNMENT
>>> +       /* Ensure enough distance with the next placed section. */
>>> +       . = ALIGN(CONFIG_FUNCTION_ALIGNMENT);
>>>  #endif
>>>         *(.text.__x86_indirect_thunk_*)
>>
>> I continue to fail to see how an alignment directive can guarantee minimum
>> distance. In the worst case such a directive inserts nothing at all.
> 
> I'm confused, you did provide a RB for this in v4:
> 
> https://lore.kernel.org/xen-devel/4cad003f-dda0-4e22-a770-5a5ff56f4d35@suse.com/
> 
> Which is basically the same code with a few comments and wording
> adjustments.

Hmm, yes. I think the aspect above was raised before, but then (perhaps)
kind of addressed. (I'm puzzled then too: Why did you drop the R-b, when
nothing substantially changed?) Yet re-reading the description, there's
nothing said to this effect. Specifically ...

>> IOW
>> at the very least there's a non-spelled-out assumption here about the last
>> item in the earlier section having suitable alignment and thus, if small
>> in size, being suitably padded.
> 
> Please bear with me, but I'm afraid I don't understand your concerns.
> 
> For livepatch build tools (which is the only consumer of such
> alignments) we already have the requirement that a function in order
> to be suitable for being live patched must reside in it's own
> section.
> 
> We do want to aim for functions (even assembly ones) to live in their
> own sections in order to be live patched, and to be properly aligned.
> However it's also fine for functions to use a different (smaller)
> alignment, the livepatch build tools will detect this and use the
> alignment reported.

... I don't think this and ...

> While we want to get to a point where everything that we care to patch
> lives in it's own section, and is properly padded to ensure minimal
> required space, I don't see why the proposed approach here should be
> blocked, as it's a step in the right direction of achieving the
> goal.
> 
> Granted, there's still assembly code that won't be suitably padded,
> but the livepatch build tools won't assume it to be padded.

... this is being pointed out. Which I think is relevant to make
explicit not the least because the build tools aren't part of the main
Xen tree. Plus many (like me) may not be overly familiar with how they
work.

>  After
> your series to enable assembly annotations we can also make sure the
> assembly annotated functions live in separate sections and are
> suitably aligned.
> 
>> Personally I don't think merely spelling
>> out such a requirement would help - it would end up being a trap for
>> someone to fall into.
> 
>> I'm further curious why .text.__x86_indirect_thunk_* is left past the
>> inserted alignment. While pretty unlikely, isn't it in principle possible
>> for the thunks there to also need patching? Aren't we instead requiring
>> then that assembly functions (and thunks) all be suitably aligned as well?
> 
> Those are defined in assembly, so requires CONFIG_FUNCTION_ALIGNMENT
> to also be applied to the function entry points in assembly files.

I see. Yet the question then remains: Why is the alignment not inserted
after them? Or will the insertion need to move later on (which would feel
odd)?

Jan
Roger Pau Monné Jan. 23, 2024, 9:32 a.m. UTC | #4
On Tue, Jan 23, 2024 at 08:53:15AM +0100, Jan Beulich wrote:
> On 22.01.2024 18:27, Roger Pau Monné wrote:
> > On Mon, Jan 22, 2024 at 12:21:47PM +0100, Jan Beulich wrote:
> >> On 22.01.2024 12:02, Roger Pau Monne wrote:
> >>> --- a/xen/arch/x86/xen.lds.S
> >>> +++ b/xen/arch/x86/xen.lds.S
> >>> @@ -99,6 +99,10 @@ SECTIONS
> >>>         *(.text)
> >>>  #ifdef CONFIG_CC_SPLIT_SECTIONS
> >>>         *(.text.*)
> >>> +#endif
> >>> +#ifdef CONFIG_FUNCTION_ALIGNMENT
> >>> +       /* Ensure enough distance with the next placed section. */
> >>> +       . = ALIGN(CONFIG_FUNCTION_ALIGNMENT);
> >>>  #endif
> >>>         *(.text.__x86_indirect_thunk_*)
> >>
> >> I continue to fail to see how an alignment directive can guarantee minimum
> >> distance. In the worst case such a directive inserts nothing at all.
> > 
> > I'm confused, you did provide a RB for this in v4:
> > 
> > https://lore.kernel.org/xen-devel/4cad003f-dda0-4e22-a770-5a5ff56f4d35@suse.com/
> > 
> > Which is basically the same code with a few comments and wording
> > adjustments.
> 
> Hmm, yes. I think the aspect above was raised before, but then (perhaps)
> kind of addressed. (I'm puzzled then too: Why did you drop the R-b, when
> nothing substantially changed?)

The RB was given quite some time ago, so I felt it was probably best
to drop it in case you wanted to re-asses the patch.  Specially given
you have now done the work to also add support for this feature to
assembly annotated functions.

> Yet re-reading the description, there's
> nothing said to this effect. Specifically ...
> 
> >> IOW
> >> at the very least there's a non-spelled-out assumption here about the last
> >> item in the earlier section having suitable alignment and thus, if small
> >> in size, being suitably padded.
> > 
> > Please bear with me, but I'm afraid I don't understand your concerns.
> > 
> > For livepatch build tools (which is the only consumer of such
> > alignments) we already have the requirement that a function in order
> > to be suitable for being live patched must reside in it's own
> > section.
> > 
> > We do want to aim for functions (even assembly ones) to live in their
> > own sections in order to be live patched, and to be properly aligned.
> > However it's also fine for functions to use a different (smaller)
> > alignment, the livepatch build tools will detect this and use the
> > alignment reported.
> 
> ... I don't think this and ...
> 
> > While we want to get to a point where everything that we care to patch
> > lives in it's own section, and is properly padded to ensure minimal
> > required space, I don't see why the proposed approach here should be
> > blocked, as it's a step in the right direction of achieving the
> > goal.
> > 
> > Granted, there's still assembly code that won't be suitably padded,
> > but the livepatch build tools won't assume it to be padded.
> 
> ... this is being pointed out. Which I think is relevant to make
> explicit not the least because the build tools aren't part of the main
> Xen tree. Plus many (like me) may not be overly familiar with how they
> work.

OK, I can integrate some of this wording in the commit message.

> >  After
> > your series to enable assembly annotations we can also make sure the
> > assembly annotated functions live in separate sections and are
> > suitably aligned.
> > 
> >> Personally I don't think merely spelling
> >> out such a requirement would help - it would end up being a trap for
> >> someone to fall into.
> > 
> >> I'm further curious why .text.__x86_indirect_thunk_* is left past the
> >> inserted alignment. While pretty unlikely, isn't it in principle possible
> >> for the thunks there to also need patching? Aren't we instead requiring
> >> then that assembly functions (and thunks) all be suitably aligned as well?
> > 
> > Those are defined in assembly, so requires CONFIG_FUNCTION_ALIGNMENT
> > to also be applied to the function entry points in assembly files.
> 
> I see. Yet the question then remains: Why is the alignment not inserted
> after them? Or will the insertion need to move later on (which would feel
> odd)?

The thunk sections will currently be consumed by *(.text.*) when using
split sections.  Looking at the assembly for them I think they are
suitable annotated to create the right symbols for livepatch tools to
pick.  They won't however have the right alignment just yet, as I
expect that will get solved with your follow up patch to respect
CONFIG_FUNCTION_ALIGNMENT in assembly annotated functions also.

Thanks, Roger.
Jan Beulich Jan. 23, 2024, 9:39 a.m. UTC | #5
On 23.01.2024 10:32, Roger Pau Monné wrote:
> On Tue, Jan 23, 2024 at 08:53:15AM +0100, Jan Beulich wrote:
>> On 22.01.2024 18:27, Roger Pau Monné wrote:
>>> On Mon, Jan 22, 2024 at 12:21:47PM +0100, Jan Beulich wrote:
>>>> I'm further curious why .text.__x86_indirect_thunk_* is left past the
>>>> inserted alignment. While pretty unlikely, isn't it in principle possible
>>>> for the thunks there to also need patching? Aren't we instead requiring
>>>> then that assembly functions (and thunks) all be suitably aligned as well?
>>>
>>> Those are defined in assembly, so requires CONFIG_FUNCTION_ALIGNMENT
>>> to also be applied to the function entry points in assembly files.
>>
>> I see. Yet the question then remains: Why is the alignment not inserted
>> after them? Or will the insertion need to move later on (which would feel
>> odd)?
> 
> The thunk sections will currently be consumed by *(.text.*) when using
> split sections.  Looking at the assembly for them I think they are
> suitable annotated to create the right symbols for livepatch tools to
> pick.  They won't however have the right alignment just yet, as I
> expect that will get solved with your follow up patch to respect
> CONFIG_FUNCTION_ALIGNMENT in assembly annotated functions also.

Not exactly, no. Those will first need converting from ENTRY() to the
new annotations model. Which I certainly intend to get to.

Jan
diff mbox series

Patch

diff --git a/xen/Kconfig b/xen/Kconfig
index 134e6e68ad84..feb5fa5ecb0a 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -37,6 +37,27 @@  config CC_HAS_VISIBILITY_ATTRIBUTE
 config CC_SPLIT_SECTIONS
 	bool
 
+# Set function alignment.
+#
+# Allow setting on a boolean basis, and then convert such selection to an
+# integer for the build system and code to consume more easily.
+#
+# Requires clang >= 7.0.0
+config CC_HAS_FUNCTION_ALIGNMENT
+	def_bool $(cc-option,-falign-functions)
+config FUNCTION_ALIGNMENT_4B
+	bool
+config FUNCTION_ALIGNMENT_8B
+	bool
+config FUNCTION_ALIGNMENT_16B
+	bool
+config FUNCTION_ALIGNMENT
+	int
+	depends on CC_HAS_FUNCTION_ALIGNMENT
+	default 16 if FUNCTION_ALIGNMENT_16B
+	default  8 if  FUNCTION_ALIGNMENT_8B
+	default  4 if  FUNCTION_ALIGNMENT_4B
+
 source "arch/$(SRCARCH)/Kconfig"
 
 config DEFCONFIG_LIST
diff --git a/xen/Makefile b/xen/Makefile
index 21832d640225..162cb2bda1c5 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -390,6 +390,9 @@  CFLAGS += -fomit-frame-pointer
 endif
 
 CFLAGS-$(CONFIG_CC_SPLIT_SECTIONS) += -ffunction-sections -fdata-sections
+ifdef CONFIG_FUNCTION_ALIGNMENT
+CFLAGS += -falign-functions=$(CONFIG_FUNCTION_ALIGNMENT)
+endif
 
 CFLAGS += -nostdinc -fno-builtin -fno-common
 CFLAGS += -Werror -Wredundant-decls -Wwrite-strings -Wno-pointer-arith
diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
index bbca1e5a5ed3..aa8ae8c38d28 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -68,6 +68,8 @@  void arch_livepatch_revive(void)
 
 int arch_livepatch_verify_func(const struct livepatch_func *func)
 {
+    BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE > CONFIG_FUNCTION_ALIGNMENT);
+
     /* If NOPing only do up to maximum amount we can put in the ->opaque. */
     if ( !func->new_addr && (func->new_size > LIVEPATCH_OPAQUE_SIZE ||
          func->new_size % ARCH_PATCH_INSN_SIZE) )
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 59b80d122fd0..afaf1e996b0e 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -44,6 +44,10 @@  SECTIONS
 #ifdef CONFIG_CC_SPLIT_SECTIONS
        *(.text.*)
 #endif
+#ifdef CONFIG_FUNCTION_ALIGNMENT
+       /* Ensure enough distance with the next placed section. */
+       . = ALIGN(CONFIG_FUNCTION_ALIGNMENT);
+#endif
 
        *(.fixup)
        *(.gnu.warning)
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 1acdffc51c22..0cd741be5b6f 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -29,6 +29,7 @@  config X86
 	select HAS_UBSAN
 	select HAS_VPCI if HVM
 	select NEEDS_LIBELF
+	select FUNCTION_ALIGNMENT_16B if !DEBUG
 
 config ARCH_DEFCONFIG
 	string
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index ee539f001b73..b00ad7120da9 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -109,6 +109,10 @@  int arch_livepatch_verify_func(const struct livepatch_func *func)
          */
         uint8_t needed = ARCH_PATCH_INSN_SIZE;
 
+        BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE +
+                     (IS_ENABLED(CONIFG_XEN_IBT) ? ENDBR64_LEN : 0) >
+                     CONFIG_FUNCTION_ALIGNMENT);
+
         if ( is_endbr64(func->old_addr) || is_endbr64_poison(func->old_addr) )
             needed += ENDBR64_LEN;
 
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 8930e14fc40e..5b3332300d44 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -99,6 +99,10 @@  SECTIONS
        *(.text)
 #ifdef CONFIG_CC_SPLIT_SECTIONS
        *(.text.*)
+#endif
+#ifdef CONFIG_FUNCTION_ALIGNMENT
+       /* Ensure enough distance with the next placed section. */
+       . = ALIGN(CONFIG_FUNCTION_ALIGNMENT);
 #endif
        *(.text.__x86_indirect_thunk_*)
 
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 310ad4229cdf..c9a21c3c8a07 100644
--- 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
 	---help---
 	  Allows a running Xen hypervisor to be dynamically patched using
 	  binary patches without rebooting. This is primarily used to binarily