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

[Xen-devel] [PATCH v1 08/13] xen/arm: alternatives: Add dynamic patching feature



This is based on the Linux commit dea5e2a4c5bc "arm64: alternatives: Add
dynamic patching feature" written by Marc Zyngier:

    We've so far relied on a patching infrastructure that only gave us
    a single alternative, without any way to provide a range of potential
    replacement instructions. For a single feature, this is an all or
    nothing thing.

    It would be interesting to have a more flexible grained way of patching the
    kernel though, where we could dynamically tune the code that gets injected.

    In order to achive this, let's introduce a new form of dynamic patching,
    assiciating a callback to a patching site. This callback gets source and
    target locations of the patching request, as well as the number of
    instructions to be patched.

    Dynamic patching is declared with the new ALTERNATIVE_CB and alternative_cb
    directives:
                    asm volatile(ALTERNATIVE_CB("mov %0, #0\n", callback)
                                 : "r" (v));
    or

                    alternative_cb callback
                            mov x0, #0
                    alternative_cb_end

    where callback is the C function computing the alternative.

    Reviewed-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
    Reviewed-by: Catalin Marinas <catalin.marinas@xxxxxxx>
    Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>

This is part of XSA-263.

Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

---
    Changes in v2:
        - Fix typo in the commit message
        - Add Stefano's acked-by
---
 xen/arch/arm/alternative.c        | 48 +++++++++++++++++++++++++++++----------
 xen/include/asm-arm/alternative.h | 44 +++++++++++++++++++++++++++++++----
 2 files changed, 75 insertions(+), 17 deletions(-)

diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
index 936cf04956..52ed7edf69 100644
--- a/xen/arch/arm/alternative.c
+++ b/xen/arch/arm/alternative.c
@@ -30,6 +30,8 @@
 #include <asm/byteorder.h>
 #include <asm/cpufeature.h>
 #include <asm/insn.h>
+/* XXX: Move ARCH_PATCH_INSN_SIZE out of livepatch.h */
+#include <asm/livepatch.h>
 #include <asm/page.h>
 
 /* Override macros from asm/page.h to make them work with mfn_t */
@@ -94,6 +96,23 @@ static u32 get_alt_insn(const struct alt_instr *alt,
     return insn;
 }
 
+static void patch_alternative(const struct alt_instr *alt,
+                              const uint32_t *origptr,
+                              uint32_t *updptr, int nr_inst)
+{
+    const uint32_t *replptr;
+    unsigned int i;
+
+    replptr = ALT_REPL_PTR(alt);
+    for ( i = 0; i < nr_inst; i++ )
+    {
+        uint32_t insn;
+
+        insn = get_alt_insn(alt, origptr + i, replptr + i);
+        updptr[i] = cpu_to_le32(insn);
+    }
+}
+
 /*
  * The region patched should be read-write to allow __apply_alternatives
  * to replacing the instructions when necessary.
@@ -105,33 +124,38 @@ static int __apply_alternatives(const struct alt_region 
*region,
                                 paddr_t update_offset)
 {
     const struct alt_instr *alt;
-    const u32 *replptr, *origptr;
+    const u32 *origptr;
     u32 *updptr;
+    alternative_cb_t alt_cb;
 
     printk(XENLOG_INFO "alternatives: Patching with alt table %p -> %p\n",
            region->begin, region->end);
 
     for ( alt = region->begin; alt < region->end; alt++ )
     {
-        u32 insn;
-        int i, nr_inst;
+        int nr_inst;
 
-        if ( !cpus_have_cap(alt->cpufeature) )
+        /* Use ARM_CB_PATCH as an unconditional patch */
+        if ( alt->cpufeature < ARM_CB_PATCH &&
+             !cpus_have_cap(alt->cpufeature) )
             continue;
 
-        BUG_ON(alt->alt_len != alt->orig_len);
+        if ( alt->cpufeature == ARM_CB_PATCH )
+            BUG_ON(alt->alt_len != 0);
+        else
+            BUG_ON(alt->alt_len != alt->orig_len);
 
         origptr = ALT_ORIG_PTR(alt);
         updptr = (void *)origptr + update_offset;
-        replptr = ALT_REPL_PTR(alt);
 
-        nr_inst = alt->alt_len / sizeof(insn);
+        nr_inst = alt->orig_len / ARCH_PATCH_INSN_SIZE;
 
