[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
 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




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.