From 9cbf2643b3ec7866e688df35d5b28c8b07ecbe6c Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Fri, 7 Jun 2024 13:16:48 +0200 Subject: [PATCH 01/15] x86/alternative: Zap alternative_ternary() Unused. Signed-off-by: Borislav Petkov (AMD) Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240607111701.8366-2-bp@kernel.org --- arch/x86/include/asm/alternative.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index ba99ef75f56c..6db78909180a 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -271,9 +271,6 @@ static inline int alternatives_text_reserved(void *start, void *end) #define alternative_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) \ asm_inline volatile(ALTERNATIVE_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) ::: "memory") -#define alternative_ternary(oldinstr, ft_flags, newinstr_yes, newinstr_no) \ - asm_inline volatile(ALTERNATIVE_TERNARY(oldinstr, ft_flags, newinstr_yes, newinstr_no) ::: "memory") - /* * Alternative inline assembly with input. * From d2a793dae219b7cd61a3d63c0a6ea76153f0629f Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Fri, 7 Jun 2024 13:16:49 +0200 Subject: [PATCH 02/15] x86/alternatives: Add nested alternatives macros Instead of making increasingly complicated ALTERNATIVE_n() implementations, use a nested alternative expression. The only difference between: ALTERNATIVE_2(oldinst, newinst1, flag1, newinst2, flag2) and ALTERNATIVE(ALTERNATIVE(oldinst, newinst1, flag1), newinst2, flag2) is that the outer alternative can add additional padding when the inner alternative is the shorter one, which then results in alt_instr::instrlen being inconsistent. However, this is easily remedied since the alt_instr entries will be consecutive and it is trivial to compute the max(alt_instr::instrlen) at runtime while patching. Specifically, after this the ALTERNATIVE_2 macro, after CPP expansion (and manual layout), looks like this: .macro ALTERNATIVE_2 oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2 740: 740: \oldinstr ; 741: .skip -(((744f-743f)-(741b-740b)) > 0) * ((744f-743f)-(741b-740b)),0x90 ; 742: .pushsection .altinstructions,"a" ; altinstr_entry 740b,743f,\ft_flags1,742b-740b,744f-743f ; .popsection ; .pushsection .altinstr_replacement,"ax" ; 743: \newinstr1 ; 744: .popsection ; ; 741: .skip -(((744f-743f)-(741b-740b)) > 0) * ((744f-743f)-(741b-740b)),0x90 ; 742: .pushsection .altinstructions,"a" ; altinstr_entry 740b,743f,\ft_flags2,742b-740b,744f-743f ; .popsection ; .pushsection .altinstr_replacement,"ax" ; 743: \newinstr2 ; 744: .popsection ; .endm The only label that is ambiguous is 740, however they all reference the same spot, so that doesn't matter. NOTE: obviously only @oldinstr may be an alternative; making @newinstr an alternative would mean patching .altinstr_replacement which very likely isn't what is intended, also the labels will be confused in that case. [ bp: Debug an issue where it would match the wrong two insns and and consider them nested due to the same signed offsets in the .alternative section and use instr_va() to compare the full virtual addresses instead. - Use new labels to denote that the new, nested alternatives are being used when staring at preprocessed output. - Use the %c constraint everywhere instead of %P and document the difference for future reference. ] Signed-off-by: Peter Zijlstra Co-developed-by: Borislav Petkov (AMD) Signed-off-by: Borislav Petkov (AMD) Acked-by: Peter Zijlstra (Intel) Link: https://lkml.kernel.org/r/20230628104952.GA2439977@hirez.programming.kicks-ass.net --- arch/x86/include/asm/alternative.h | 120 ++++++++++++++++++++++++++++- arch/x86/kernel/alternative.c | 20 ++++- tools/objtool/arch/x86/special.c | 23 ++++++ tools/objtool/special.c | 16 ++-- 4 files changed, 167 insertions(+), 12 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 6db78909180a..7d9ebc516bde 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -161,8 +161,13 @@ static inline int alternatives_text_reserved(void *start, void *end) #define alt_end_marker "663" #define alt_slen "662b-661b" +#define n_alt_slen "772b-771b" + #define alt_total_slen alt_end_marker"b-661b" +#define n_alt_total_slen "773b-771b" + #define alt_rlen(num) e_replacement(num)"f-"b_replacement(num)"f" +#define n_alt_rlen "775f-774f" #define OLDINSTR(oldinstr, num) \ "# ALT: oldnstr\n" \ @@ -172,6 +177,14 @@ static inline int alternatives_text_reserved(void *start, void *end) "((" alt_rlen(num) ")-(" alt_slen ")),0x90\n" \ alt_end_marker ":\n" +#define N_OLDINSTR(oldinstr) \ + "# N_ALT: oldinstr\n" \ + "771:\n\t" oldinstr "\n772:\n" \ + "# N_ALT: padding\n" \ + ".skip -(((" n_alt_rlen ")-(" n_alt_slen ")) > 0) * " \ + "((" n_alt_rlen ")-(" n_alt_slen ")),0x90\n" \ + "773:\n" + /* * gas compatible max based on the idea from: * http://graphics.stanford.edu/~seander/bithacks.html#IntegerMinOrMax @@ -209,10 +222,25 @@ static inline int alternatives_text_reserved(void *start, void *end) " .byte " alt_total_slen "\n" /* source len */ \ " .byte " alt_rlen(num) "\n" /* replacement len */ +#define N_ALTINSTR_ENTRY(ft_flags) \ + ".pushsection .altinstructions,\"a\"\n" \ + " .long 771b - .\n" /* label */ \ + " .long 774f - .\n" /* new instruction */ \ + " .4byte " __stringify(ft_flags) "\n" /* feature + flags */ \ + " .byte " n_alt_total_slen "\n" /* source len */ \ + " .byte " n_alt_rlen "\n" /* replacement len */ \ + ".popsection\n" + #define ALTINSTR_REPLACEMENT(newinstr, num) /* replacement */ \ "# ALT: replacement " #num "\n" \ b_replacement(num)":\n\t" newinstr "\n" e_replacement(num) ":\n" +#define N_ALTINSTR_REPLACEMENT(newinstr) /* replacement */ \ + ".pushsection .altinstr_replacement, \"ax\"\n" \ + "# N_ALT: replacement\n" \ + "774:\n\t" newinstr "\n775:\n" \ + ".popsection\n" + /* alternative assembly primitive: */ #define ALTERNATIVE(oldinstr, newinstr, ft_flags) \ OLDINSTR(oldinstr, 1) \ @@ -223,6 +251,12 @@ static inline int alternatives_text_reserved(void *start, void *end) ALTINSTR_REPLACEMENT(newinstr, 1) \ ".popsection\n" +/* Nested alternatives macro variant */ +#define N_ALTERNATIVE(oldinstr, newinstr, ft_flags) \ + N_OLDINSTR(oldinstr) \ + N_ALTINSTR_ENTRY(ft_flags) \ + N_ALTINSTR_REPLACEMENT(newinstr) + #define ALTERNATIVE_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) \ OLDINSTR_2(oldinstr, 1, 2) \ ".pushsection .altinstructions,\"a\"\n" \ @@ -234,11 +268,21 @@ static inline int alternatives_text_reserved(void *start, void *end) ALTINSTR_REPLACEMENT(newinstr2, 2) \ ".popsection\n" +#define N_ALTERNATIVE_2(oldinst, newinst1, flag1, newinst2, flag2) \ + N_ALTERNATIVE(N_ALTERNATIVE(oldinst, newinst1, flag1), \ + newinst2, flag2) + /* If @feature is set, patch in @newinstr_yes, otherwise @newinstr_no. */ #define ALTERNATIVE_TERNARY(oldinstr, ft_flags, newinstr_yes, newinstr_no) \ ALTERNATIVE_2(oldinstr, newinstr_no, X86_FEATURE_ALWAYS, \ newinstr_yes, ft_flags) +/* If @feature is set, patch in @newinstr_yes, otherwise @newinstr_no. */ +#define N_ALTERNATIVE_TERNARY(oldinstr, ft_flags, newinstr_yes, newinstr_no) \ + N_ALTERNATIVE_2(oldinstr, newinstr_no, X86_FEATURE_ALWAYS, \ + newinstr_yes, ft_flags) + + #define ALTERNATIVE_3(oldinsn, newinsn1, ft_flags1, newinsn2, ft_flags2, \ newinsn3, ft_flags3) \ OLDINSTR_3(oldinsn, 1, 2, 3) \ @@ -253,6 +297,12 @@ static inline int alternatives_text_reserved(void *start, void *end) ALTINSTR_REPLACEMENT(newinsn3, 3) \ ".popsection\n" + +#define N_ALTERNATIVE_3(oldinst, newinst1, flag1, newinst2, flag2, \ + newinst3, flag3) \ + N_ALTERNATIVE(N_ALTERNATIVE_2(oldinst, newinst1, flag1, newinst2, flag2), \ + newinst3, flag3) + /* * Alternative instructions for different CPU types or capabilities. * @@ -271,6 +321,12 @@ static inline int alternatives_text_reserved(void *start, void *end) #define alternative_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) \ asm_inline volatile(ALTERNATIVE_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) ::: "memory") +#define n_alternative(oldinstr, newinstr, ft_flags) \ + asm_inline volatile (N_ALTERNATIVE(oldinstr, newinstr, ft_flags) : : : "memory") + +#define n_alternative_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) \ + asm_inline volatile(N_ALTERNATIVE_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) ::: "memory") + /* * Alternative inline assembly with input. * @@ -288,11 +344,32 @@ static inline int alternatives_text_reserved(void *start, void *end) asm_inline volatile (ALTERNATIVE(oldinstr, newinstr, ft_flags) \ : output : "i" (0), ## input) -/* Like alternative_io, but for replacing a direct call with another one. */ +#define n_alternative_io(oldinstr, newinstr, ft_flags, output, input...) \ + asm_inline volatile (N_ALTERNATIVE(oldinstr, newinstr, ft_flags) \ + : output : "i" (0), ## input) + +/* + * Like alternative_io, but for replacing a direct call with another one. + * + * Use the %c operand modifier which is the generic way to print a bare + * constant expression with all syntax-specific punctuation omitted. %P + * is the x86-specific variant which can handle constants too, for + * historical reasons, but it should be used primarily for PIC + * references: i.e., if used for a function, it would add the PLT + * suffix. + */ #define alternative_call(oldfunc, newfunc, ft_flags, output, input...) \ asm_inline volatile (ALTERNATIVE("call %c[old]", "call %c[new]", ft_flags) \ : output : [old] "i" (oldfunc), [new] "i" (newfunc), ## input) +#define n_alternative_input(oldinstr, newinstr, ft_flags, input...) \ + asm_inline volatile (N_ALTERNATIVE(oldinstr, newinstr, ft_flags) \ + : : "i" (0), ## input) + +#define n_alternative_call(oldfunc, newfunc, ft_flags, output, input...) \ + asm_inline volatile (N_ALTERNATIVE("call %c[old]", "call %c[new]", ft_flags) \ + : output : [old] "i" (oldfunc), [new] "i" (newfunc), ## input) + /* * Like alternative_call, but there are two features and respective functions. * If CPU has feature2, function2 is used. @@ -307,6 +384,15 @@ static inline int alternatives_text_reserved(void *start, void *end) : [old] "i" (oldfunc), [new1] "i" (newfunc1), \ [new2] "i" (newfunc2), ## input) +#define n_alternative_call_2(oldfunc, newfunc1, ft_flags1, newfunc2, ft_flags2, \ + output, input...) \ + asm_inline volatile (N_ALTERNATIVE_2("call %c[old]", "call %c[new1]", ft_flags1,\ + "call %c[new2]", ft_flags2) \ + : output, ASM_CALL_CONSTRAINT \ + : [old] "i" (oldfunc), [new1] "i" (newfunc1), \ + [new2] "i" (newfunc2), ## input) + + /* * use this macro(s) if you need more than one output parameter * in alternative_io @@ -403,6 +489,27 @@ void nop_func(void); .popsection .endm +#define __N_ALTERNATIVE(oldinst, newinst, flag) \ +740: \ + oldinst ; \ +741: \ + .skip -(((744f-743f)-(741b-740b)) > 0) * ((744f-743f)-(741b-740b)),0x90 ;\ +742: \ + .pushsection .altinstructions,"a" ; \ + altinstr_entry 740b,743f,flag,742b-740b,744f-743f ; \ + .popsection ; \ + .pushsection .altinstr_replacement,"ax" ; \ +743: \ + newinst ; \ +744: \ + .popsection ; + + +.macro N_ALTERNATIVE oldinstr, newinstr, ft_flags + __N_ALTERNATIVE(\oldinstr, \newinstr, \ft_flags) +.endm + + #define old_len 141b-140b #define new_len1 144f-143f #define new_len2 145f-144f @@ -417,7 +524,6 @@ void nop_func(void); #define alt_max_2(a, b) ((a) ^ (((a) ^ (b)) & -(-((a) < (b))))) #define alt_max_3(a, b, c) (alt_max_2(alt_max_2(a, b), c)) - /* * Same as ALTERNATIVE macro above but for two alternatives. If CPU * has @feature1, it replaces @oldinstr with @newinstr1. If CPU has @@ -445,6 +551,11 @@ void nop_func(void); .popsection .endm +.macro N_ALTERNATIVE_2 oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2 + __N_ALTERNATIVE(__N_ALTERNATIVE(\oldinstr, \newinstr1, \ft_flags1), + \newinstr2, \ft_flags2) +.endm + .macro ALTERNATIVE_3 oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2, newinstr3, ft_flags3 140: \oldinstr @@ -470,6 +581,11 @@ void nop_func(void); .popsection .endm +.macro N_ALTERNATIVE_3 oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2, newinstr3, ft_flags3 + __N_ALTERNATIVE(N_ALTERNATIVE_2(\oldinstr, \newinstr1, \ft_flags1, \newinstr2, \ft_flags2), + \newinstr3, \ft_flags3) +.endm + /* If @feature is set, patch in @newinstr_yes, otherwise @newinstr_no. */ #define ALTERNATIVE_TERNARY(oldinstr, ft_flags, newinstr_yes, newinstr_no) \ ALTERNATIVE_2 oldinstr, newinstr_no, X86_FEATURE_ALWAYS, \ diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index 89de61243272..37596a417094 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -432,6 +432,11 @@ static int alt_replace_call(u8 *instr, u8 *insn_buff, struct alt_instr *a) return 5; } +static inline u8 * instr_va(struct alt_instr *i) +{ + return (u8 *)&i->instr_offset + i->instr_offset; +} + /* * Replace instructions with better alternatives for this CPU type. This runs * before SMP is initialized to avoid SMP problems with self modifying code. @@ -447,7 +452,7 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start, { u8 insn_buff[MAX_PATCH_LEN]; u8 *instr, *replacement; - struct alt_instr *a; + struct alt_instr *a, *b; DPRINTK(ALT, "alt table %px, -> %px", start, end); @@ -473,7 +478,18 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start, for (a = start; a < end; a++) { int insn_buff_sz = 0; - instr = (u8 *)&a->instr_offset + a->instr_offset; + /* + * In case of nested ALTERNATIVE()s the outer alternative might + * add more padding. To ensure consistent patching find the max + * padding for all alt_instr entries for this site (nested + * alternatives result in consecutive entries). + */ + for (b = a+1; b < end && instr_va(b) == instr_va(a); b++) { + u8 len = max(a->instrlen, b->instrlen); + a->instrlen = b->instrlen = len; + } + + instr = instr_va(a); replacement = (u8 *)&a->repl_offset + a->repl_offset; BUG_ON(a->instrlen > sizeof(insn_buff)); BUG_ON(a->cpuid >= (NCAPINTS + NBUGINTS) * 32); diff --git a/tools/objtool/arch/x86/special.c b/tools/objtool/arch/x86/special.c index 4134d27c696b..4ea0f9815fda 100644 --- a/tools/objtool/arch/x86/special.c +++ b/tools/objtool/arch/x86/special.c @@ -9,6 +9,29 @@ void arch_handle_alternative(unsigned short feature, struct special_alt *alt) { + static struct special_alt *group, *prev; + + /* + * Recompute orig_len for nested ALTERNATIVE()s. + */ + if (group && group->orig_sec == alt->orig_sec && + group->orig_off == alt->orig_off) { + + struct special_alt *iter = group; + for (;;) { + unsigned int len = max(iter->orig_len, alt->orig_len); + iter->orig_len = alt->orig_len = len; + + if (iter == prev) + break; + + iter = list_next_entry(iter, list); + } + + } else group = alt; + + prev = alt; + switch (feature) { case X86_FEATURE_SMAP: /* diff --git a/tools/objtool/special.c b/tools/objtool/special.c index 91b1950f5bd8..097a69db82a0 100644 --- a/tools/objtool/special.c +++ b/tools/objtool/special.c @@ -84,6 +84,14 @@ static int get_alt_entry(struct elf *elf, const struct special_entry *entry, entry->new_len); } + orig_reloc = find_reloc_by_dest(elf, sec, offset + entry->orig); + if (!orig_reloc) { + WARN_FUNC("can't find orig reloc", sec, offset + entry->orig); + return -1; + } + + reloc_to_sec_off(orig_reloc, &alt->orig_sec, &alt->orig_off); + if (entry->feature) { unsigned short feature; @@ -94,14 +102,6 @@ static int get_alt_entry(struct elf *elf, const struct special_entry *entry, arch_handle_alternative(feature, alt); } - orig_reloc = find_reloc_by_dest(elf, sec, offset + entry->orig); - if (!orig_reloc) { - WARN_FUNC("can't find orig reloc", sec, offset + entry->orig); - return -1; - } - - reloc_to_sec_off(orig_reloc, &alt->orig_sec, &alt->orig_off); - if (!entry->group || alt->new_len) { new_reloc = find_reloc_by_dest(elf, sec, offset + entry->new); if (!new_reloc) { From b94c1fe10be544070f53e4a1e5164087e446945c Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Fri, 7 Jun 2024 13:16:50 +0200 Subject: [PATCH 03/15] x86/alternative: Convert alternative() Split conversion deliberately into minimal pieces to ease bisection because debugging alternatives is a nightmare. Signed-off-by: Borislav Petkov (AMD) Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240607111701.8366-4-bp@kernel.org --- arch/x86/include/asm/alternative.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 7d9ebc516bde..c05bff2e3626 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -316,14 +316,11 @@ static inline int alternatives_text_reserved(void *start, void *end) * without volatile and memory clobber. */ #define alternative(oldinstr, newinstr, ft_flags) \ - asm_inline volatile (ALTERNATIVE(oldinstr, newinstr, ft_flags) : : : "memory") + asm_inline volatile(N_ALTERNATIVE(oldinstr, newinstr, ft_flags) : : : "memory") #define alternative_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) \ asm_inline volatile(ALTERNATIVE_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) ::: "memory") -#define n_alternative(oldinstr, newinstr, ft_flags) \ - asm_inline volatile (N_ALTERNATIVE(oldinstr, newinstr, ft_flags) : : : "memory") - #define n_alternative_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) \ asm_inline volatile(N_ALTERNATIVE_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) ::: "memory") From 8cb1f14b707d3d9b7d2b330c0acd62537576c6a7 Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Fri, 7 Jun 2024 13:16:51 +0200 Subject: [PATCH 04/15] x86/alternative: Convert alternative_2() Signed-off-by: Borislav Petkov (AMD) Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240607111701.8366-5-bp@kernel.org --- arch/x86/include/asm/alternative.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index c05bff2e3626..21ead5dea15a 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -319,9 +319,6 @@ static inline int alternatives_text_reserved(void *start, void *end) asm_inline volatile(N_ALTERNATIVE(oldinstr, newinstr, ft_flags) : : : "memory") #define alternative_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) \ - asm_inline volatile(ALTERNATIVE_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) ::: "memory") - -#define n_alternative_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) \ asm_inline volatile(N_ALTERNATIVE_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) ::: "memory") /* From bb91576965e79d17e8e23fccdef91c4bf09d1a74 Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Fri, 7 Jun 2024 13:16:52 +0200 Subject: [PATCH 05/15] x86/alternative: Convert alternative_input() Signed-off-by: Borislav Petkov (AMD) Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240607111701.8366-6-bp@kernel.org --- arch/x86/include/asm/alternative.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 21ead5dea15a..428d6efeb333 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -330,7 +330,7 @@ static inline int alternatives_text_reserved(void *start, void *end) * Leaving an unused argument 0 to keep API compatibility. */ #define alternative_input(oldinstr, newinstr, ft_flags, input...) \ - asm_inline volatile (ALTERNATIVE(oldinstr, newinstr, ft_flags) \ + asm_inline volatile(N_ALTERNATIVE(oldinstr, newinstr, ft_flags) \ : : "i" (0), ## input) /* Like alternative_input, but with a single output argument */ @@ -356,10 +356,6 @@ static inline int alternatives_text_reserved(void *start, void *end) asm_inline volatile (ALTERNATIVE("call %c[old]", "call %c[new]", ft_flags) \ : output : [old] "i" (oldfunc), [new] "i" (newfunc), ## input) -#define n_alternative_input(oldinstr, newinstr, ft_flags, input...) \ - asm_inline volatile (N_ALTERNATIVE(oldinstr, newinstr, ft_flags) \ - : : "i" (0), ## input) - #define n_alternative_call(oldfunc, newfunc, ft_flags, output, input...) \ asm_inline volatile (N_ALTERNATIVE("call %c[old]", "call %c[new]", ft_flags) \ : output : [old] "i" (oldfunc), [new] "i" (newfunc), ## input) From ad36085ee3563001cf1720eeb93c2e85715eb5cd Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Fri, 7 Jun 2024 13:16:53 +0200 Subject: [PATCH 06/15] x86/alternative: Convert alternative_io() Signed-off-by: Borislav Petkov (AMD) Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240607111701.8366-7-bp@kernel.org --- arch/x86/include/asm/alternative.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 428d6efeb333..6a746811c602 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -335,11 +335,7 @@ static inline int alternatives_text_reserved(void *start, void *end) /* Like alternative_input, but with a single output argument */ #define alternative_io(oldinstr, newinstr, ft_flags, output, input...) \ - asm_inline volatile (ALTERNATIVE(oldinstr, newinstr, ft_flags) \ - : output : "i" (0), ## input) - -#define n_alternative_io(oldinstr, newinstr, ft_flags, output, input...) \ - asm_inline volatile (N_ALTERNATIVE(oldinstr, newinstr, ft_flags) \ + asm_inline volatile(N_ALTERNATIVE(oldinstr, newinstr, ft_flags) \ : output : "i" (0), ## input) /* From a880f9ef6bf7edc36753a9e40388d42ae8d7e099 Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Fri, 7 Jun 2024 13:16:54 +0200 Subject: [PATCH 07/15] x86/alternative: Convert alternative_call() Signed-off-by: Borislav Petkov (AMD) Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240607111701.8366-8-bp@kernel.org --- arch/x86/include/asm/alternative.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 6a746811c602..b6597574fa43 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -349,11 +349,7 @@ static inline int alternatives_text_reserved(void *start, void *end) * suffix. */ #define alternative_call(oldfunc, newfunc, ft_flags, output, input...) \ - asm_inline volatile (ALTERNATIVE("call %c[old]", "call %c[new]", ft_flags) \ - : output : [old] "i" (oldfunc), [new] "i" (newfunc), ## input) - -#define n_alternative_call(oldfunc, newfunc, ft_flags, output, input...) \ - asm_inline volatile (N_ALTERNATIVE("call %c[old]", "call %c[new]", ft_flags) \ + asm_inline volatile(N_ALTERNATIVE("call %c[old]", "call %c[new]", ft_flags) \ : output : [old] "i" (oldfunc), [new] "i" (newfunc), ## input) /* From 719ac02347ee5f94a9f3d5c2fe84640f855432a9 Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Fri, 7 Jun 2024 13:16:55 +0200 Subject: [PATCH 08/15] x86/alternative: Convert alternative_call_2() Signed-off-by: Borislav Petkov (AMD) Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240607111701.8366-9-bp@kernel.org --- arch/x86/include/asm/alternative.h | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index b6597574fa43..bc260f27d7f1 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -358,23 +358,14 @@ static inline int alternatives_text_reserved(void *start, void *end) * Otherwise, if CPU has feature1, function1 is used. * Otherwise, old function is used. */ -#define alternative_call_2(oldfunc, newfunc1, ft_flags1, newfunc2, ft_flags2, \ - output, input...) \ - asm_inline volatile (ALTERNATIVE_2("call %c[old]", "call %c[new1]", ft_flags1, \ - "call %c[new2]", ft_flags2) \ - : output, ASM_CALL_CONSTRAINT \ - : [old] "i" (oldfunc), [new1] "i" (newfunc1), \ +#define alternative_call_2(oldfunc, newfunc1, ft_flags1, newfunc2, ft_flags2, \ + output, input...) \ + asm_inline volatile(N_ALTERNATIVE_2("call %c[old]", "call %c[new1]", ft_flags1, \ + "call %c[new2]", ft_flags2) \ + : output, ASM_CALL_CONSTRAINT \ + : [old] "i" (oldfunc), [new1] "i" (newfunc1), \ [new2] "i" (newfunc2), ## input) -#define n_alternative_call_2(oldfunc, newfunc1, ft_flags1, newfunc2, ft_flags2, \ - output, input...) \ - asm_inline volatile (N_ALTERNATIVE_2("call %c[old]", "call %c[new1]", ft_flags1,\ - "call %c[new2]", ft_flags2) \ - : output, ASM_CALL_CONSTRAINT \ - : [old] "i" (oldfunc), [new1] "i" (newfunc1), \ - [new2] "i" (newfunc2), ## input) - - /* * use this macro(s) if you need more than one output parameter * in alternative_io From d2d302b1bbe28dba3bd8da855ac9c16aa5dbd00e Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Fri, 7 Jun 2024 13:16:56 +0200 Subject: [PATCH 09/15] x86/alternative: Convert ALTERNATIVE_TERNARY() The C macro. Signed-off-by: Borislav Petkov (AMD) Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240607111701.8366-10-bp@kernel.org --- arch/x86/include/asm/alternative.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index bc260f27d7f1..007baab32c14 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -274,15 +274,9 @@ static inline int alternatives_text_reserved(void *start, void *end) /* If @feature is set, patch in @newinstr_yes, otherwise @newinstr_no. */ #define ALTERNATIVE_TERNARY(oldinstr, ft_flags, newinstr_yes, newinstr_no) \ - ALTERNATIVE_2(oldinstr, newinstr_no, X86_FEATURE_ALWAYS, \ - newinstr_yes, ft_flags) - -/* If @feature is set, patch in @newinstr_yes, otherwise @newinstr_no. */ -#define N_ALTERNATIVE_TERNARY(oldinstr, ft_flags, newinstr_yes, newinstr_no) \ N_ALTERNATIVE_2(oldinstr, newinstr_no, X86_FEATURE_ALWAYS, \ newinstr_yes, ft_flags) - #define ALTERNATIVE_3(oldinsn, newinsn1, ft_flags1, newinsn2, ft_flags2, \ newinsn3, ft_flags3) \ OLDINSTR_3(oldinsn, 1, 2, 3) \ From 93694129c6e84d3013f5b2787e2ff88dd706c4f0 Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Fri, 7 Jun 2024 13:16:57 +0200 Subject: [PATCH 10/15] x86/alternative: Convert ALTERNATIVE_3() Zap the hack of using an ALTERNATIVE_3() internal label, as suggested by bgerst: https://lore.kernel.org/r/CAMzpN2i4oJ-Dv0qO46Fd-DxNv5z9=x%2BvO%2B8g=47NiiAf8QEJYA@mail.gmail.com in favor of a label local to this macro only, as it should be done. Signed-off-by: Borislav Petkov (AMD) Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240607111701.8366-11-bp@kernel.org --- arch/x86/include/asm/alternative.h | 24 ++++-------------------- arch/x86/kernel/fpu/xstate.h | 14 +++++--------- 2 files changed, 9 insertions(+), 29 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 007baab32c14..fba12ad237e2 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -277,26 +277,10 @@ static inline int alternatives_text_reserved(void *start, void *end) N_ALTERNATIVE_2(oldinstr, newinstr_no, X86_FEATURE_ALWAYS, \ newinstr_yes, ft_flags) -#define ALTERNATIVE_3(oldinsn, newinsn1, ft_flags1, newinsn2, ft_flags2, \ - newinsn3, ft_flags3) \ - OLDINSTR_3(oldinsn, 1, 2, 3) \ - ".pushsection .altinstructions,\"a\"\n" \ - ALTINSTR_ENTRY(ft_flags1, 1) \ - ALTINSTR_ENTRY(ft_flags2, 2) \ - ALTINSTR_ENTRY(ft_flags3, 3) \ - ".popsection\n" \ - ".pushsection .altinstr_replacement, \"ax\"\n" \ - ALTINSTR_REPLACEMENT(newinsn1, 1) \ - ALTINSTR_REPLACEMENT(newinsn2, 2) \ - ALTINSTR_REPLACEMENT(newinsn3, 3) \ - ".popsection\n" - - -#define N_ALTERNATIVE_3(oldinst, newinst1, flag1, newinst2, flag2, \ - newinst3, flag3) \ - N_ALTERNATIVE(N_ALTERNATIVE_2(oldinst, newinst1, flag1, newinst2, flag2), \ - newinst3, flag3) - +#define ALTERNATIVE_3(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2, \ + newinstr3, ft_flags3) \ + N_ALTERNATIVE(N_ALTERNATIVE_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2), \ + newinstr3, ft_flags3) /* * Alternative instructions for different CPU types or capabilities. * diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h index 05df04f39628..2ee0b9c53dcc 100644 --- a/arch/x86/kernel/fpu/xstate.h +++ b/arch/x86/kernel/fpu/xstate.h @@ -106,21 +106,17 @@ static inline u64 xfeatures_mask_independent(void) * Otherwise, if XSAVEOPT is enabled, XSAVEOPT replaces XSAVE because XSAVEOPT * supports modified optimization which is not supported by XSAVE. * - * We use XSAVE as a fallback. - * - * The 661 label is defined in the ALTERNATIVE* macros as the address of the - * original instruction which gets replaced. We need to use it here as the - * address of the instruction where we might get an exception at. + * Use XSAVE as a fallback. */ #define XSTATE_XSAVE(st, lmask, hmask, err) \ - asm volatile(ALTERNATIVE_3(XSAVE, \ + asm volatile("1: " ALTERNATIVE_3(XSAVE, \ XSAVEOPT, X86_FEATURE_XSAVEOPT, \ XSAVEC, X86_FEATURE_XSAVEC, \ XSAVES, X86_FEATURE_XSAVES) \ "\n" \ "xor %[err], %[err]\n" \ "3:\n" \ - _ASM_EXTABLE_TYPE_REG(661b, 3b, EX_TYPE_EFAULT_REG, %[err]) \ + _ASM_EXTABLE_TYPE_REG(1b, 3b, EX_TYPE_EFAULT_REG, %[err]) \ : [err] "=r" (err) \ : "D" (st), "m" (*st), "a" (lmask), "d" (hmask) \ : "memory") @@ -130,11 +126,11 @@ static inline u64 xfeatures_mask_independent(void) * XSAVE area format. */ #define XSTATE_XRESTORE(st, lmask, hmask) \ - asm volatile(ALTERNATIVE(XRSTOR, \ + asm volatile("1: " ALTERNATIVE(XRSTOR, \ XRSTORS, X86_FEATURE_XSAVES) \ "\n" \ "3:\n" \ - _ASM_EXTABLE_TYPE(661b, 3b, EX_TYPE_FPU_RESTORE) \ + _ASM_EXTABLE_TYPE(1b, 3b, EX_TYPE_FPU_RESTORE) \ : \ : "D" (st), "m" (*st), "a" (lmask), "d" (hmask) \ : "memory") From a6c7a6a18b10a4ac0913e7abdbdce928a2601bdb Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Fri, 7 Jun 2024 13:16:58 +0200 Subject: [PATCH 11/15] x86/alternative: Convert the asm ALTERNATIVE() macro Signed-off-by: Borislav Petkov (AMD) Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240607111701.8366-12-bp@kernel.org --- arch/x86/include/asm/alternative.h | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index fba12ad237e2..31b9a47b9df9 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -422,24 +422,6 @@ void nop_func(void); * @newinstr. ".skip" directive takes care of proper instruction padding * in case @newinstr is longer than @oldinstr. */ -.macro ALTERNATIVE oldinstr, newinstr, ft_flags -140: - \oldinstr -141: - .skip -(((144f-143f)-(141b-140b)) > 0) * ((144f-143f)-(141b-140b)),0x90 -142: - - .pushsection .altinstructions,"a" - altinstr_entry 140b,143f,\ft_flags,142b-140b,144f-143f - .popsection - - .pushsection .altinstr_replacement,"ax" -143: - \newinstr -144: - .popsection -.endm - #define __N_ALTERNATIVE(oldinst, newinst, flag) \ 740: \ oldinst ; \ @@ -455,12 +437,10 @@ void nop_func(void); 744: \ .popsection ; - -.macro N_ALTERNATIVE oldinstr, newinstr, ft_flags +.macro ALTERNATIVE oldinstr, newinstr, ft_flags __N_ALTERNATIVE(\oldinstr, \newinstr, \ft_flags) .endm - #define old_len 141b-140b #define new_len1 144f-143f #define new_len2 145f-144f From 08a621fcf4a4a4d765315ffe6a28f5d31e8237b2 Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Fri, 7 Jun 2024 13:16:59 +0200 Subject: [PATCH 12/15] x86/alternative: Convert the asm ALTERNATIVE_2() macro Signed-off-by: Borislav Petkov (AMD) Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240607111701.8366-13-bp@kernel.org --- arch/x86/include/asm/alternative.h | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 31b9a47b9df9..28e07a038964 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -461,28 +461,6 @@ void nop_func(void); * @feature2, it replaces @oldinstr with @feature2. */ .macro ALTERNATIVE_2 oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2 -140: - \oldinstr -141: - .skip -((alt_max_2(new_len1, new_len2) - (old_len)) > 0) * \ - (alt_max_2(new_len1, new_len2) - (old_len)),0x90 -142: - - .pushsection .altinstructions,"a" - altinstr_entry 140b,143f,\ft_flags1,142b-140b,144f-143f - altinstr_entry 140b,144f,\ft_flags2,142b-140b,145f-144f - .popsection - - .pushsection .altinstr_replacement,"ax" -143: - \newinstr1 -144: - \newinstr2 -145: - .popsection -.endm - -.macro N_ALTERNATIVE_2 oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2 __N_ALTERNATIVE(__N_ALTERNATIVE(\oldinstr, \newinstr1, \ft_flags1), \newinstr2, \ft_flags2) .endm From 1a6ade825079243f08846870561aca0e1fdfb803 Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Fri, 7 Jun 2024 13:17:00 +0200 Subject: [PATCH 13/15] x86/alternative: Convert the asm ALTERNATIVE_3() macro Signed-off-by: Borislav Petkov (AMD) Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240607111701.8366-14-bp@kernel.org --- arch/x86/include/asm/alternative.h | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 28e07a038964..5278cfb1f745 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -466,31 +466,6 @@ void nop_func(void); .endm .macro ALTERNATIVE_3 oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2, newinstr3, ft_flags3 -140: - \oldinstr -141: - .skip -((alt_max_3(new_len1, new_len2, new_len3) - (old_len)) > 0) * \ - (alt_max_3(new_len1, new_len2, new_len3) - (old_len)),0x90 -142: - - .pushsection .altinstructions,"a" - altinstr_entry 140b,143f,\ft_flags1,142b-140b,144f-143f - altinstr_entry 140b,144f,\ft_flags2,142b-140b,145f-144f - altinstr_entry 140b,145f,\ft_flags3,142b-140b,146f-145f - .popsection - - .pushsection .altinstr_replacement,"ax" -143: - \newinstr1 -144: - \newinstr2 -145: - \newinstr3 -146: - .popsection -.endm - -.macro N_ALTERNATIVE_3 oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2, newinstr3, ft_flags3 __N_ALTERNATIVE(N_ALTERNATIVE_2(\oldinstr, \newinstr1, \ft_flags1, \newinstr2, \ft_flags2), \newinstr3, \ft_flags3) .endm From f776e41fdcc4141876ef6f297318ab04c2382eb7 Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Fri, 7 Jun 2024 13:17:01 +0200 Subject: [PATCH 14/15] x86/alternative: Replace the old macros Now that the new macros have been gradually put in place, replace the old ones. Leave the new label numbers starting at 7xx as a hint that the new nested alternatives are being used now. Signed-off-by: Borislav Petkov (AMD) Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240607111701.8366-15-bp@kernel.org --- arch/x86/include/asm/alternative.h | 153 ++++++----------------------- 1 file changed, 32 insertions(+), 121 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 5278cfb1f745..89fa50d27a08 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -156,131 +156,51 @@ static inline int alternatives_text_reserved(void *start, void *end) #define ALT_CALL_INSTR "call BUG_func" -#define b_replacement(num) "664"#num -#define e_replacement(num) "665"#num +#define alt_slen "772b-771b" +#define alt_total_slen "773b-771b" +#define alt_rlen "775f-774f" -#define alt_end_marker "663" -#define alt_slen "662b-661b" -#define n_alt_slen "772b-771b" - -#define alt_total_slen alt_end_marker"b-661b" -#define n_alt_total_slen "773b-771b" - -#define alt_rlen(num) e_replacement(num)"f-"b_replacement(num)"f" -#define n_alt_rlen "775f-774f" - -#define OLDINSTR(oldinstr, num) \ - "# ALT: oldnstr\n" \ - "661:\n\t" oldinstr "\n662:\n" \ - "# ALT: padding\n" \ - ".skip -(((" alt_rlen(num) ")-(" alt_slen ")) > 0) * " \ - "((" alt_rlen(num) ")-(" alt_slen ")),0x90\n" \ - alt_end_marker ":\n" - -#define N_OLDINSTR(oldinstr) \ - "# N_ALT: oldinstr\n" \ +#define OLDINSTR(oldinstr) \ + "# ALT: oldinstr\n" \ "771:\n\t" oldinstr "\n772:\n" \ - "# N_ALT: padding\n" \ - ".skip -(((" n_alt_rlen ")-(" n_alt_slen ")) > 0) * " \ - "((" n_alt_rlen ")-(" n_alt_slen ")),0x90\n" \ + "# ALT: padding\n" \ + ".skip -(((" alt_rlen ")-(" alt_slen ")) > 0) * " \ + "((" alt_rlen ")-(" alt_slen ")),0x90\n" \ "773:\n" -/* - * gas compatible max based on the idea from: - * http://graphics.stanford.edu/~seander/bithacks.html#IntegerMinOrMax - * - * The additional "-" is needed because gas uses a "true" value of -1. - */ -#define alt_max_short(a, b) "((" a ") ^ (((" a ") ^ (" b ")) & -(-((" a ") < (" b ")))))" - -/* - * Pad the second replacement alternative with additional NOPs if it is - * additionally longer than the first replacement alternative. - */ -#define OLDINSTR_2(oldinstr, num1, num2) \ - "# ALT: oldinstr2\n" \ - "661:\n\t" oldinstr "\n662:\n" \ - "# ALT: padding2\n" \ - ".skip -((" alt_max_short(alt_rlen(num1), alt_rlen(num2)) " - (" alt_slen ")) > 0) * " \ - "(" alt_max_short(alt_rlen(num1), alt_rlen(num2)) " - (" alt_slen ")), 0x90\n" \ - alt_end_marker ":\n" - -#define OLDINSTR_3(oldinsn, n1, n2, n3) \ - "# ALT: oldinstr3\n" \ - "661:\n\t" oldinsn "\n662:\n" \ - "# ALT: padding3\n" \ - ".skip -((" alt_max_short(alt_max_short(alt_rlen(n1), alt_rlen(n2)), alt_rlen(n3)) \ - " - (" alt_slen ")) > 0) * " \ - "(" alt_max_short(alt_max_short(alt_rlen(n1), alt_rlen(n2)), alt_rlen(n3)) \ - " - (" alt_slen ")), 0x90\n" \ - alt_end_marker ":\n" - -#define ALTINSTR_ENTRY(ft_flags, num) \ - " .long 661b - .\n" /* label */ \ - " .long " b_replacement(num)"f - .\n" /* new instruction */ \ - " .4byte " __stringify(ft_flags) "\n" /* feature + flags */ \ - " .byte " alt_total_slen "\n" /* source len */ \ - " .byte " alt_rlen(num) "\n" /* replacement len */ - -#define N_ALTINSTR_ENTRY(ft_flags) \ +#define ALTINSTR_ENTRY(ft_flags) \ ".pushsection .altinstructions,\"a\"\n" \ " .long 771b - .\n" /* label */ \ " .long 774f - .\n" /* new instruction */ \ " .4byte " __stringify(ft_flags) "\n" /* feature + flags */ \ - " .byte " n_alt_total_slen "\n" /* source len */ \ - " .byte " n_alt_rlen "\n" /* replacement len */ \ + " .byte " alt_total_slen "\n" /* source len */ \ + " .byte " alt_rlen "\n" /* replacement len */ \ ".popsection\n" -#define ALTINSTR_REPLACEMENT(newinstr, num) /* replacement */ \ - "# ALT: replacement " #num "\n" \ - b_replacement(num)":\n\t" newinstr "\n" e_replacement(num) ":\n" - -#define N_ALTINSTR_REPLACEMENT(newinstr) /* replacement */ \ - ".pushsection .altinstr_replacement, \"ax\"\n" \ - "# N_ALT: replacement\n" \ - "774:\n\t" newinstr "\n775:\n" \ +#define ALTINSTR_REPLACEMENT(newinstr) /* replacement */ \ + ".pushsection .altinstr_replacement, \"ax\"\n" \ + "# ALT: replacement\n" \ + "774:\n\t" newinstr "\n775:\n" \ ".popsection\n" /* alternative assembly primitive: */ #define ALTERNATIVE(oldinstr, newinstr, ft_flags) \ - OLDINSTR(oldinstr, 1) \ - ".pushsection .altinstructions,\"a\"\n" \ - ALTINSTR_ENTRY(ft_flags, 1) \ - ".popsection\n" \ - ".pushsection .altinstr_replacement, \"ax\"\n" \ - ALTINSTR_REPLACEMENT(newinstr, 1) \ - ".popsection\n" - -/* Nested alternatives macro variant */ -#define N_ALTERNATIVE(oldinstr, newinstr, ft_flags) \ - N_OLDINSTR(oldinstr) \ - N_ALTINSTR_ENTRY(ft_flags) \ - N_ALTINSTR_REPLACEMENT(newinstr) + OLDINSTR(oldinstr) \ + ALTINSTR_ENTRY(ft_flags) \ + ALTINSTR_REPLACEMENT(newinstr) #define ALTERNATIVE_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) \ - OLDINSTR_2(oldinstr, 1, 2) \ - ".pushsection .altinstructions,\"a\"\n" \ - ALTINSTR_ENTRY(ft_flags1, 1) \ - ALTINSTR_ENTRY(ft_flags2, 2) \ - ".popsection\n" \ - ".pushsection .altinstr_replacement, \"ax\"\n" \ - ALTINSTR_REPLACEMENT(newinstr1, 1) \ - ALTINSTR_REPLACEMENT(newinstr2, 2) \ - ".popsection\n" - -#define N_ALTERNATIVE_2(oldinst, newinst1, flag1, newinst2, flag2) \ - N_ALTERNATIVE(N_ALTERNATIVE(oldinst, newinst1, flag1), \ - newinst2, flag2) + ALTERNATIVE(ALTERNATIVE(oldinstr, newinstr1, ft_flags1), newinstr2, ft_flags2) /* If @feature is set, patch in @newinstr_yes, otherwise @newinstr_no. */ #define ALTERNATIVE_TERNARY(oldinstr, ft_flags, newinstr_yes, newinstr_no) \ - N_ALTERNATIVE_2(oldinstr, newinstr_no, X86_FEATURE_ALWAYS, \ - newinstr_yes, ft_flags) + ALTERNATIVE_2(oldinstr, newinstr_no, X86_FEATURE_ALWAYS, newinstr_yes, ft_flags) #define ALTERNATIVE_3(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2, \ newinstr3, ft_flags3) \ - N_ALTERNATIVE(N_ALTERNATIVE_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2), \ + ALTERNATIVE(ALTERNATIVE_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2), \ newinstr3, ft_flags3) + /* * Alternative instructions for different CPU types or capabilities. * @@ -294,10 +214,10 @@ static inline int alternatives_text_reserved(void *start, void *end) * without volatile and memory clobber. */ #define alternative(oldinstr, newinstr, ft_flags) \ - asm_inline volatile(N_ALTERNATIVE(oldinstr, newinstr, ft_flags) : : : "memory") + asm_inline volatile(ALTERNATIVE(oldinstr, newinstr, ft_flags) : : : "memory") #define alternative_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) \ - asm_inline volatile(N_ALTERNATIVE_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) ::: "memory") + asm_inline volatile(ALTERNATIVE_2(oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2) ::: "memory") /* * Alternative inline assembly with input. @@ -308,12 +228,12 @@ static inline int alternatives_text_reserved(void *start, void *end) * Leaving an unused argument 0 to keep API compatibility. */ #define alternative_input(oldinstr, newinstr, ft_flags, input...) \ - asm_inline volatile(N_ALTERNATIVE(oldinstr, newinstr, ft_flags) \ + asm_inline volatile(ALTERNATIVE(oldinstr, newinstr, ft_flags) \ : : "i" (0), ## input) /* Like alternative_input, but with a single output argument */ #define alternative_io(oldinstr, newinstr, ft_flags, output, input...) \ - asm_inline volatile(N_ALTERNATIVE(oldinstr, newinstr, ft_flags) \ + asm_inline volatile(ALTERNATIVE(oldinstr, newinstr, ft_flags) \ : output : "i" (0), ## input) /* @@ -327,7 +247,7 @@ static inline int alternatives_text_reserved(void *start, void *end) * suffix. */ #define alternative_call(oldfunc, newfunc, ft_flags, output, input...) \ - asm_inline volatile(N_ALTERNATIVE("call %c[old]", "call %c[new]", ft_flags) \ + asm_inline volatile(ALTERNATIVE("call %c[old]", "call %c[new]", ft_flags) \ : output : [old] "i" (oldfunc), [new] "i" (newfunc), ## input) /* @@ -338,7 +258,7 @@ static inline int alternatives_text_reserved(void *start, void *end) */ #define alternative_call_2(oldfunc, newfunc1, ft_flags1, newfunc2, ft_flags2, \ output, input...) \ - asm_inline volatile(N_ALTERNATIVE_2("call %c[old]", "call %c[new1]", ft_flags1, \ + asm_inline volatile(ALTERNATIVE_2("call %c[old]", "call %c[new1]", ft_flags1, \ "call %c[new2]", ft_flags2) \ : output, ASM_CALL_CONSTRAINT \ : [old] "i" (oldfunc), [new1] "i" (newfunc1), \ @@ -422,7 +342,7 @@ void nop_func(void); * @newinstr. ".skip" directive takes care of proper instruction padding * in case @newinstr is longer than @oldinstr. */ -#define __N_ALTERNATIVE(oldinst, newinst, flag) \ +#define __ALTERNATIVE(oldinst, newinst, flag) \ 740: \ oldinst ; \ 741: \ @@ -438,7 +358,7 @@ void nop_func(void); .popsection ; .macro ALTERNATIVE oldinstr, newinstr, ft_flags - __N_ALTERNATIVE(\oldinstr, \newinstr, \ft_flags) + __ALTERNATIVE(\oldinstr, \newinstr, \ft_flags) .endm #define old_len 141b-140b @@ -446,27 +366,18 @@ void nop_func(void); #define new_len2 145f-144f #define new_len3 146f-145f -/* - * gas compatible max based on the idea from: - * http://graphics.stanford.edu/~seander/bithacks.html#IntegerMinOrMax - * - * The additional "-" is needed because gas uses a "true" value of -1. - */ -#define alt_max_2(a, b) ((a) ^ (((a) ^ (b)) & -(-((a) < (b))))) -#define alt_max_3(a, b, c) (alt_max_2(alt_max_2(a, b), c)) - /* * Same as ALTERNATIVE macro above but for two alternatives. If CPU * has @feature1, it replaces @oldinstr with @newinstr1. If CPU has * @feature2, it replaces @oldinstr with @feature2. */ .macro ALTERNATIVE_2 oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2 - __N_ALTERNATIVE(__N_ALTERNATIVE(\oldinstr, \newinstr1, \ft_flags1), + __ALTERNATIVE(__ALTERNATIVE(\oldinstr, \newinstr1, \ft_flags1), \newinstr2, \ft_flags2) .endm .macro ALTERNATIVE_3 oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2, newinstr3, ft_flags3 - __N_ALTERNATIVE(N_ALTERNATIVE_2(\oldinstr, \newinstr1, \ft_flags1, \newinstr2, \ft_flags2), + __ALTERNATIVE(ALTERNATIVE_2(\oldinstr, \newinstr1, \ft_flags1, \newinstr2, \ft_flags2), \newinstr3, \ft_flags3) .endm From 0d3db1f14abb4eb28613fbeb1e2ad92bac76debf Mon Sep 17 00:00:00 2001 From: "Borislav Petkov (AMD)" Date: Tue, 18 Jun 2024 21:57:27 +0200 Subject: [PATCH 15/15] x86/alternatives, kvm: Fix a couple of CALLs without a frame pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit objtool complains: arch/x86/kvm/kvm.o: warning: objtool: .altinstr_replacement+0xc5: call without frame pointer save/setup vmlinux.o: warning: objtool: .altinstr_replacement+0x2eb: call without frame pointer save/setup Make sure %rSP is an output operand to the respective asm() statements. The test_cc() hunk and ALT_OUTPUT_SP() courtesy of peterz. Also from him add some helpful debugging info to the documentation. Now on to the explanations: tl;dr: The alternatives macros are pretty fragile. If I do ALT_OUTPUT_SP(output) in order to be able to package in a %rsp reference for objtool so that a stack frame gets properly generated, the inline asm input operand with positional argument 0 in clear_page(): "0" (page) gets "renumbered" due to the added : "+r" (current_stack_pointer), "=D" (page) and then gcc says: ./arch/x86/include/asm/page_64.h:53:9: error: inconsistent operand constraints in an ‘asm’ The fix is to use an explicit "D" constraint which points to a singleton register class (gcc terminology) which ends up doing what is expected here: the page pointer - input and output - should be in the same %rdi register. Other register classes have more than one register in them - example: "r" and "=r" or "A": ‘A’ The ‘a’ and ‘d’ registers. This class is used for instructions that return double word results in the ‘ax:dx’ register pair. Single word values will be allocated either in ‘ax’ or ‘dx’. so using "D" and "=D" just works in this particular case. And yes, one would say, sure, why don't you do "+D" but then: : "+r" (current_stack_pointer), "+D" (page) : [old] "i" (clear_page_orig), [new1] "i" (clear_page_rep), [new2] "i" (clear_page_erms), : "cc", "memory", "rax", "rcx") now find the Waldo^Wcomma which throws a wrench into all this. Because that silly macro has an "input..." consume-all last macro arg and in it, one is supposed to supply input *and* clobbers, leading to silly syntax snafus. Yap, they need to be cleaned up, one fine day... Closes: https://lore.kernel.org/oe-kbuild-all/202406141648.jO9qNGLa-lkp@intel.com/ Reported-by: kernel test robot Signed-off-by: Borislav Petkov (AMD) Acked-by: Sean Christopherson Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/r/20240625112056.GDZnqoGDXgYuWBDUwu@fat_crate.local --- arch/x86/include/asm/alternative.h | 11 +++++++---- arch/x86/include/asm/page_64.h | 2 +- arch/x86/kernel/alternative.c | 2 +- arch/x86/kvm/emulate.c | 2 +- tools/objtool/Documentation/objtool.txt | 19 +++++++++++++++++++ 5 files changed, 29 insertions(+), 7 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 89fa50d27a08..ca9ae606aab9 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -246,9 +246,10 @@ static inline int alternatives_text_reserved(void *start, void *end) * references: i.e., if used for a function, it would add the PLT * suffix. */ -#define alternative_call(oldfunc, newfunc, ft_flags, output, input...) \ - asm_inline volatile(ALTERNATIVE("call %c[old]", "call %c[new]", ft_flags) \ - : output : [old] "i" (oldfunc), [new] "i" (newfunc), ## input) +#define alternative_call(oldfunc, newfunc, ft_flags, output, input...) \ + asm_inline volatile(ALTERNATIVE("call %c[old]", "call %c[new]", ft_flags) \ + : ALT_OUTPUT_SP(output) \ + : [old] "i" (oldfunc), [new] "i" (newfunc), ## input) /* * Like alternative_call, but there are two features and respective functions. @@ -260,7 +261,7 @@ static inline int alternatives_text_reserved(void *start, void *end) output, input...) \ asm_inline volatile(ALTERNATIVE_2("call %c[old]", "call %c[new1]", ft_flags1, \ "call %c[new2]", ft_flags2) \ - : output, ASM_CALL_CONSTRAINT \ + : ALT_OUTPUT_SP(output) \ : [old] "i" (oldfunc), [new1] "i" (newfunc1), \ [new2] "i" (newfunc2), ## input) @@ -276,6 +277,8 @@ static inline int alternatives_text_reserved(void *start, void *end) */ #define ASM_NO_INPUT_CLOBBER(clbr...) "i" (0) : clbr +#define ALT_OUTPUT_SP(...) ASM_CALL_CONSTRAINT, ## __VA_ARGS__ + /* Macro for creating assembler functions avoiding any C magic. */ #define DEFINE_ASM_FUNC(func, instr, sec) \ asm (".pushsection " #sec ", \"ax\"\n" \ diff --git a/arch/x86/include/asm/page_64.h b/arch/x86/include/asm/page_64.h index cc6b8e087192..af4302d79b59 100644 --- a/arch/x86/include/asm/page_64.h +++ b/arch/x86/include/asm/page_64.h @@ -54,7 +54,7 @@ static inline void clear_page(void *page) clear_page_rep, X86_FEATURE_REP_GOOD, clear_page_erms, X86_FEATURE_ERMS, "=D" (page), - "0" (page) + "D" (page) : "cc", "memory", "rax", "rcx"); } diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index 37596a417094..333b16181357 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -1657,7 +1657,7 @@ static noinline void __init alt_reloc_selftest(void) */ asm_inline volatile ( ALTERNATIVE("", "lea %[mem], %%" _ASM_ARG1 "; call __alt_reloc_selftest;", X86_FEATURE_ALWAYS) - : /* output */ + : ASM_CALL_CONSTRAINT : [mem] "m" (__alt_reloc_selftest_addr) : _ASM_ARG1 ); diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 5d4c86133453..c8cc578646d0 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -1069,7 +1069,7 @@ static __always_inline u8 test_cc(unsigned int condition, unsigned long flags) flags = (flags & EFLAGS_MASK) | X86_EFLAGS_IF; asm("push %[flags]; popf; " CALL_NOSPEC - : "=a"(rc) : [thunk_target]"r"(fop), [flags]"r"(flags)); + : "=a"(rc), ASM_CALL_CONSTRAINT : [thunk_target]"r"(fop), [flags]"r"(flags)); return rc; } diff --git a/tools/objtool/Documentation/objtool.txt b/tools/objtool/Documentation/objtool.txt index fe39c2a8ef0d..7c3ee959b63c 100644 --- a/tools/objtool/Documentation/objtool.txt +++ b/tools/objtool/Documentation/objtool.txt @@ -284,6 +284,25 @@ the objtool maintainers. Otherwise the stack frame may not get created before the call. + objtool can help with pinpointing the exact function where it happens: + + $ OBJTOOL_ARGS="--verbose" make arch/x86/kvm/ + + arch/x86/kvm/kvm.o: warning: objtool: .altinstr_replacement+0xc5: call without frame pointer save/setup + arch/x86/kvm/kvm.o: warning: objtool: em_loop.part.0+0x29: (alt) + arch/x86/kvm/kvm.o: warning: objtool: em_loop.part.0+0x0: <=== (sym) + LD [M] arch/x86/kvm/kvm-intel.o + 0000 0000000000028220 : + 0000 28220: 0f b6 47 61 movzbl 0x61(%rdi),%eax + 0004 28224: 3c e2 cmp $0xe2,%al + 0006 28226: 74 2c je 28254 + 0008 28228: 48 8b 57 10 mov 0x10(%rdi),%rdx + 000c 2822c: 83 f0 05 xor $0x5,%eax + 000f 2822f: 48 c1 e0 04 shl $0x4,%rax + 0013 28233: 25 f0 00 00 00 and $0xf0,%eax + 0018 28238: 81 e2 d5 08 00 00 and $0x8d5,%edx + 001e 2823e: 80 ce 02 or $0x2,%dh + ... 2. file.o: warning: objtool: .text+0x53: unreachable instruction