-        for ( i = 0; i < nr_inst; i++ )
-        {
-            insn = get_alt_insn(alt, origptr + i, replptr + i);
-            *(updptr + i) = cpu_to_le32(insn);
-        }
+        if ( alt->cpufeature < ARM_CB_PATCH )
+            alt_cb = patch_alternative;
+        else
+            alt_cb = ALT_REPL_PTR(alt);
+
+        alt_cb(alt, origptr, updptr, nr_inst);
 
         /* Ensure the new instructions reached the memory and nuke */
         clean_and_invalidate_dcache_va_range(origptr,
diff --git a/xen/include/asm-arm/alternative.h 
b/xen/include/asm-arm/alternative.h
index 4e33d1cdf7..9b4b02811b 100644
--- a/xen/include/asm-arm/alternative.h
+++ b/xen/include/asm-arm/alternative.h
@@ -3,6 +3,8 @@
 
 #include <asm/cpufeature.h>
 
+#define ARM_CB_PATCH ARM_NCAPS
+
 #ifndef __ASSEMBLY__
 
 #include <xen/init.h>
@@ -18,16 +20,24 @@ struct alt_instr {
 };
 
 /* Xen: helpers used by common code. */
-#define __ALT_PTR(a,f)         ((u32 *)((void *)&(a)->f + (a)->f))
+#define __ALT_PTR(a,f)         ((void *)&(a)->f + (a)->f)
 #define ALT_ORIG_PTR(a)                __ALT_PTR(a, orig_offset)
 #define ALT_REPL_PTR(a)                __ALT_PTR(a, alt_offset)
 
+typedef void (*alternative_cb_t)(const struct alt_instr *alt,
+                                const uint32_t *origptr, uint32_t *updptr,
+                                int nr_inst);
+
 void __init apply_alternatives_all(void);
 int apply_alternatives(const struct alt_instr *start, const struct alt_instr 
*end);
 
-#define ALTINSTR_ENTRY(feature)                                                
      \
+#define ALTINSTR_ENTRY(feature, cb)                                          \
        " .word 661b - .\n"                             /* label           */ \
+       " .if " __stringify(cb) " == 0\n"                                     \
        " .word 663f - .\n"                             /* new instruction */ \
+       " .else\n"                                                            \
+       " .word " __stringify(cb) "- .\n"               /* callback */        \
+       " .endif\n"                                                           \
        " .hword " __stringify(feature) "\n"            /* feature bit     */ \
        " .byte 662b-661b\n"                            /* source len      */ \
        " .byte 664f-663f\n"                            /* replacement len */
@@ -45,15 +55,18 @@ int apply_alternatives(const struct alt_instr *start, const 
struct alt_instr *en
  * but most assemblers die if insn1 or insn2 have a .inst. This should
  * be fixed in a binutils release posterior to 2.25.51.0.2 (anything
  * containing commit 4e4d08cf7399b606 or c1baaddf8861).
+ *
+ * Alternatives with callbacks do not generate replacement instructions.
  */
-#define __ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg_enabled)    \
+#define __ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg_enabled, cb)        
\
        ".if "__stringify(cfg_enabled)" == 1\n"                         \
        "661:\n\t"                                                      \
        oldinstr "\n"                                                   \
        "662:\n"                                                        \
        ".pushsection .altinstructions,\"a\"\n"                         \
-       ALTINSTR_ENTRY(feature)                                         \
+       ALTINSTR_ENTRY(feature,cb)                                      \
        ".popsection\n"                                                 \
+       " .if " __stringify(cb) " == 0\n"                               \
        ".pushsection .altinstr_replacement, \"a\"\n"                   \
        "663:\n\t"                                                      \
        newinstr "\n"                                                   \
@@ -61,11 +74,17 @@ int apply_alternatives(const struct alt_instr *start, const 
struct alt_instr *en
        ".popsection\n\t"                                               \
        ".org   . - (664b-663b) + (662b-661b)\n\t"                      \
        ".org   . - (662b-661b) + (664b-663b)\n"                        \
+       ".else\n\t"                                                     \
+       "663:\n\t"                                                      \
+       "664:\n\t"                                                      \
+       ".endif\n"                                                      \
        ".endif\n"
 
 #define _ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg, ...)        \
-       __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg))
+       __ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg), 0)
 
+#define ALTERNATIVE_CB(oldinstr, cb) \
+       __ALTERNATIVE_CFG(oldinstr, "NOT_AN_INSTRUCTION", ARM_CB_PATCH, 1, cb)
 #else
 
 #include <asm/asm_defns.h>
@@ -126,6 +145,14 @@ int apply_alternatives(const struct alt_instr *start, 
const struct alt_instr *en
 663:
 .endm
 
+.macro alternative_cb cb
+       .set .Lasm_alt_mode, 0
+       .pushsection .altinstructions, "a"
+       altinstruction_entry 661f, \cb, ARM_CB_PATCH, 662f-661f, 0
+       .popsection
+661:
+.endm
+
 /*
  * Complete an alternative code sequence.
  */
@@ -135,6 +162,13 @@ int apply_alternatives(const struct alt_instr *start, 
const struct alt_instr *en
        .org    . - (662b-661b) + (664b-663b)
 .endm
 
+/*
+ * Callback-based alternative epilogue
+ */
+.macro alternative_cb_end
+662:
+.endm
+
 #define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...)  \
        alternative_insn insn1, insn2, cap, IS_ENABLED(cfg)
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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