| Message ID | 1529656278-878-4-git-send-email-will.deacon@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Will, On Fri, Jun 22, 2018 at 09:31:17AM +0100, Will Deacon wrote: > When invalidating the instruction cache for a kernel mapping via > flush_icache_range(), it is also necessary to flush the pipeline for > other CPUs so that instructions fetched into the pipeline before the > I-cache invalidation are discarded. For example, if module 'foo' is > unloaded and then module 'bar' is loaded into the same area of memory, > a CPU could end up executing instructions from 'foo' when branching into > 'bar' if these instructions were fetched into the pipeline before 'foo' > was unloaded. > > Whilst this is highly unlikely to occur in practice, particularly as > any exception acts as a context-synchronizing operation, following the > letter of the architecture requires us to execute an ISB on each CPU > in order for the new instruction stream to be visible. > > Signed-off-by: Will Deacon <will.deacon@arm.com> I hit a warning via kgdb_flush_swbreak_addr() because if IRQs disabled (and it actually deadlocks for me; running as a guest under KVM on TX1): WARNING: CPU: 3 PID: 1 at /home/cmarinas/work/Linux/linux-2.6-aarch64/kernel/smp.c:416 smp_call_function_many+0xd4/0x350 Modules linked in: CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.18.0-rc2-00006-ga5cfc8429d36 #97 Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015 pstate: 200003c5 (nzCv DAIF -PAN -UAO) pc : smp_call_function_many+0xd4/0x350 lr : smp_call_function+0x38/0x68 sp : ffff0000080737f0 x29: ffff0000080737f0 x28: ffff000009169000 x27: 0000000000000003 x26: 0000000000000000 x25: ffff00000815a3d0 x24: 0000000000000001 x23: 0000000000000000 x22: ffff000008eba730 x21: ffff000009169940 x20: 0000000000000000 x19: 0000000000000003 x18: ffffffffffffffff x17: 0000000000000001 x16: 0000000000000019 x15: ffff0000091696c8 x14: ffff00008930ef07 x13: ffff00000930ef1d x12: 0000000000000010 x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f x9 : 656565652aff6223 x8 : ffffffffffffffff x7 : fefefefefefefefe x6 : ffff7dfffe7fe014 x5 : ffff000009169940 x4 : ffff000009147018 x3 : 0000000000000001 x2 : 0000000000000000 x1 : ffff00000815a3d0 x0 : 0000000000000000 Call trace: smp_call_function_many+0xd4/0x350 smp_call_function+0x38/0x68 kick_all_cpus_sync+0x20/0x28 kgdb_flush_swbreak_addr+0x14/0x20 dbg_activate_sw_breakpoints+0x74/0xb0 gdb_serial_stub+0x6ec/0xc80 kgdb_cpu_enter+0x36c/0x578 kgdb_handle_exception+0x130/0x238 kgdb_compiled_brk_fn+0x28/0x38 brk_handler+0x80/0xd8 do_debug_exception+0x94/0x170 el1_dbg+0x18/0x78 kgdb_breakpoint+0x20/0x40 run_breakpoint_test+0x58/0xb0 configure_kgdbts+0x1b8/0x538 init_kgdbts+0x1c/0x2c do_one_initcall+0x58/0x170 kernel_init_freeable+0x184/0x22c kernel_init+0x10/0x108 ret_from_fork+0x10/0x18
diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h index d264a7274811..a0ec27066e6f 100644 --- a/arch/arm64/include/asm/cacheflush.h +++ b/arch/arm64/include/asm/cacheflush.h @@ -71,7 +71,7 @@ * - kaddr - page address * - size - region size */ -extern void flush_icache_range(unsigned long start, unsigned long end); +extern void __flush_icache_range(unsigned long start, unsigned long end); extern int invalidate_icache_range(unsigned long start, unsigned long end); extern void __flush_dcache_area(void *addr, size_t len); extern void __inval_dcache_area(void *addr, size_t len); @@ -81,6 +81,17 @@ extern void __clean_dcache_area_pou(void *addr, size_t len); extern long __flush_cache_user_range(unsigned long start, unsigned long end); extern void sync_icache_aliases(void *kaddr, unsigned long len); +static inline void flush_icache_range(unsigned long start, unsigned long end) +{ + __flush_icache_range(start, end); + + /* + * IPI all online CPUs so that they undergo a context synchronization + * event and are forced to refetch the new instructions. + */ + kick_all_cpus_sync(); +} + static inline void flush_cache_mm(struct mm_struct *mm) { } diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c index 1d2b6d768efe..0a338a1cd2d7 100644 --- a/arch/arm64/kernel/cpu_errata.c +++ b/arch/arm64/kernel/cpu_errata.c @@ -101,7 +101,7 @@ static void __copy_hyp_vect_bpi(int slot, const char *hyp_vecs_start, for (i = 0; i < SZ_2K; i += 0x80) memcpy(dst + i, hyp_vecs_start, hyp_vecs_end - hyp_vecs_start); - flush_icache_range((uintptr_t)dst, (uintptr_t)dst + SZ_2K); + __flush_icache_range((uintptr_t)dst, (uintptr_t)dst + SZ_2K); } static void __install_bp_hardening_cb(bp_hardening_cb_t fn, diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c index 816d03c4c913..0f6a2e0cfde0 100644 --- a/arch/arm64/kernel/insn.c +++ b/arch/arm64/kernel/insn.c @@ -216,8 +216,8 @@ int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn) ret = aarch64_insn_write(tp, insn); if (ret == 0) - flush_icache_range((uintptr_t)tp, - (uintptr_t)tp + AARCH64_INSN_SIZE); + __flush_icache_range((uintptr_t)tp, + (uintptr_t)tp + AARCH64_INSN_SIZE); return ret; } @@ -283,18 +283,8 @@ int __kprobes aarch64_insn_patch_text(void *addrs[], u32 insns[], int cnt) if (ret) return ret; - if (aarch64_insn_hotpatch_safe(insn, insns[0])) { - /* - * ARMv8 architecture doesn't guarantee all CPUs see - * the new instruction after returning from function - * aarch64_insn_patch_text_nosync(). So send IPIs to - * all other CPUs to achieve instruction - * synchronization. - */ - ret = aarch64_insn_patch_text_nosync(addrs[0], insns[0]); - kick_all_cpus_sync(); - return ret; - } + if (aarch64_insn_hotpatch_safe(insn, insns[0])) + return aarch64_insn_patch_text_nosync(addrs[0], insns[0]); } return aarch64_insn_patch_text_sync(addrs, insns, cnt); diff --git a/arch/arm64/mm/cache.S b/arch/arm64/mm/cache.S index 30334d81b021..0c22ede52f90 100644 --- a/arch/arm64/mm/cache.S +++ b/arch/arm64/mm/cache.S @@ -35,7 +35,7 @@ * - start - virtual start address of region * - end - virtual end address of region */ -ENTRY(flush_icache_range) +ENTRY(__flush_icache_range) /* FALLTHROUGH */ /* @@ -77,7 +77,7 @@ alternative_else_nop_endif 9: mov x0, #-EFAULT b 1b -ENDPROC(flush_icache_range) +ENDPROC(__flush_icache_range) ENDPROC(__flush_cache_user_range) /*
When invalidating the instruction cache for a kernel mapping via flush_icache_range(), it is also necessary to flush the pipeline for other CPUs so that instructions fetched into the pipeline before the I-cache invalidation are discarded. For example, if module 'foo' is unloaded and then module 'bar' is loaded into the same area of memory, a CPU could end up executing instructions from 'foo' when branching into 'bar' if these instructions were fetched into the pipeline before 'foo' was unloaded. Whilst this is highly unlikely to occur in practice, particularly as any exception acts as a context-synchronizing operation, following the letter of the architecture requires us to execute an ISB on each CPU in order for the new instruction stream to be visible. Signed-off-by: Will Deacon <will.deacon@arm.com> --- arch/arm64/include/asm/cacheflush.h | 13 ++++++++++++- arch/arm64/kernel/cpu_errata.c | 2 +- arch/arm64/kernel/insn.c | 18 ++++-------------- arch/arm64/mm/cache.S | 4 ++-- 4 files changed, 19 insertions(+), 18 deletions(-)