[PATCH 0/8] x86/altcall: Switch to a simpler scheme

[PATCH 0/8] x86/altcall: Switch to a simpler scheme

[Andrew Cooper]

See patch 7 for details.  This simplifies _apply_alternatives() specifically
by removing a special and quite complicated case, and encodes the metadata
about altcalls differently and more efficiently.

https://gitlab.com/xen-project/hardware/xen-staging/-/pipelines/1780872355

Andrew Cooper (8):
  x86/altcall: Split alternative-call.h out of alternative.h
  x86/altcall: Rename alternative_branches() to boot_apply_alt_calls()
  x86/alternatives: Rework information passing into
    nmi_apply_alternatives()
  x86/alternatives: Factor seal_endbr64() out of _apply_alternatives()
  x86/altcall: Introduce new simpler scheme
  xen/livepatch: Support new altcall scheme
  x86/altcall: Switch to simpler scheme
  x86/alternatives: Simplify _apply_alternatives() now altcall is
    separate

 .../eclair_analysis/ECLAIR/deviations.ecl     |   4 +-
 xen/arch/x86/alternative.c                    | 267 ++++++++++--------
 .../asm/{alternative.h => alternative-call.h} | 179 +-----------
 xen/arch/x86/include/asm/alternative.h        | 263 -----------------
 xen/arch/x86/include/asm/hvm/hvm.h            |   2 +-
 xen/arch/x86/setup.c                          |   3 +-
 xen/arch/x86/xen.lds.S                        |   4 +
 xen/common/core_parking.c                     |   4 +-
 xen/common/livepatch.c                        |  58 ++++
 xen/include/xen/alternative-call.h            |  24 +-
 10 files changed, 252 insertions(+), 556 deletions(-)
 copy xen/arch/x86/include/asm/{alternative.h => alternative-call.h} (63%)

-- 
2.39.5

[PATCH 1/8] x86/altcall: Split alternative-call.h out of alternative.h

[Andrew Cooper]

... in preparation for changing how they're implemented.

Update the MISRA deviations with the new path.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Anthony PERARD <anthony.perard@vates.tech>
CC: Michal Orzel <michal.orzel@amd.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Julien Grall <julien@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: consulting@bugseng.com <consulting@bugseng.com>
CC: Nicola Vetrini <nicola.vetrini@bugseng.com>

In terms of naming, while tailcalls can technically be jumps, they're still
usually reasoned about as being calls.

It appears that everywhere else which needs alternative_{v,}call() gets it
transitively through hvm.h
---
 .../eclair_analysis/ECLAIR/deviations.ecl     |   4 +-
 .../asm/{alternative.h => alternative-call.h} | 171 +-----------
 xen/arch/x86/include/asm/alternative.h        | 262 ------------------
 xen/arch/x86/include/asm/hvm/hvm.h            |   2 +-
 xen/common/core_parking.c                     |   4 +-
 xen/include/xen/alternative-call.h            |  10 +-
 6 files changed, 16 insertions(+), 437 deletions(-)
 copy xen/arch/x86/include/asm/{alternative.h => alternative-call.h} (64%)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 2c8fb9271391..9c67358d4663 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -414,8 +414,8 @@ of the short-circuit evaluation strategy of such logical operators."
 -doc_end
 
 -doc_begin="Macros alternative_v?call[0-9] use sizeof and typeof to check that the argument types match the corresponding parameter ones."
--config=MC3A2.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_vcall[0-9]$))&&file(^xen/arch/x86/include/asm/alternative\\.h*$)))"}
--config=B.UNEVALEFF,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_v?call[0-9]$))&&file(^xen/arch/x86/include/asm/alterantive\\.h*$)))"}
+-config=MC3A2.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_vcall[0-9]$))&&file(^xen/arch/x86/include/asm/alternative-call\\.h*$)))"}
+-config=B.UNEVALEFF,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_v?call[0-9]$))&&file(^xen/arch/x86/include/asm/alterantive-call\\.h*$)))"}
 -doc_end
 
 -doc_begin="Anything, no matter how complicated, inside the BUILD_BUG_ON macro is subject to a compile-time evaluation without relevant side effects."
diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative-call.h
similarity index 64%
copy from xen/arch/x86/include/asm/alternative.h
copy to xen/arch/x86/include/asm/alternative-call.h
index 7326ad942836..828ea32a9625 100644
--- a/xen/arch/x86/include/asm/alternative.h
+++ b/xen/arch/x86/include/asm/alternative-call.h
@@ -1,165 +1,8 @@
-#ifndef __X86_ALTERNATIVE_H__
-#define __X86_ALTERNATIVE_H__
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef X86_ALTERNATIVE_CALL_H
+#define X86_ALTERNATIVE_CALL_H
 
