On Sat, Apr 04, 2020 at 12:08:08PM +0900, Masami Hiramatsu wrote: > From c609be0b6403245612503fca1087628655bab96c Mon Sep 17 00:00:00 2001 > From: Masami Hiramatsu <mhiramat@xxxxxxxxxx> > Date: Fri, 3 Apr 2020 16:58:22 +0900 > Subject: [PATCH] x86: insn: Add insn_is_fpu() > > Add insn_is_fpu(insn) which tells that the insn is > whether touch the MMX/XMM/YMM register or the instruction > of FP coprocessor. > > Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx> With that I get a lot of warnings: FPU instruction outside of kernel_fpu_{begin,end}() two random examples (x86-64-allmodconfig build): arch/x86/xen/enlighten.o: warning: objtool: xen_vcpu_restore()+0x341: FPU instruction outside of kernel_fpu_{begin,end}() $ ./objdump-func.sh defconfig-build/arch/x86/xen/enlighten.o xen_vcpu_restore | grep 341 0341 841: 0f 92 c3 setb %bl arch/x86/events/core.o: warning: objtool: x86_pmu_stop()+0x6d: FPU instruction outside of kernel_fpu_{begin,end}() $ ./objdump-func.sh defconfig-build/arch/x86/events/core.o x86_pmu_stop | grep 6d 006d 23ad: 41 0f 92 c6 setb %r14b Which seems to suggest something goes wobbly with SETB, but I'm not seeing what in a hurry. --- --- a/arch/x86/include/asm/fpu/api.h +++ b/arch/x86/include/asm/fpu/api.h @@ -12,6 +12,13 @@ #define _ASM_X86_FPU_API_H #include <linux/bottom_half.h> +#define annotate_fpu() ({ \ + asm volatile("%c0:\n\t" \ + ".pushsection .discard.fpu_safe\n\t" \ + ".long %c0b - .\n\t" \ + ".popsection\n\t" : : "i" (__COUNTER__)); \ +}) + /* * Use kernel_fpu_begin/end() if you intend to use FPU in kernel context. It * disables preemption so be careful if you intend to use it for long periods --- a/arch/x86/include/asm/fpu/internal.h +++ b/arch/x86/include/asm/fpu/internal.h @@ -437,6 +437,7 @@ static inline int copy_fpregs_to_fpstate * Legacy FPU register saving, FNSAVE always clears FPU registers, * so we have to mark them inactive: */ + annotate_fpu(); asm volatile("fnsave %[fp]; fwait" : [fp] "=m" (fpu->state.fsave)); return 0; @@ -462,6 +463,7 @@ static inline void copy_kernel_to_fpregs * "m" is a random variable that should be in L1. */ if (unlikely(static_cpu_has_bug(X86_BUG_FXSAVE_LEAK))) { + annotate_fpu(); asm volatile( "fnclex\n\t" "emms\n\t" --- a/arch/x86/kernel/fpu/init.c +++ b/arch/x86/kernel/fpu/init.c @@ -38,7 +38,10 @@ static void fpu__init_cpu_generic(void) fpstate_init_soft(¤t->thread.fpu.state.soft); else #endif + { + annotate_fpu(); asm volatile ("fninit"); + } } /* @@ -61,6 +64,7 @@ static bool fpu__probe_without_cpuid(voi cr0 &= ~(X86_CR0_TS | X86_CR0_EM); write_cr0(cr0); + annotate_fpu(); asm volatile("fninit ; fnstsw %0 ; fnstcw %1" : "+m" (fsw), "+m" (fcw)); pr_info("x86/fpu: Probing for FPU: FSW=0x%04hx FCW=0x%04hx\n", fsw, fcw); --- a/tools/objtool/arch.h +++ b/tools/objtool/arch.h @@ -27,6 +27,7 @@ enum insn_type { INSN_CLAC, INSN_STD, INSN_CLD, + INSN_FPU, INSN_OTHER, }; --- a/tools/objtool/arch/x86/decode.c +++ b/tools/objtool/arch/x86/decode.c @@ -92,6 +92,11 @@ int arch_decode_instruction(struct elf * *len = insn.length; *type = INSN_OTHER; + if (insn_is_fpu(&insn)) { + *type = INSN_FPU; + return 0; + } + if (insn.vex_prefix.nbytes) return 0; @@ -357,48 +362,54 @@ int arch_decode_instruction(struct elf * case 0x0f: - if (op2 == 0x01) { - + switch (op2) { + case 0x01: if (modrm == 0xca) *type = INSN_CLAC; else if (modrm == 0xcb) *type = INSN_STAC; + break; - } else if (op2 >= 0x80 && op2 <= 0x8f) { - + case 0x80 ... 0x8f: /* Jcc */ *type = INSN_JUMP_CONDITIONAL; + break; - } else if (op2 == 0x05 || op2 == 0x07 || op2 == 0x34 || - op2 == 0x35) { - - /* sysenter, sysret */ + case 0x05: /* syscall */ + case 0x07: /* sysret */ + case 0x34: /* sysenter */ + case 0x35: /* sysexit */ *type = INSN_CONTEXT_SWITCH; + break; - } else if (op2 == 0x0b || op2 == 0xb9) { - - /* ud2 */ + case 0xff: /* ud0 */ + case 0xb9: /* ud1 */ + case 0x0b: /* ud2 */ *type = INSN_BUG; + break; - } else if (op2 == 0x0d || op2 == 0x1f) { - + case 0x0d: + case 0x1f: /* nopl/nopw */ *type = INSN_NOP; + break; - } else if (op2 == 0xa0 || op2 == 0xa8) { - - /* push fs/gs */ + case 0xa0: /* push fs */ + case 0xa8: /* push gs */ *type = INSN_STACK; op->src.type = OP_SRC_CONST; op->dest.type = OP_DEST_PUSH; + break; - } else if (op2 == 0xa1 || op2 == 0xa9) { - - /* pop fs/gs */ + case 0xa1: /* pop fs */ + case 0xa9: /* pop gs */ *type = INSN_STACK; op->src.type = OP_SRC_POP; op->dest.type = OP_DEST_MEM; - } + break; + default: + break; + } break; case 0xc9: --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1316,6 +1316,43 @@ static int read_unwind_hints(struct objt return 0; } +static int read_fpu_hints(struct objtool_file *file) +{ + struct section *sec; + struct instruction *insn; + struct rela *rela; + + sec = find_section_by_name(file->elf, ".rela.discard.fpu_safe"); + if (!sec) + return 0; + + list_for_each_entry(rela, &sec->rela_list, list) { + if (rela->sym->type != STT_SECTION) { + WARN("unexpected relocation symbol type in %s", sec->name); + return -1; + } + + insn = find_insn(file, rela->sym->sec, rela->addend); + if (!insn) { + WARN("bad .discard.fpu_safe entry"); + return -1; + } + + if (insn->type != INSN_FPU) { + WARN_FUNC("fpu_safe hint not an FPU instruction", + insn->sec, insn->offset); +// return -1; + } + + while (insn && insn->type == INSN_FPU) { + insn->fpu_safe = true; + insn = next_insn_same_func(file, insn); + } + } + + return 0; +} + static int read_retpoline_hints(struct objtool_file *file) { struct section *sec; @@ -1422,6 +1459,10 @@ static int decode_sections(struct objtoo if (ret) return ret; + ret = read_fpu_hints(file); + if (ret) + return ret; + return 0; } @@ -2167,6 +2208,16 @@ static int validate_branch(struct objtoo if (dead_end_function(file, insn->call_dest)) return 0; + if (insn->call_dest) { + if (!strcmp(insn->call_dest->name, "kernel_fpu_begin") || + !strcmp(insn->call_dest->name, "emulator_get_fpu")) + state.fpu = true; + + if (!strcmp(insn->call_dest->name, "kernel_fpu_end") || + !strcmp(insn->call_dest->name, "emulator_put_fpu")) + state.fpu = false; + } + break; case INSN_JUMP_CONDITIONAL: @@ -2275,6 +2326,13 @@ static int validate_branch(struct objtoo state.df = false; break; + case INSN_FPU: + if (!state.fpu && !insn->fpu_safe) { + WARN_FUNC("FPU instruction outside of kernel_fpu_{begin,end}()", sec, insn->offset); + return 1; + } + break; + default: break; } --- a/tools/objtool/check.h +++ b/tools/objtool/check.h @@ -20,6 +20,7 @@ struct insn_state { unsigned char type; bool bp_scratch; bool drap, end, uaccess, df; + bool fpu; unsigned int uaccess_stack; int drap_reg, drap_offset; struct cfi_reg vals[CFI_NUM_REGS]; @@ -34,7 +35,7 @@ struct instruction { enum insn_type type; unsigned long immediate; bool alt_group, dead_end, ignore, hint, save, restore, ignore_alts; - bool retpoline_safe; + bool retpoline_safe, fpu_safe; u8 visited; struct symbol *call_dest; struct instruction *jump_dest; _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx