| Message ID | 20180522174254.27551-9-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: SSBD (aka Spectre-v4) mitigation (XSA-263) | expand |
On Tue, 22 May 2018, Julien Grall wrote: > 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@linaro.org> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > > This is patch of XSA-263. This patch is part of XSA-263. > Signed-off-by: Julien Grall <julien.grall@arm.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> > --- > 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 bd62183def..673150d1c0 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 >
diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c index bd62183def..673150d1c0 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)
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@linaro.org> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> This is patch of XSA-263. Signed-off-by: Julien Grall <julien.grall@arm.com> --- xen/arch/arm/alternative.c | 48 +++++++++++++++++++++++++++++---------- xen/include/asm-arm/alternative.h | 44 +++++++++++++++++++++++++++++++---- 2 files changed, 75 insertions(+), 17 deletions(-)