-#ifdef __ASSEMBLY__
-#include <asm/alternative-asm.h>
-#else
-
-#include <xen/macros.h>
-#include <xen/types.h>
-
-#include <asm/asm-macros.h>
-#include <asm/cpufeatureset.h>
-
-struct __packed alt_instr {
-    int32_t  orig_offset;   /* original instruction */
-    int32_t  repl_offset;   /* offset to replacement instruction */
-    uint16_t cpuid;         /* cpuid bit set for replacement */
-    uint8_t  orig_len;      /* length of original instruction */
-    uint8_t  repl_len;      /* length of new instruction */
-    uint8_t  pad_len;       /* length of build-time padding */
-    uint8_t  priv;          /* Private, for use by apply_alternatives() */
-};
-
-#define __ALT_PTR(a,f)      ((uint8_t *)((void *)&(a)->f + (a)->f))
-#define ALT_ORIG_PTR(a)     __ALT_PTR(a, orig_offset)
-#define ALT_REPL_PTR(a)     __ALT_PTR(a, repl_offset)
-
-extern void add_nops(void *insns, unsigned int len);
-/* Similar to alternative_instructions except it can be run with IRQs enabled. */
-extern int apply_alternatives(struct alt_instr *start, struct alt_instr *end);
-extern void alternative_instructions(void);
-extern void alternative_branches(void);
-
-#define alt_orig_len       "(.LXEN%=_orig_e - .LXEN%=_orig_s)"
-#define alt_pad_len        "(.LXEN%=_orig_p - .LXEN%=_orig_e)"
-#define alt_total_len      "(.LXEN%=_orig_p - .LXEN%=_orig_s)"
-#define alt_repl_s(num)    ".LXEN%=_repl_s"#num
-#define alt_repl_e(num)    ".LXEN%=_repl_e"#num
-#define alt_repl_len(num)  "(" alt_repl_e(num) " - " alt_repl_s(num) ")"
-
-/*
- * GAS's idea of true is sometimes 1 and sometimes -1, while Clang's idea
- * was consistently 1 up to 6.x (it matches GAS's now).  Transform it to
- * uniformly 1.
- */
-#define AS_TRUE(x) "((" x ") & 1)"
-
-#define as_max(a, b) "(("a") ^ ((("a") ^ ("b")) & -("AS_TRUE("("a") < ("b")")")))"
-
-#define OLDINSTR(oldinstr, padding)                              \
-    ".LXEN%=_orig_s:\n\t" oldinstr "\n .LXEN%=_orig_e:\n\t"      \
-    ".LXEN%=_diff = " padding "\n\t"                             \
-    "mknops ("AS_TRUE(".LXEN%=_diff > 0")" * .LXEN%=_diff)\n\t"  \
-    ".LXEN%=_orig_p:\n\t"
-
-#define OLDINSTR_1(oldinstr, n1)                                 \
-    OLDINSTR(oldinstr, alt_repl_len(n1) "-" alt_orig_len)
-
-#define OLDINSTR_2(oldinstr, n1, n2)                             \
-    OLDINSTR(oldinstr,                                           \
-             as_max(alt_repl_len(n1),                            \
-                    alt_repl_len(n2)) "-" alt_orig_len)
-
-#define ALTINSTR_ENTRY(feature, num)                                    \
-        " .if " STR(feature) " >= " STR(NCAPINTS * 32) "\n"             \
-        " .error \"alternative feature outside of featureset range\"\n" \
-        " .endif\n"                                                     \
-        " .long .LXEN%=_orig_s - .\n"             /* label           */ \
-        " .long " alt_repl_s(num)" - .\n"         /* new instruction */ \
-        " .word " STR(feature) "\n"               /* feature bit     */ \
-        " .byte " alt_orig_len "\n"               /* source len      */ \
-        " .byte " alt_repl_len(num) "\n"          /* replacement len */ \
-        " .byte " alt_pad_len "\n"                /* padding len     */ \
-        " .byte 0\n"                              /* priv            */
-
-#define DISCARD_ENTRY(num)                        /* repl <= total */   \
-        " .byte 0xff + (" alt_repl_len(num) ") - (" alt_total_len ")\n"
-
-#define ALTINSTR_REPLACEMENT(newinstr, num)       /* replacement */     \
-        alt_repl_s(num)":\n\t" newinstr "\n" alt_repl_e(num) ":\n\t"
-
-/* alternative assembly primitive: */
-#define ALTERNATIVE(oldinstr, newinstr, feature)                        \
-        OLDINSTR_1(oldinstr, 1)                                         \
-        ".pushsection .altinstructions, \"a\", @progbits\n"             \
-        ALTINSTR_ENTRY(feature, 1)                                      \
-        ".section .discard, \"a\", @progbits\n"                         \
-        ".byte " alt_total_len "\n" /* total_len <= 255 */              \
-        DISCARD_ENTRY(1)                                                \
-        ".section .altinstr_replacement, \"ax\", @progbits\n"           \
-        ALTINSTR_REPLACEMENT(newinstr, 1)                               \
-        ".popsection\n"
-
-#define ALTERNATIVE_2(oldinstr, newinstr1, feature1, newinstr2, feature2) \
-        OLDINSTR_2(oldinstr, 1, 2)                                      \
-        ".pushsection .altinstructions, \"a\", @progbits\n"             \
-        ALTINSTR_ENTRY(feature1, 1)                                     \
-        ALTINSTR_ENTRY(feature2, 2)                                     \
-        ".section .discard, \"a\", @progbits\n"                         \
-        ".byte " alt_total_len "\n" /* total_len <= 255 */              \
-        DISCARD_ENTRY(1)                                                \
-        DISCARD_ENTRY(2)                                                \
-        ".section .altinstr_replacement, \"ax\", @progbits\n"           \
-        ALTINSTR_REPLACEMENT(newinstr1, 1)                              \
-        ALTINSTR_REPLACEMENT(newinstr2, 2)                              \
-        ".popsection\n"
-
-/*
- * Alternative instructions for different CPU types or capabilities.
- *
- * This allows to use optimized instructions even on generic binary
- * kernels.
- *
- * length of oldinstr must be longer or equal the length of newinstr
- * It can be padded with nops as needed.
- *
- * For non barrier like inlines please define new variants
- * without volatile and memory clobber.
- */
-#define alternative(oldinstr, newinstr, feature)                        \
-        asm volatile (ALTERNATIVE(oldinstr, newinstr, feature) : : : "memory")
-
-#define alternative_2(oldinstr, newinstr1, feature1, newinstr2, feature2) \
-	asm volatile (ALTERNATIVE_2(oldinstr, newinstr1, feature1,	\
-				    newinstr2, feature2)		\
-		      : : : "memory")
-
-/*
- * Alternative inline assembly with input.
- *
- * Pecularities:
- * No memory clobber here.
- * Argument numbers start with 1.
- * Best is to use constraints that are fixed size (like (%1) ... "r")
- * If you use variable sized constraints like "m" or "g" in the
- * replacement make sure to pad to the worst case length.
- */
-#define alternative_input(oldinstr, newinstr, feature, input...)	\
-	asm volatile (ALTERNATIVE(oldinstr, newinstr, feature)		\
-		      : : input)
-
-/* Like alternative_input, but with a single output argument */
-#define alternative_io(oldinstr, newinstr, feature, output, input...)	\
-	asm volatile (ALTERNATIVE(oldinstr, newinstr, feature)		\
-		      : output : input)
-
-/*
- * This is similar to alternative_io. But it has two features and
- * respective instructions.
- *
- * If CPU has feature2, newinstr2 is used.
- * Otherwise, if CPU has feature1, newinstr1 is used.
- * Otherwise, oldinstr is used.
- */
-#define alternative_io_2(oldinstr, newinstr1, feature1, newinstr2,	\
-			 feature2, output, input...)			\
-	asm volatile(ALTERNATIVE_2(oldinstr, newinstr1, feature1,	\
-				   newinstr2, feature2)			\
-		     : output : input)
-
-/* Use this macro(s) if you need more than one output parameter. */
-#define ASM_OUTPUT2(a...) a
+#include <asm/alternative.h>
 
 /*
  * Machinery to allow converting indirect to direct calls, when the called
@@ -303,7 +146,7 @@ extern void alternative_branches(void);
 })
 
 #define alternative_call3(func, arg1, arg2, arg3) ({     \
-    typeof(arg1) v1_ = (arg1);                            \
+    typeof(arg1) v1_ = (arg1);                           \
     typeof(arg2) v2_ = (arg2);                           \
     typeof(arg3) v3_ = (arg3);                           \
     ALT_CALL_ARG(v1_, 1);                                \
@@ -423,6 +266,4 @@ extern void alternative_branches(void);
 #define alternative_call(func, args...) \
     alternative_call_(count_args(args))(func, ## args)
 
-#endif /*  !__ASSEMBLY__  */
-
-#endif /* __X86_ALTERNATIVE_H__ */
+#endif /* X86_ALTERNATIVE_CALL_H */
diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative.h
index 7326ad942836..2d2ace97f794 100644
--- a/xen/arch/x86/include/asm/alternative.h
+++ b/xen/arch/x86/include/asm/alternative.h
@@ -161,268 +161,6 @@ extern void alternative_branches(void);
 /* Use this macro(s) if you need more than one output parameter. */
 #define ASM_OUTPUT2(a...) a
 
-/*
- * Machinery to allow converting indirect to direct calls, when the called
- * function is determined once at boot and later never changed.
- */
-
-#define ALT_CALL_arg1 "rdi"
-#define ALT_CALL_arg2 "rsi"
-#define ALT_CALL_arg3 "rdx"
-#define ALT_CALL_arg4 "rcx"
-#define ALT_CALL_arg5 "r8"
-#define ALT_CALL_arg6 "r9"
-
-#ifdef CONFIG_CC_IS_CLANG
-/*
- * Clang doesn't follow the psABI and doesn't truncate parameter values at the
- * callee.  This can lead to bad code being generated when using alternative
- * calls.
- *
- * Workaround it by using a temporary intermediate variable that's zeroed
- * before being assigned the parameter value, as that forces clang to zero the
- * register at the caller.
- *
- * This has been reported upstream:
- * https://github.com/llvm/llvm-project/issues/12579
- * https://github.com/llvm/llvm-project/issues/82598
- */
-#define ALT_CALL_ARG(arg, n)                                            \
-    register unsigned long a ## n ## _ asm ( ALT_CALL_arg ## n ) = ({   \
-        unsigned long tmp = 0;                                          \
-        BUILD_BUG_ON(sizeof(arg) > sizeof(unsigned long));              \
-        *(typeof(arg) *)&tmp = (arg);                                   \
-        tmp;                                                            \
-    })
-#else
-#define ALT_CALL_ARG(arg, n) \
-    register typeof(arg) a ## n ## _ asm ( ALT_CALL_arg ## n ) = \
-        ({ BUILD_BUG_ON(sizeof(arg) > sizeof(void *)); (arg); })
-#endif
-#define ALT_CALL_NO_ARG(n) \
-    register unsigned long a ## n ## _ asm ( ALT_CALL_arg ## n )
-
-#define ALT_CALL_NO_ARG6 ALT_CALL_NO_ARG(6)
-#define ALT_CALL_NO_ARG5 ALT_CALL_NO_ARG(5); ALT_CALL_NO_ARG6
-#define ALT_CALL_NO_ARG4 ALT_CALL_NO_ARG(4); ALT_CALL_NO_ARG5
-#define ALT_CALL_NO_ARG3 ALT_CALL_NO_ARG(3); ALT_CALL_NO_ARG4
-#define ALT_CALL_NO_ARG2 ALT_CALL_NO_ARG(2); ALT_CALL_NO_ARG3
-#define ALT_CALL_NO_ARG1 ALT_CALL_NO_ARG(1); ALT_CALL_NO_ARG2
-
-/*
- * Unfortunately ALT_CALL_NO_ARG() above can't use a fake initializer (to
- * suppress "uninitialized variable" warnings), as various versions of gcc
- * older than 8.1 fall on the nose in various ways with that (always because
- * of some other construct elsewhere in the same function needing to use the
- * same hard register). Otherwise the asm() below could uniformly use "+r"
- * output constraints, making unnecessary all these ALT_CALL<n>_OUT macros.
- */
-#define ALT_CALL0_OUT "=r" (a1_), "=r" (a2_), "=r" (a3_), \
-                      "=r" (a4_), "=r" (a5_), "=r" (a6_)
-#define ALT_CALL1_OUT "+r" (a1_), "=r" (a2_), "=r" (a3_), \
-                      "=r" (a4_), "=r" (a5_), "=r" (a6_)
-#define ALT_CALL2_OUT "+r" (a1_), "+r" (a2_), "=r" (a3_), \
-                      "=r" (a4_), "=r" (a5_), "=r" (a6_)
-#define ALT_CALL3_OUT "+r" (a1_), "+r" (a2_), "+r" (a3_), \
-                      "=r" (a4_), "=r" (a5_), "=r" (a6_)
-#define ALT_CALL4_OUT "+r" (a1_), "+r" (a2_), "+r" (a3_), \
-                      "+r" (a4_), "=r" (a5_), "=r" (a6_)
-#define ALT_CALL5_OUT "+r" (a1_), "+r" (a2_), "+r" (a3_), \
-                      "+r" (a4_), "+r" (a5_), "=r" (a6_)
-#define ALT_CALL6_OUT "+r" (a1_), "+r" (a2_), "+r" (a3_), \
-                      "+r" (a4_), "+r" (a5_), "+r" (a6_)
-
-#define alternative_callN(n, rettype, func) ({                     \
-    rettype ret_;                                                  \
-    register unsigned long r10_ asm("r10");                        \
-    register unsigned long r11_ asm("r11");                        \
-    asm volatile (ALTERNATIVE("call *%c[addr](%%rip)", "call .",   \
-                              X86_FEATURE_ALWAYS)                  \
-                  : ALT_CALL ## n ## _OUT, "=a" (ret_),            \
-                    "=r" (r10_), "=r" (r11_) ASM_CALL_CONSTRAINT   \
-                  : [addr] "i" (&(func)), "g" (func)               \
-                  : "memory" );                                    \
-    ret_;                                                          \
-})
-
-#define alternative_vcall0(func) ({             \
-    ALT_CALL_NO_ARG1;                           \
-    (void)sizeof(func());                       \
-    (void)alternative_callN(0, int, func);      \
-})
-
-#define alternative_call0(func) ({              \
-    ALT_CALL_NO_ARG1;                           \
-    alternative_callN(0, typeof(func()), func); \
-})
-
-#define alternative_vcall1(func, arg) ({           \
-    typeof(arg) v1_ = (arg);                       \
-    ALT_CALL_ARG(v1_, 1);                          \
-    ALT_CALL_NO_ARG2;                              \
-    (void)sizeof(func(arg));                       \
-    (void)alternative_callN(1, int, func);         \
-})
-
-#define alternative_call1(func, arg) ({            \
-    typeof(arg) v1_ = (arg);                       \
-    ALT_CALL_ARG(v1_, 1);                          \
-    ALT_CALL_NO_ARG2;                              \
-    alternative_callN(1, typeof(func(arg)), func); \
-})
-
-#define alternative_vcall2(func, arg1, arg2) ({           \
-    typeof(arg1) v1_ = (arg1);                            \
-    typeof(arg2) v2_ = (arg2);                            \
-    ALT_CALL_ARG(v1_, 1);                                 \
-    ALT_CALL_ARG(v2_, 2);                                 \
-    ALT_CALL_NO_ARG3;                                     \
-    (void)sizeof(func(arg1, arg2));                       \
-    (void)alternative_callN(2, int, func);                \
-})
-
-#define alternative_call2(func, arg1, arg2) ({            \
-    typeof(arg1) v1_ = (arg1);                            \
-    typeof(arg2) v2_ = (arg2);                            \
-    ALT_CALL_ARG(v1_, 1);                                 \
-    ALT_CALL_ARG(v2_, 2);                                 \
-    ALT_CALL_NO_ARG3;                                     \
-    alternative_callN(2, typeof(func(arg1, arg2)), func); \
-})
-
-#define alternative_vcall3(func, arg1, arg2, arg3) ({    \
-    typeof(arg1) v1_ = (arg1);                           \
-    typeof(arg2) v2_ = (arg2);                           \
-    typeof(arg3) v3_ = (arg3);                           \
-    ALT_CALL_ARG(v1_, 1);                                \
-    ALT_CALL_ARG(v2_, 2);                                \
-    ALT_CALL_ARG(v3_, 3);                                \
-    ALT_CALL_NO_ARG4;                                    \
-    (void)sizeof(func(arg1, arg2, arg3));                \
-    (void)alternative_callN(3, int, func);               \
-})
-
-#define alternative_call3(func, arg1, arg2, arg3) ({     \
-    typeof(arg1) v1_ = (arg1);                            \
-    typeof(arg2) v2_ = (arg2);                           \
-    typeof(arg3) v3_ = (arg3);                           \
-    ALT_CALL_ARG(v1_, 1);                                \
-    ALT_CALL_ARG(v2_, 2);                                \
-    ALT_CALL_ARG(v3_, 3);                                \
-    ALT_CALL_NO_ARG4;                                    \
-    alternative_callN(3, typeof(func(arg1, arg2, arg3)), \
-                      func);                             \
-})
-
-#define alternative_vcall4(func, arg1, arg2, arg3, arg4) ({ \
-    typeof(arg1) v1_ = (arg1);                              \
-    typeof(arg2) v2_ = (arg2);                              \
-    typeof(arg3) v3_ = (arg3);                              \
-    typeof(arg4) v4_ = (arg4);                              \
-    ALT_CALL_ARG(v1_, 1);                                   \
-    ALT_CALL_ARG(v2_, 2);                                   \
-    ALT_CALL_ARG(v3_, 3);                                   \
-    ALT_CALL_ARG(v4_, 4);                                   \
-    ALT_CALL_NO_ARG5;                                       \
-    (void)sizeof(func(arg1, arg2, arg3, arg4));             \
-    (void)alternative_callN(4, int, func);                  \
-})
-
-#define alternative_call4(func, arg1, arg2, arg3, arg4) ({  \
-    typeof(arg1) v1_ = (arg1);                              \
-    typeof(arg2) v2_ = (arg2);                              \
-    typeof(arg3) v3_ = (arg3);                              \
-    typeof(arg4) v4_ = (arg4);                              \
-    ALT_CALL_ARG(v1_, 1);                                   \
-    ALT_CALL_ARG(v2_, 2);                                   \
-    ALT_CALL_ARG(v3_, 3);                                   \
-    ALT_CALL_ARG(v4_, 4);                                   \
-    ALT_CALL_NO_ARG5;                                       \
-    alternative_callN(4, typeof(func(arg1, arg2,            \
-                                     arg3, arg4)),          \
-                      func);                                \
-})
-
-#define alternative_vcall5(func, arg1, arg2, arg3, arg4, arg5) ({ \
-    typeof(arg1) v1_ = (arg1);                                    \
-    typeof(arg2) v2_ = (arg2);                                    \
-    typeof(arg3) v3_ = (arg3);                                    \
-    typeof(arg4) v4_ = (arg4);                                    \
-    typeof(arg5) v5_ = (arg5);                                    \
-    ALT_CALL_ARG(v1_, 1);                                         \
-    ALT_CALL_ARG(v2_, 2);                                         \
-    ALT_CALL_ARG(v3_, 3);                                         \
-    ALT_CALL_ARG(v4_, 4);                                         \
-    ALT_CALL_ARG(v5_, 5);                                         \
-    ALT_CALL_NO_ARG6;                                             \
-    (void)sizeof(func(arg1, arg2, arg3, arg4, arg5));             \
-    (void)alternative_callN(5, int, func);                        \
-})
-
-#define alternative_call5(func, arg1, arg2, arg3, arg4, arg5) ({  \
-    typeof(arg1) v1_ = (arg1);                                    \
-    typeof(arg2) v2_ = (arg2);                                    \
-    typeof(arg3) v3_ = (arg3);                                    \
-    typeof(arg4) v4_ = (arg4);                                    \
-    typeof(arg5) v5_ = (arg5);                                    \
-    ALT_CALL_ARG(v1_, 1);                                         \
-    ALT_CALL_ARG(v2_, 2);                                         \
-    ALT_CALL_ARG(v3_, 3);                                         \
-    ALT_CALL_ARG(v4_, 4);                                         \
-    ALT_CALL_ARG(v5_, 5);                                         \
-    ALT_CALL_NO_ARG6;                                             \
-    alternative_callN(5, typeof(func(arg1, arg2, arg3,            \
-                                     arg4, arg5)),                \
-                      func);                                      \
-})
-
-#define alternative_vcall6(func, arg1, arg2, arg3, arg4, arg5, arg6) ({ \
-    typeof(arg1) v1_ = (arg1);                                          \
-    typeof(arg2) v2_ = (arg2);                                          \
-    typeof(arg3) v3_ = (arg3);                                          \
-    typeof(arg4) v4_ = (arg4);                                          \
-    typeof(arg5) v5_ = (arg5);                                          \
-    typeof(arg6) v6_ = (arg6);                                          \
-    ALT_CALL_ARG(v1_, 1);                                               \
-    ALT_CALL_ARG(v2_, 2);                                               \
-    ALT_CALL_ARG(v3_, 3);                                               \
-    ALT_CALL_ARG(v4_, 4);                                               \
-    ALT_CALL_ARG(v5_, 5);                                               \
-    ALT_CALL_ARG(v6_, 6);                                               \
-    (void)sizeof(func(arg1, arg2, arg3, arg4, arg5, arg6));             \
-    (void)alternative_callN(6, int, func);                              \
-})
-
-#define alternative_call6(func, arg1, arg2, arg3, arg4, arg5, arg6) ({  \
-    typeof(arg1) v1_ = (arg1);                                          \
-    typeof(arg2) v2_ = (arg2);                                          \
-    typeof(arg3) v3_ = (arg3);                                          \
-    typeof(arg4) v4_ = (arg4);                                          \
-    typeof(arg5) v5_ = (arg5);                                          \
-    typeof(arg6) v6_ = (arg6);                                          \
-    ALT_CALL_ARG(v1_, 1);                                               \
-    ALT_CALL_ARG(v2_, 2);                                               \
-    ALT_CALL_ARG(v3_, 3);                                               \
-    ALT_CALL_ARG(v4_, 4);                                               \
-    ALT_CALL_ARG(v5_, 5);                                               \
-    ALT_CALL_ARG(v6_, 6);                                               \
-    alternative_callN(6, typeof(func(arg1, arg2, arg3,                  \
-                                     arg4, arg5, arg6)),                \
-                      func);                                            \
-})
-
-#define alternative_vcall__(nr) alternative_vcall ## nr
-#define alternative_call__(nr)  alternative_call ## nr
-
-#define alternative_vcall_(nr) alternative_vcall__(nr)
-#define alternative_call_(nr)  alternative_call__(nr)
-
-#define alternative_vcall(func, args...) \
-    alternative_vcall_(count_args(args))(func, ## args)
-
-#define alternative_call(func, args...) \
-    alternative_call_(count_args(args))(func, ## args)
-
 #endif /*  !__ASSEMBLY__  */
 
 #endif /* __X86_ALTERNATIVE_H__ */
diff --git a/xen/arch/x86/include/asm/hvm/hvm.h b/xen/arch/x86/include/asm/hvm/hvm.h
index 963e8201130a..bf8bc2e100bd 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -9,9 +9,9 @@
 #ifndef __ASM_X86_HVM_HVM_H__
 #define __ASM_X86_HVM_HVM_H__
 
+#include <xen/alternative-call.h>
 #include <xen/mm.h>
 
-#include <asm/alternative.h>
 #include <asm/asm_defns.h>
 #include <asm/current.h>
 #include <asm/x86_emulate.h>
diff --git a/xen/common/core_parking.c b/xen/common/core_parking.c
index a970ffeab8c3..7d6a18cdcf4c 100644
--- a/xen/common/core_parking.c
+++ b/xen/common/core_parking.c
@@ -15,10 +15,10 @@
  *  General Public License for more details.
  */
 
-#include <xen/types.h>
+#include <xen/alternative-call.h>
 #include <xen/cpu.h>
-#include <xen/init.h>
 #include <xen/cpumask.h>
+#include <xen/init.h>
 #include <xen/param.h>
 
 #include <asm/smp.h>
diff --git a/xen/include/xen/alternative-call.h b/xen/include/xen/alternative-call.h
index 62672b732431..39339c3f0f76 100644
--- a/xen/include/xen/alternative-call.h
+++ b/xen/include/xen/alternative-call.h
@@ -13,10 +13,10 @@
  *
  * For architectures to support:
  *
- * - Implement alternative_{,v}call() in asm/alternative.h.  Code generation
- *   requirements are to emit a function pointer call at build time, and stash
- *   enough metadata to simplify the call at boot once the implementation has
- *   been resolved.
+ * - Implement alternative_{,v}call() in asm/alternative-call.h.  Code
+ *   generation requirements are to emit a function pointer call at build
+ *   time, and stash enough metadata to simplify the call at boot once the
+ *   implementation has been resolved.
  * - Select ALTERNATIVE_CALL in Kconfig.
  *
  * To use:
@@ -48,7 +48,7 @@
 
 #ifdef CONFIG_ALTERNATIVE_CALL
 
-#include <asm/alternative.h>
+#include <asm/alternative-call.h>
 
 #ifdef CONFIG_LIVEPATCH
 /* Must keep for livepatches to resolve alternative calls. */
-- 
2.39.5

[PATCH 2/8] x86/altcall: Rename alternative_branches() to boot_apply_alt_calls()

[Andrew Cooper]

The alternatives APIs are not great; rename alternative_branches() to be more
precise.  Centralise the declaration in xen/alternative-call.h, in the
expectation that x86 won't be the only user in the long term.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/alternative.c             |  2 +-
 xen/arch/x86/include/asm/alternative.h |  1 -
 xen/arch/x86/setup.c                   |  3 ++-
 xen/include/xen/alternative-call.h     | 10 +++++++++-
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index 1ba35cb9ede9..d1a3b7ea7ca6 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -493,7 +493,7 @@ void __init alternative_instructions(void)
     _alternative_instructions(false);
 }
 
-void __init alternative_branches(void)
+void __init boot_apply_alt_calls(void)
 {
     local_irq_disable();
     _alternative_instructions(true);
diff --git a/xen/arch/x86/include/asm/alternative.h b/xen/arch/x86/include/asm/alternative.h
index 2d2ace97f794..29c3d724b07f 100644
--- a/xen/arch/x86/include/asm/alternative.h
+++ b/xen/arch/x86/include/asm/alternative.h
@@ -29,7 +29,6 @@ extern void add_nops(void *insns, unsigned int len);
 /* Similar to alternative_instructions except it can be run with IRQs enabled. */
 extern int apply_alternatives(struct alt_instr *start, struct alt_instr *end);
 extern void alternative_instructions(void);
-extern void alternative_branches(void);
 
 #define alt_orig_len       "(.LXEN%=_orig_e - .LXEN%=_orig_s)"
 #define alt_pad_len        "(.LXEN%=_orig_p - .LXEN%=_orig_e)"
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index cf1ea040dd90..25189541244d 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1,4 +1,5 @@
 #include <xen/acpi.h>
+#include <xen/alternative-call.h>
 #include <xen/bitops.h>
 #include <xen/console.h>
 #include <xen/cpu.h>
@@ -2082,7 +2083,7 @@ void asmlinkage __init noreturn __start_xen(void)
 
     do_presmp_initcalls();
 
-    alternative_branches();
+    boot_apply_alt_calls();
 
     /*
      * NB: when running as a PV shim VCPUOP_up/down is wired to the shim
diff --git a/xen/include/xen/alternative-call.h b/xen/include/xen/alternative-call.h
index 39339c3f0f76..3c855bfa44f5 100644
--- a/xen/include/xen/alternative-call.h
+++ b/xen/include/xen/alternative-call.h
@@ -17,6 +17,8 @@
  *   generation requirements are to emit a function pointer call at build
  *   time, and stash enough metadata to simplify the call at boot once the
  *   implementation has been resolved.
+ * - Implement boot_apply_alt_calls() to convert the function pointer calls
+ *   into direct calls on boot.
  * - Select ALTERNATIVE_CALL in Kconfig.
  *
  * To use:
@@ -57,7 +59,13 @@
 # define __alt_call_maybe_initdata __initdata
 #endif
 
-#else
+/*
+ * Devirtualise the alternative_{,v}call()'s on boot.  Convert still-NULL
+ * function pointers into traps.
+ */
+void boot_apply_alt_calls(void);
+
+#else /* CONFIG_ALTERNATIVE_CALL */
 
 #define alternative_call(func, args...)  (func)(args)
 #define alternative_vcall(func, args...) (func)(args)
-- 
2.39.5

[PATCH 3/8] x86/alternatives: Rework information passing into nmi_apply_alternatives()

[Andrew Cooper]

nmi_apply_alternatives() is soon going to need to dispatch to multiple
functions, and a force parameter is not a good way of passing information.

Introduce ALT_INSNS and ALT_CALLS to pass in at the top level to select the
operation(s) desired.  They represent what will happen when we've separated
the altcalls out of the general alternative instructions infrastructure,
although in the short term we still need to synthesise the force parameter for
_apply_alternatives().

Move two externs to reduce their scope a little.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/alternative.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index d1a3b7ea7ca6..9aa591b364a4 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -19,8 +19,6 @@
 
 #define MAX_PATCH_LEN (255-1)
 
-extern struct alt_instr __alt_instructions[], __alt_instructions_end[];
-
 #ifdef K8_NOP1
 static const unsigned char k8nops[] init_or_livepatch_const = {
     K8_NOP1,
@@ -387,9 +385,13 @@ int apply_alternatives(struct alt_instr *start, struct alt_instr *end)
 }
 #endif
 
+#define ALT_INSNS (1U << 0)
+#define ALT_CALLS (1U << 1)
 static unsigned int __initdata alt_todo;
 static unsigned int __initdata alt_done;
 
+extern struct alt_instr __alt_instructions[], __alt_instructions_end[];
+
 /*
  * At boot time, we patch alternatives in NMI context.  This means that the
  * active NMI-shadow will defer any further NMIs, removing the slim race
@@ -419,7 +421,7 @@ static int __init cf_check nmi_apply_alternatives(
         flush_local(FLUSH_TLB_GLOBAL);
 
         rc = _apply_alternatives(__alt_instructions, __alt_instructions_end,
-                                 alt_done);
+                                 alt_todo == ALT_CALLS);
         if ( rc )
             panic("Unable to apply alternatives: %d\n", rc);
 
@@ -442,7 +444,7 @@ static int __init cf_check nmi_apply_alternatives(
  * This routine is called with local interrupt disabled and used during
  * bootup.
  */
-static void __init _alternative_instructions(bool force)
+static void __init _alternative_instructions(unsigned int what)
 {
     unsigned int i;
     nmi_callback_t *saved_nmi_callback;
@@ -460,7 +462,7 @@ static void __init _alternative_instructions(bool force)
     ASSERT(!local_irq_is_enabled());
 
     /* Set what operation to perform /before/ setting the callback. */
-    alt_todo = 1u << force;
+    alt_todo = what;
     barrier();
 
     /*
@@ -490,12 +492,12 @@ static void __init _alternative_instructions(bool force)
 void __init alternative_instructions(void)
 {
     arch_init_ideal_nops();
-    _alternative_instructions(false);
+    _alternative_instructions(ALT_INSNS);
 }
 
 void __init boot_apply_alt_calls(void)
 {
     local_irq_disable();
-    _alternative_instructions(true);
+    _alternative_instructions(ALT_CALLS);
     local_irq_enable();
 }
-- 
2.39.5

[PATCH 4/8] x86/alternatives: Factor seal_endbr64() out of _apply_alternatives()

[Andrew Cooper]

We are going to need to reposition the call in a change with several moving
parts.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/alternative.c | 70 ++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 30 deletions(-)

diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index 9aa591b364a4..4b9f8d860153 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -162,6 +162,44 @@ text_poke(void *addr, const void *opcode, size_t len)
 extern void *const __initdata_cf_clobber_start[];
 extern void *const __initdata_cf_clobber_end[];
 
+/*
+ * In CET-IBT enabled builds, clobber endbr64 instructions after altcall has
+ * finished optimising all indirect branches to direct ones.
+ */
+static void __init seal_endbr64(void)
+{
+    void *const *val;
+    unsigned int clobbered = 0;
+
+    if ( !cpu_has_xen_ibt )
+        return;
+
+    /*
+     * This is some minor structure (ab)use.  We walk the entire contents
+     * of .init.{ro,}data.cf_clobber as if it were an array of pointers.
+     *
+     * If the pointer points into .text, and at an endbr64 instruction,
+     * nop out the endbr64.  This causes the pointer to no longer be a
+     * legal indirect branch target under CET-IBT.  This is a
+     * defence-in-depth measure, to reduce the options available to an
+     * adversary who has managed to hijack a function pointer.
+     */
+    for ( val = __initdata_cf_clobber_start;
+          val < __initdata_cf_clobber_end;
+          val++ )
+    {
+        void *ptr = *val;
+
+        if ( !is_kernel_text(ptr) || !is_endbr64(ptr) )
+            continue;
+
+        place_endbr64_poison(ptr);
+        clobbered++;
+    }
+
+    printk("altcall: Optimised away %u endbr64 instructions\n", clobbered);
+}
+
 /*
  * Replace instructions with better alternatives for this CPU type.
  * This runs before SMP is initialized to avoid SMP problems with
@@ -344,36 +382,8 @@ static int init_or_livepatch _apply_alternatives(struct alt_instr *start,
      * Clobber endbr64 instructions now that altcall has finished optimising
      * all indirect branches to direct ones.
      */
-    if ( force && cpu_has_xen_ibt && system_state < SYS_STATE_active )
-    {
-        void *const *val;
-        unsigned int clobbered = 0;
-
-        /*
-         * This is some minor structure (ab)use.  We walk the entire contents
-         * of .init.{ro,}data.cf_clobber as if it were an array of pointers.
-         *
-         * If the pointer points into .text, and at an endbr64 instruction,
-         * nop out the endbr64.  This causes the pointer to no longer be a
-         * legal indirect branch target under CET-IBT.  This is a
-         * defence-in-depth measure, to reduce the options available to an
-         * adversary who has managed to hijack a function pointer.
-         */
-        for ( val = __initdata_cf_clobber_start;
-              val < __initdata_cf_clobber_end;
-              val++ )
-        {
-            void *ptr = *val;
-
-            if ( !is_kernel_text(ptr) || !is_endbr64(ptr) )
-                continue;
-
-            place_endbr64_poison(ptr);
-            clobbered++;
-        }
-
-        printk("altcall: Optimised away %u endbr64 instructions\n", clobbered);
-    }
+    if ( force && system_state < SYS_STATE_active )
+        seal_endbr64();
 
     return 0;
 }
-- 
2.39.5

[PATCH 5/8] x86/altcall: Introduce new simpler scheme

[Andrew Cooper]

Encoding altcalls as regular alternatives leads to an unreasonable amount of
complexity in _apply_alternatives().

Introduce apply_alt_calls(), and an .alt_call_sites section which simply
tracks the source address (relative, to save on space).  That's literally all
that is needed in order to devirtualise the function pointers.

apply_alt_calls() is mostly as per _apply_alternatives(), except the size is
known to be 6 bytes.  Drop the logic for JMP *RIPREL, as there's no support
for tailcall optimisations, nor a feasbile plan on how to introduce support.
Pad with a redundant prefix to avoid needing a separate NOP on the end.

Wire it up in nmi_apply_alternatives(), although the section is empty at this
juncture so nothing happens in practice.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>

Finding a 6-byte UD instruction that is distinct from ud2 turns out to be
quite challengning.  The easy way involves a length changing prefix, which is
best avoided.  Suggestions for alternative patterns welcome.

---
 xen/arch/x86/alternative.c                  | 94 +++++++++++++++++++++
 xen/arch/x86/include/asm/alternative-call.h |  7 ++
 xen/arch/x86/xen.lds.S                      |  4 +
 3 files changed, 105 insertions(+)

diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index 4b9f8d860153..f6594e21a14c 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -388,6 +388,92 @@ static int init_or_livepatch _apply_alternatives(struct alt_instr *start,
     return 0;
 }
 
+/*
+ * At build time, alternative calls are emitted as:
+ *   ff 15 xx xx xx xx  =>  call *disp32(%rip)
+ *
+ * During boot, we devirtualise by editing to:
+ *   2e e8 xx xx xx xx  =>  cs call disp32
+ *
+ * or, if the function pointer is still NULL, poison to:
+ *   0f 0b 0f 0b 0f 0b  =>  ud2a (x3)
+ */
+static int init_or_livepatch apply_alt_calls(
+    const struct alt_call *start, const struct alt_call *end)
+{
+    const struct alt_call *a;
+
+    for ( a = start; a < end; a++ )
+    {
+        const uint8_t *dest;
+        uint8_t buf[6], *orig = ALT_CALL_PTR(a);
+        long disp;
+
+        /* It's likely that this won't change, but check just to be safe. */
+        BUILD_BUG_ON(ALT_CALL_LEN(a) != 6);
+
+        if ( orig[0] != 0xff || orig[1] != 0x15 )
+        {
+            printk(XENLOG_ERR
+                   "Altcall for %ps [%6ph] not CALL *RIPREL\n",
+                   orig, orig);
+            return -EINVAL;
+        }
+
+        disp = *(int32_t *)(orig + 2);
+        dest = *(const void **)(orig + 6 + disp);
+
+        if ( dest )
+        {
+            /*
+             * When building for CET-IBT, all function pointer targets
+             * should have an endbr64 instruction.
+             *
+             * If this is not the case, leave a warning because
+             * something is probably wrong with the build.  A CET-IBT
+             * enabled system might have exploded already.
+             *
+             * Otherwise, skip the endbr64 instruction.  This is a
+             * marginal perf improvement which saves on instruction
+             * decode bandwidth.
+             */
+            if ( IS_ENABLED(CONFIG_XEN_IBT) )
+            {
+                if ( is_endbr64(dest) )
+                    dest += ENDBR64_LEN;
+                else
+                    printk(XENLOG_WARNING
+                           "Altcall %ps dest %ps has no endbr64\n",
+                           orig, dest);
+            }
+
+            disp = dest - (orig + 6);
+            ASSERT(disp == (int32_t)disp);
+
+            buf[0] = 0x2e;
+            buf[1] = 0xe8;
+            *(int32_t *)(buf + 2) = disp;
+        }
+        else
+        {
+            /*
+             * The function pointer is still NULL.  Seal the whole call, as
+             * it's not used.
+             */
+            buf[0] = 0x0f;
+            buf[1] = 0x0b;
+            buf[2] = 0x0f;
+            buf[3] = 0x0b;
+            buf[4] = 0x0f;
+            buf[5] = 0x0b;
+        }
+
+        text_poke(orig, buf, sizeof(buf));
+    }
+
+    return 0;
+}
+
 #ifdef CONFIG_LIVEPATCH
 int apply_alternatives(struct alt_instr *start, struct alt_instr *end)
 {
@@ -401,6 +487,7 @@ static unsigned int __initdata alt_todo;
 static unsigned int __initdata alt_done;
 
 extern struct alt_instr __alt_instructions[], __alt_instructions_end[];
+extern struct alt_call __alt_call_sites_start[], __alt_call_sites_end[];
 
 /*
  * At boot time, we patch alternatives in NMI context.  This means that the
@@ -435,6 +522,13 @@ static int __init cf_check nmi_apply_alternatives(
         if ( rc )
             panic("Unable to apply alternatives: %d\n", rc);
 
+        if ( alt_todo & ALT_CALLS )
+        {
+            rc = apply_alt_calls(__alt_call_sites_start, __alt_call_sites_end);
+            if ( rc )
+                panic("Unable to apply alternative calls: %d\n", rc);
+        }
+
         /*
          * Reinstate perms on .text to be RX.  This also cleans out the dirty
          * bits, which matters when CET Shstk is active.
diff --git a/xen/arch/x86/include/asm/alternative-call.h b/xen/arch/x86/include/asm/alternative-call.h
index 828ea32a9625..49a04a7cc45b 100644
--- a/xen/arch/x86/include/asm/alternative-call.h
+++ b/xen/arch/x86/include/asm/alternative-call.h
@@ -4,6 +4,13 @@
 
 #include <asm/alternative.h>
 
+/* Simply the relative position of the source call. */
+struct alt_call {
+    int32_t offset;
+};
+#define ALT_CALL_PTR(a) ((void *)&(a)->offset + (a)->offset)
+#define ALT_CALL_LEN(a) (6)
+
 /*
  * Machinery to allow converting indirect to direct calls, when the called
  * function is determined once at boot and later never changed.
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index d4dd6434c466..53bafc98a536 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -260,6 +260,10 @@ SECTIONS
         __alt_instructions = .;
         *(.altinstructions)
         __alt_instructions_end = .;
+        . = ALIGN(4);
+        __alt_call_sites_start = .;
+        *(.alt_call_sites)
+        __alt_call_sites_end = .;
 
        LOCK_PROFILE_DATA
 
-- 
2.39.5

[PATCH 6/8] xen/livepatch: Support new altcall scheme

[Andrew Cooper]

The new altcall scheme uses an .alt_call_sites section.  Wire this up in very
much the same way as the .altinstructions section, although there is less
sanity checking necessary.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/arch/x86/alternative.c         |  6 ++++
 xen/common/livepatch.c             | 58 ++++++++++++++++++++++++++++++
 xen/include/xen/alternative-call.h |  8 +++--
 3 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index f6594e21a14c..22af224f08f7 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -479,6 +479,12 @@ int apply_alternatives(struct alt_instr *start, struct alt_instr *end)
 {
     return _apply_alternatives(start, end, true);
 }
+
+int livepatch_apply_alt_calls(const struct alt_call *start,
+                              const struct alt_call *end)
+{
+    return apply_alt_calls(start, end);
+}
 #endif
 
 #define ALT_INSNS (1U << 0)
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 6ce77bf021b7..be9b7e367553 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -905,6 +905,64 @@ static int prepare_payload(struct payload *payload,
 #endif
     }
 
+    sec = livepatch_elf_sec_by_name(elf, ".alt_call_sites");
+    if ( sec )
+    {
+#ifdef CONFIG_ALTERNATIVE_CALL
+        const struct alt_call *a, *start, *end;
+
+        if ( !section_ok(elf, sec, sizeof(*a)) )
+            return -EINVAL;
+
+        /* Tolerate an empty .alt_call_sites section... */
+        if ( sec->sec->sh_size == 0 )
+            goto alt_call_done;
+
+        /* ... but otherwise, there needs to be something to alter... */
+        if ( payload->text_size == 0 )
+        {
+            printk(XENLOG_ERR LIVEPATCH "%s Alternative calls provided, but no .text\n",
+                   elf->name);
+            return -EINVAL;
+        }
+
+        start = sec->addr;
+        end = sec->addr + sec->sec->sh_size;
+
+        for ( a = start; a < end; a++ )
+        {
+            const void *orig = ALT_CALL_PTR(a);
+            size_t len = ALT_CALL_LEN(a);
+
+            /* orig must be fully within .text. */
+            if ( orig       < payload->text_addr ||
+                 len        > payload->text_size ||
+                 orig + len > payload->text_addr + payload->text_size )
+            {
+                printk(XENLOG_ERR LIVEPATCH
+                       "%s: Alternative call %p+%#zx outside payload text %p+%#zx\n",
+                       elf->name, orig, len,
+                       payload->text_addr, payload->text_size);
+                return -EINVAL;
+            }
+        }
+
+        rc = livepatch_apply_alt_calls(start, end);
+        if ( rc )
+        {
+            printk(XENLOG_ERR LIVEPATCH "%s: Applying alternative calls failed: %d\n",
+                   elf->name, rc);
+            return rc;
+        }
+
+    alt_call_done:;
+#else /* CONFIG_ALTERNATIVE_CALL */
+        printk(XENLOG_ERR LIVEPATCH "%s: Alternative calls not supported\n",
+               elf->name);
+        return -EOPNOTSUPP;
+#endif /* !CONFIG_ALTERNATIVE_CALL */
+    }
+
     sec = livepatch_elf_sec_by_name(elf, ".ex_table");
     if ( sec )
     {
diff --git a/xen/include/xen/alternative-call.h b/xen/include/xen/alternative-call.h
index 3c855bfa44f5..767c2149bce7 100644
--- a/xen/include/xen/alternative-call.h
+++ b/xen/include/xen/alternative-call.h
@@ -17,8 +17,8 @@
  *   generation requirements are to emit a function pointer call at build
  *   time, and stash enough metadata to simplify the call at boot once the
  *   implementation has been resolved.
- * - Implement boot_apply_alt_calls() to convert the function pointer calls
- *   into direct calls on boot.
+ * - Implement {boot,livepatch}_apply_alt_calls() to convert the function
+ *   pointer calls into direct calls on boot/livepatch.
  * - Select ALTERNATIVE_CALL in Kconfig.
  *
  * To use:
@@ -65,6 +65,10 @@
  */
 void boot_apply_alt_calls(void);
 
+/* As per boot_apply_alt_calls() but for a livepatch. */
+int livepatch_apply_alt_calls(const struct alt_call *start,
+                              const struct alt_call *end);
+
 #else /* CONFIG_ALTERNATIVE_CALL */
 
 #define alternative_call(func, args...)  (func)(args)
-- 
2.39.5

[PATCH 7/8] x86/altcall: Switch to simpler scheme

[Andrew Cooper]

With all the infrastructure in place, switch from using ALTERNATIVE() to
simply populating .alt_call_sites.

Before, _apply_alternatives() would devirtualise in two passes; the first
being opportunistic, and the second (signified by the force parameter) sealing
any call with a still-NULL function pointer.

Now, all devirtualising is performed together, at the point in time of the
second pass previously.  The call to seal_endbr64() needs delaying until after
apply_alt_calls() is complete, or we have a narrow window with real indirect
branches and no ENDBR64 instructions.

Under the hood, the following changes are happening:

  Section                Old size   New size   Change (%)
  .alt_call_sites               0    0x00730   +0x0730
  .altinstructions        0x1350a    0x11fe0   -0x152a (-7%)
  .altinstr_replacement   0x015f2    0x00e35   -0x07bd (-23%)

The changes aren't quite equal because inlining is affected by the smaller
asm() block.  Nevertheless, the metadata is held in 1/3 of the space, and
there are no CALL instructions held in the replacement section any more.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/alternative.c                  | 9 ++-------
 xen/arch/x86/include/asm/alternative-call.h | 9 ++++++---
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index 22af224f08f7..047bfc6e424b 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -378,13 +378,6 @@ static int init_or_livepatch _apply_alternatives(struct alt_instr *start,
         text_poke(orig, buf, total_len);
     }
 
-    /*
-     * Clobber endbr64 instructions now that altcall has finished optimising
-     * all indirect branches to direct ones.
-     */
-    if ( force && system_state < SYS_STATE_active )
-        seal_endbr64();
-
     return 0;
 }
 
@@ -533,6 +526,8 @@ static int __init cf_check nmi_apply_alternatives(
             rc = apply_alt_calls(__alt_call_sites_start, __alt_call_sites_end);
             if ( rc )
                 panic("Unable to apply alternative calls: %d\n", rc);
+
+            seal_endbr64();
         }
 
         /*
diff --git a/xen/arch/x86/include/asm/alternative-call.h b/xen/arch/x86/include/asm/alternative-call.h
index 49a04a7cc45b..bbc49a5274d9 100644
--- a/xen/arch/x86/include/asm/alternative-call.h
+++ b/xen/arch/x86/include/asm/alternative-call.h
@@ -2,7 +2,8 @@
 #ifndef X86_ALTERNATIVE_CALL_H
 #define X86_ALTERNATIVE_CALL_H
 
-#include <asm/alternative.h>
+#include <xen/macros.h>
+#include <xen/stdint.h>
 
 /* Simply the relative position of the source call. */
 struct alt_call {
@@ -86,8 +87,10 @@ struct alt_call {
     rettype ret_;                                                  \
     register unsigned long r10_ asm("r10");                        \
     register unsigned long r11_ asm("r11");                        \
-    asm volatile (ALTERNATIVE("call *%c[addr](%%rip)", "call .",   \
-                              X86_FEATURE_ALWAYS)                  \
+    asm volatile ("1: call *%c[addr](%%rip)\n\t"                   \
+                  ".pushsection .alt_call_sites, \"a\", @progbits\n\t"  \
+                  ".long 1b - .\n\t"                               \
+                  ".popsection"                                    \
                   : ALT_CALL ## n ## _OUT, "=a" (ret_),            \
                     "=r" (r10_), "=r" (r11_) ASM_CALL_CONSTRAINT   \
                   : [addr] "i" (&(func)), "g" (func)               \
-- 
2.39.5

[PATCH 8/8] x86/alternatives: Simplify _apply_alternatives() now altcall is separate

[Andrew Cooper]

With altcall handled separately, the special case in _apply_alternatives() is
unused and can be dropped.  The force parameter (used to signify the seal
pass) can be removed too.

In turn, nmi_apply_alternatives() no longer needs to call
_apply_alternatives() on the second pass.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/alternative.c | 94 ++++----------------------------------
 1 file changed, 10 insertions(+), 84 deletions(-)

diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index 047bfc6e424b..43b009888c02 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -206,14 +206,9 @@ static void __init seal_endbr64(void)
  * self modifying code. This implies that asymmetric systems where
  * APs have less capabilities than the boot processor are not handled.
  * Tough. Make sure you disable such features by hand.
- *
- * The caller will set the "force" argument to true for the final
- * invocation, such that no CALLs/JMPs to NULL pointers will be left
- * around. See also the further comment below.
  */
 static int init_or_livepatch _apply_alternatives(struct alt_instr *start,
-                                                 struct alt_instr *end,
-                                                 bool force)
+                                                 struct alt_instr *end)
 {
     struct alt_instr *a, *base;
 
@@ -274,10 +269,7 @@ static int init_or_livepatch _apply_alternatives(struct alt_instr *start,
 
         /* Skip patch sites already handled during the first pass. */
         if ( a->priv )
-        {
-            ASSERT(force);
             continue;
-        }
 
         /* If there is no replacement to make, see about optimising the nops. */
         if ( !boot_cpu_has(a->cpuid) )
@@ -301,76 +293,7 @@ static int init_or_livepatch _apply_alternatives(struct alt_instr *start,
 
         /* 0xe8/0xe9 are relative branches; fix the offset. */
         if ( a->repl_len >= 5 && (*buf & 0xfe) == 0xe8 )
-        {
-            /*
-             * Detect the special case of indirect-to-direct branch patching:
-             * - replacement is a direct CALL/JMP (opcodes 0xE8/0xE9; already
-             *   checked above),
-             * - replacement's displacement is -5 (pointing back at the very
-             *   insn, which makes no sense in a real replacement insn),
-             * - original is an indirect CALL/JMP (opcodes 0xFF/2 or 0xFF/4)
-             *   using RIP-relative addressing.
-             * Some branch destinations may still be NULL when we come here
-             * the first time. Defer patching of those until the post-presmp-
-             * initcalls re-invocation (with force set to true). If at that
-             * point the branch destination is still NULL, insert "UD2; UD0"
-             * (for ease of recognition) instead of CALL/JMP.
-             */
-            if ( a->cpuid == X86_FEATURE_ALWAYS &&
-                 *(int32_t *)(buf + 1) == -5 &&
-                 a->orig_len >= 6 &&
-                 orig[0] == 0xff &&
-                 orig[1] == (*buf & 1 ? 0x25 : 0x15) )
-            {
-                long disp = *(int32_t *)(orig + 2);
-                const uint8_t *dest = *(void **)(orig + 6 + disp);
-
-                if ( dest )
-                {
-                    /*
-                     * When building for CET-IBT, all function pointer targets
-                     * should have an endbr64 instruction.
-                     *
-                     * If this is not the case, leave a warning because
-                     * something is probably wrong with the build.  A CET-IBT
-                     * enabled system might have exploded already.
-                     *
-                     * Otherwise, skip the endbr64 instruction.  This is a
-                     * marginal perf improvement which saves on instruction
-                     * decode bandwidth.
-                     */
-                    if ( IS_ENABLED(CONFIG_XEN_IBT) )
-                    {
-                        if ( is_endbr64(dest) )
-                            dest += ENDBR64_LEN;
-                        else
-                            printk(XENLOG_WARNING
-                                   "altcall %ps dest %ps has no endbr64\n",
-                                   orig, dest);
-                    }
-
-                    disp = dest - (orig + 5);
-                    ASSERT(disp == (int32_t)disp);
-                    *(int32_t *)(buf + 1) = disp;
-                }
-                else if ( force )
-                {
-                    buf[0] = 0x0f;
-                    buf[1] = 0x0b;
-                    buf[2] = 0x0f;
-                    buf[3] = 0xff;
-                    buf[4] = 0xff;
-                }
-                else
-                    continue;
-            }
-            else if ( force && system_state < SYS_STATE_active )
-                ASSERT_UNREACHABLE();
-            else
-                *(int32_t *)(buf + 1) += repl - orig;
-        }
-        else if ( force && system_state < SYS_STATE_active  )
-            ASSERT_UNREACHABLE();
+            *(int32_t *)(buf + 1) += repl - orig;
 
         a->priv = 1;
 
@@ -470,7 +393,7 @@ static int init_or_livepatch apply_alt_calls(
 #ifdef CONFIG_LIVEPATCH
 int apply_alternatives(struct alt_instr *start, struct alt_instr *end)
 {
-    return _apply_alternatives(start, end, true);
+    return _apply_alternatives(start, end);
 }
 
 int livepatch_apply_alt_calls(const struct alt_call *start,
@@ -516,10 +439,13 @@ static int __init cf_check nmi_apply_alternatives(
                                  PAGE_HYPERVISOR_RWX);
         flush_local(FLUSH_TLB_GLOBAL);
 
-        rc = _apply_alternatives(__alt_instructions, __alt_instructions_end,
-                                 alt_todo == ALT_CALLS);
-        if ( rc )
-            panic("Unable to apply alternatives: %d\n", rc);
+        if ( alt_todo & ALT_INSNS )
+        {
+            rc = _apply_alternatives(__alt_instructions,
+                                     __alt_instructions_end);
+            if ( rc )
+                panic("Unable to apply alternatives: %d\n", rc);
+        }
 
         if ( alt_todo & ALT_CALLS )
         {
-- 
2.39.5

Re: [PATCH 1/8] x86/altcall: Split alternative-call.h out of alternative.h

[Jan Beulich]

On 23.04.2025 03:02, Andrew Cooper wrote:
> ... in preparation for changing how they're implemented.
> 
> Update the MISRA deviations with the new path.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH 2/8] x86/altcall: Rename alternative_branches() to boot_apply_alt_calls()

[Jan Beulich]

On 23.04.2025 03:02, Andrew Cooper wrote:
> The alternatives APIs are not great; rename alternative_branches() to be more
> precise.  Centralise the declaration in xen/alternative-call.h, in the
> expectation that x86 won't be the only user in the long term.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH 3/8] x86/alternatives: Rework information passing into nmi_apply_alternatives()

[Jan Beulich]

On 23.04.2025 03:02, Andrew Cooper wrote:
> nmi_apply_alternatives() is soon going to need to dispatch to multiple
> functions, and a force parameter is not a good way of passing information.
> 
> Introduce ALT_INSNS and ALT_CALLS to pass in at the top level to select the
> operation(s) desired.  They represent what will happen when we've separated
> the altcalls out of the general alternative instructions infrastructure,
> although in the short term we still need to synthesise the force parameter for
> _apply_alternatives().
> 
> Move two externs to reduce their scope a little.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH 4/8] x86/alternatives: Factor seal_endbr64() out of _apply_alternatives()

[Jan Beulich]

On 23.04.2025 03:02, Andrew Cooper wrote:
> We are going to need to reposition the call in a change with several moving
> parts.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH 5/8] x86/altcall: Introduce new simpler scheme

[Jan Beulich]

On 23.04.2025 03:02, Andrew Cooper wrote:
> Encoding altcalls as regular alternatives leads to an unreasonable amount of
> complexity in _apply_alternatives().
> 
> Introduce apply_alt_calls(), and an .alt_call_sites section which simply
> tracks the source address (relative, to save on space).  That's literally all
> that is needed in order to devirtualise the function pointers.
> 
> apply_alt_calls() is mostly as per _apply_alternatives(), except the size is
> known to be 6 bytes.  Drop the logic for JMP *RIPREL, as there's no support
> for tailcall optimisations, nor a feasbile plan on how to introduce support.
> Pad with a redundant prefix to avoid needing a separate NOP on the end.
> 
> Wire it up in nmi_apply_alternatives(), although the section is empty at this
> juncture so nothing happens in practice.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

> Finding a 6-byte UD instruction that is distinct from ud2 turns out to be
> quite challengning.  The easy way involves a length changing prefix, which is
> best avoided.  Suggestions for alternative patterns welcome.

(Intel syntax, sorry.)

	ud0	edi, [edi+edi-1]
	ud1	edi, [edi+edi-1]

	ud0	edi, cs:[rdi+rdi-1]
	ud1	edi, cs:[rdi+rdi-1]

Jan

Re: [PATCH 6/8] xen/livepatch: Support new altcall scheme

[Jan Beulich]

On 23.04.2025 03:02, Andrew Cooper wrote:
> The new altcall scheme uses an .alt_call_sites section.  Wire this up in very
> much the same way as the .altinstructions section, although there is less
> sanity checking necessary.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH 7/8] x86/altcall: Switch to simpler scheme

[Jan Beulich]

On 23.04.2025 03:02, Andrew Cooper wrote:
> --- a/xen/arch/x86/alternative.c
> +++ b/xen/arch/x86/alternative.c
> @@ -378,13 +378,6 @@ static int init_or_livepatch _apply_alternatives(struct alt_instr *start,
>          text_poke(orig, buf, total_len);
>      }
>  
> -    /*
> -     * Clobber endbr64 instructions now that altcall has finished optimising
> -     * all indirect branches to direct ones.
> -     */
> -    if ( force && system_state < SYS_STATE_active )
> -        seal_endbr64();
> -
>      return 0;
>  }
>  
> @@ -533,6 +526,8 @@ static int __init cf_check nmi_apply_alternatives(
>              rc = apply_alt_calls(__alt_call_sites_start, __alt_call_sites_end);
>              if ( rc )
>                  panic("Unable to apply alternative calls: %d\n", rc);
> +
> +            seal_endbr64();
>          }

Are you deliberately losing the comment? Other than this:
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [PATCH 8/8] x86/alternatives: Simplify _apply_alternatives() now altcall is separate

[Jan Beulich]

On 23.04.2025 03:02, Andrew Cooper wrote:
> With altcall handled separately, the special case in _apply_alternatives() is
> unused and can be dropped.  The force parameter (used to signify the seal
> pass) can be removed too.
> 
> In turn, nmi_apply_alternatives() no longer needs to call
> _apply_alternatives() on the second pass.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Re: [PATCH 7/8] x86/altcall: Switch to simpler scheme

[Andrew Cooper]

On 23/04/2025 10:01 am, Jan Beulich wrote:
> On 23.04.2025 03:02, Andrew Cooper wrote:
>> --- a/xen/arch/x86/alternative.c
>> +++ b/xen/arch/x86/alternative.c
>> @@ -378,13 +378,6 @@ static int init_or_livepatch _apply_alternatives(struct alt_instr *start,
>>          text_poke(orig, buf, total_len);
>>      }
>>  
>> -    /*
>> -     * Clobber endbr64 instructions now that altcall has finished optimising
>> -     * all indirect branches to direct ones.
>> -     */
>> -    if ( force && system_state < SYS_STATE_active )
>> -        seal_endbr64();
>> -
>>      return 0;
>>  }
>>  
>> @@ -533,6 +526,8 @@ static int __init cf_check nmi_apply_alternatives(
>>              rc = apply_alt_calls(__alt_call_sites_start, __alt_call_sites_end);
>>              if ( rc )
>>                  panic("Unable to apply alternative calls: %d\n", rc);
>> +
>> +            seal_endbr64();
>>          }
> Are you deliberately losing the comment?

Yes, counterbalancing the comment gained in patch 4.

>  Other than this:
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks,

~Andrew

Re: [PATCH 6/8] xen/livepatch: Support new altcall scheme

[Roger Pau Monné]

On Wed, Apr 23, 2025 at 02:02:35AM +0100, Andrew Cooper wrote:
> The new altcall scheme uses an .alt_call_sites section.  Wire this up in very
> much the same way as the .altinstructions section, although there is less
> sanity checking necessary.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

One nit/comment below.

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>  xen/arch/x86/alternative.c         |  6 ++++
>  xen/common/livepatch.c             | 58 ++++++++++++++++++++++++++++++
>  xen/include/xen/alternative-call.h |  8 +++--
>  3 files changed, 70 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
> index f6594e21a14c..22af224f08f7 100644
> --- a/xen/arch/x86/alternative.c
> +++ b/xen/arch/x86/alternative.c
> @@ -479,6 +479,12 @@ int apply_alternatives(struct alt_instr *start, struct alt_instr *end)
>  {
>      return _apply_alternatives(start, end, true);
>  }
> +
> +int livepatch_apply_alt_calls(const struct alt_call *start,
> +                              const struct alt_call *end)
> +{
> +    return apply_alt_calls(start, end);
> +}
>  #endif
>  
>  #define ALT_INSNS (1U << 0)
> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> index 6ce77bf021b7..be9b7e367553 100644
> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -905,6 +905,64 @@ static int prepare_payload(struct payload *payload,
>  #endif
>      }
>  
> +    sec = livepatch_elf_sec_by_name(elf, ".alt_call_sites");
> +    if ( sec )
> +    {
> +#ifdef CONFIG_ALTERNATIVE_CALL
> +        const struct alt_call *a, *start, *end;
> +
> +        if ( !section_ok(elf, sec, sizeof(*a)) )
> +            return -EINVAL;
> +
> +        /* Tolerate an empty .alt_call_sites section... */
> +        if ( sec->sec->sh_size == 0 )

You could possibly move this check to the outer `if` condition, and
avoid the alt_call_done label?

As even in the !CONFIG_ALTERNATIVE_CALL case skipping an empty section
would be OK.

Thanks, Roger.

Re: [PATCH 6/8] xen/livepatch: Support new altcall scheme

[Andrew Cooper]

On 23/04/2025 12:16 pm, Roger Pau Monné wrote:
> On Wed, Apr 23, 2025 at 02:02:35AM +0100, Andrew Cooper wrote:
>> The new altcall scheme uses an .alt_call_sites section.  Wire this up in very
>> much the same way as the .altinstructions section, although there is less
>> sanity checking necessary.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

>
> One nit/comment below.
>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Ross Lagerwall <ross.lagerwall@citrix.com>
>> ---
>>  xen/arch/x86/alternative.c         |  6 ++++
>>  xen/common/livepatch.c             | 58 ++++++++++++++++++++++++++++++
>>  xen/include/xen/alternative-call.h |  8 +++--
>>  3 files changed, 70 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
>> index f6594e21a14c..22af224f08f7 100644
>> --- a/xen/arch/x86/alternative.c
>> +++ b/xen/arch/x86/alternative.c
>> @@ -479,6 +479,12 @@ int apply_alternatives(struct alt_instr *start, struct alt_instr *end)
>>  {
>>      return _apply_alternatives(start, end, true);
>>  }
>> +
>> +int livepatch_apply_alt_calls(const struct alt_call *start,
>> +                              const struct alt_call *end)
>> +{
>> +    return apply_alt_calls(start, end);
>> +}
>>  #endif
>>  
>>  #define ALT_INSNS (1U << 0)
>> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
>> index 6ce77bf021b7..be9b7e367553 100644
>> --- a/xen/common/livepatch.c
>> +++ b/xen/common/livepatch.c
>> @@ -905,6 +905,64 @@ static int prepare_payload(struct payload *payload,
>>  #endif
>>      }
>>  
>> +    sec = livepatch_elf_sec_by_name(elf, ".alt_call_sites");
>> +    if ( sec )
>> +    {
>> +#ifdef CONFIG_ALTERNATIVE_CALL
>> +        const struct alt_call *a, *start, *end;
>> +
>> +        if ( !section_ok(elf, sec, sizeof(*a)) )
>> +            return -EINVAL;
>> +
>> +        /* Tolerate an empty .alt_call_sites section... */
>> +        if ( sec->sec->sh_size == 0 )
> You could possibly move this check to the outer `if` condition, and
> avoid the alt_call_done label?
>
> As even in the !CONFIG_ALTERNATIVE_CALL case skipping an empty section
> would be OK.

.altinstructions is like this.  It was put in as part of e74360e4ba4a, I
believe because it was decided that an empty section wasn't wanted.

We can revisit the decision, but the logic should be consistent.

~Andrew