| Message ID | 20240422181434.3463252-4-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/alternatives: Adjust all insn-relative fields | expand |
This of course intended to say indent... ~Andrew
On 22.04.2024 20:14, Andrew Cooper wrote: > ... to make subsequent patches legible. > > No functional change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> even if (perhaps due to changes in the "real" patch) some of this re- indenting wants doing differently, just with ... > --- a/xen/arch/x86/alternative.c > +++ b/xen/arch/x86/alternative.c > @@ -244,78 +244,80 @@ static void init_or_livepatch _apply_alternatives(struct alt_instr *start, > > memcpy(buf, repl, a->repl_len); > > - /* 0xe8/0xe9 are relative branches; fix the offset. */ > - if ( a->repl_len >= 5 && (*buf & 0xfe) == 0xe8 ) > { > - /* > - * Detect the special case of indirect-to-direct branch patching: > - * - replacement is a direct CALL/JMP (opcodes 0xE8/0xE9; already > - * checked above), > - * - replacement's displacement is -5 (pointing back at the very > - * insn, which makes no sense in a real replacement insn), > - * - original is an indirect CALL/JMP (opcodes 0xFF/2 or 0xFF/4) > - * using RIP-relative addressing. > - * Some branch destinations may still be NULL when we come here > - * the first time. Defer patching of those until the post-presmp- > - * initcalls re-invocation (with force set to true). If at that > - * point the branch destination is still NULL, insert "UD2; UD0" > - * (for ease of recognition) instead of CALL/JMP. > - */ > - if ( a->cpuid == X86_FEATURE_ALWAYS && > - *(int32_t *)(buf + 1) == -5 && > - a->orig_len >= 6 && > - orig[0] == 0xff && > - orig[1] == (*buf & 1 ? 0x25 : 0x15) ) > + /* 0xe8/0xe9 are relative branches; fix the offset. */ > + if ( a->repl_len >= 5 && (*buf & 0xfe) == 0xe8 ) > { > - long disp = *(int32_t *)(orig + 2); > - const uint8_t *dest = *(void **)(orig + 6 + disp); > - > - if ( dest ) > + /* > + * Detect the special case of indirect-to-direct branch patching: > + * - replacement is a direct CALL/JMP (opcodes 0xE8/0xE9; already > + * checked above), > + * - replacement's displacement is -5 (pointing back at the very > + * insn, which makes no sense in a real replacement insn), > + * - original is an indirect CALL/JMP (opcodes 0xFF/2 or 0xFF/4) > + * using RIP-relative addressing. > + * Some branch destinations may still be NULL when we come here > + * the first time. Defer patching of those until the post-presmp- > + * initcalls re-invocation (with force set to true). If at that > + * point the branch destination is still NULL, insert "UD2; UD0" > + * (for ease of recognition) instead of CALL/JMP. > + */ > + if ( a->cpuid == X86_FEATURE_ALWAYS && > + *(int32_t *)(buf + 1) == -5 && > + a->orig_len >= 6 && > + orig[0] == 0xff && > + orig[1] == (*buf & 1 ? 0x25 : 0x15) ) > { > - /* > - * When building for CET-IBT, all function pointer targets > - * should have an endbr64 instruction. > - * > - * If this is not the case, leave a warning because > - * something is probably wrong with the build. A CET-IBT > - * enabled system might have exploded already. > - * > - * Otherwise, skip the endbr64 instruction. This is a > - * marginal perf improvement which saves on instruction > - * decode bandwidth. > - */ > - if ( IS_ENABLED(CONFIG_XEN_IBT) ) > + long disp = *(int32_t *)(orig + 2); > + const uint8_t *dest = *(void **)(orig + 6 + disp); > + > + if ( dest ) > { > - if ( is_endbr64(dest) ) > - dest += ENDBR64_LEN; > - else > - printk(XENLOG_WARNING > - "altcall %ps dest %ps has no endbr64\n", > - orig, dest); > + /* > + * When building for CET-IBT, all function pointer targets > + * should have an endbr64 instruction. > + * > + * If this is not the case, leave a warning because > + * something is probably wrong with the build. A CET-IBT > + * enabled system might have exploded already. > + * > + * Otherwise, skip the endbr64 instruction. This is a > + * marginal perf improvement which saves on instruction > + * decode bandwidth. > + */ > + if ( IS_ENABLED(CONFIG_XEN_IBT) ) > + { > + if ( is_endbr64(dest) ) > + dest += ENDBR64_LEN; > + else > + printk(XENLOG_WARNING > + "altcall %ps dest %ps has no endbr64\n", > + orig, dest); > + } > + > + disp = dest - (orig + 5); > + ASSERT(disp == (int32_t)disp); > + *(int32_t *)(buf + 1) = disp; > } > - > - disp = dest - (orig + 5); > - ASSERT(disp == (int32_t)disp); > - *(int32_t *)(buf + 1) = disp; > - } > - else if ( force ) > - { > - buf[0] = 0x0f; > - buf[1] = 0x0b; > - buf[2] = 0x0f; > - buf[3] = 0xff; > - buf[4] = 0xff; > + else if ( force ) > + { > + buf[0] = 0x0f; > + buf[1] = 0x0b; > + buf[2] = 0x0f; > + buf[3] = 0xff; > + buf[4] = 0xff; > + } > + else > + continue; > } > + else if ( force && system_state < SYS_STATE_active ) > + ASSERT_UNREACHABLE(); > else > - continue; > + *(int32_t *)(buf + 1) += repl - orig; > } > - else if ( force && system_state < SYS_STATE_active ) > + else if ( force && system_state < SYS_STATE_active ) ... this undone. Jan
diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c index 5bd256365def..2ca4dfd569bc 100644 --- a/xen/arch/x86/alternative.c +++ b/xen/arch/x86/alternative.c @@ -244,78 +244,80 @@ static void init_or_livepatch _apply_alternatives(struct alt_instr *start, memcpy(buf, repl, a->repl_len); - /* 0xe8/0xe9 are relative branches; fix the offset. */ - if ( a->repl_len >= 5 && (*buf & 0xfe) == 0xe8 ) { - /* - * Detect the special case of indirect-to-direct branch patching: - * - replacement is a direct CALL/JMP (opcodes 0xE8/0xE9; already - * checked above), - * - replacement's displacement is -5 (pointing back at the very - * insn, which makes no sense in a real replacement insn), - * - original is an indirect CALL/JMP (opcodes 0xFF/2 or 0xFF/4) - * using RIP-relative addressing. - * Some branch destinations may still be NULL when we come here - * the first time. Defer patching of those until the post-presmp- - * initcalls re-invocation (with force set to true). If at that - * point the branch destination is still NULL, insert "UD2; UD0" - * (for ease of recognition) instead of CALL/JMP. - */ - if ( a->cpuid == X86_FEATURE_ALWAYS && - *(int32_t *)(buf + 1) == -5 && - a->orig_len >= 6 && - orig[0] == 0xff && - orig[1] == (*buf & 1 ? 0x25 : 0x15) ) + /* 0xe8/0xe9 are relative branches; fix the offset. */ + if ( a->repl_len >= 5 && (*buf & 0xfe) == 0xe8 ) { - long disp = *(int32_t *)(orig + 2); - const uint8_t *dest = *(void **)(orig + 6 + disp); - - if ( dest ) + /* + * Detect the special case of indirect-to-direct branch patching: + * - replacement is a direct CALL/JMP (opcodes 0xE8/0xE9; already + * checked above), + * - replacement's displacement is -5 (pointing back at the very + * insn, which makes no sense in a real replacement insn), + * - original is an indirect CALL/JMP (opcodes 0xFF/2 or 0xFF/4) + * using RIP-relative addressing. + * Some branch destinations may still be NULL when we come here + * the first time. Defer patching of those until the post-presmp- + * initcalls re-invocation (with force set to true). If at that + * point the branch destination is still NULL, insert "UD2; UD0" + * (for ease of recognition) instead of CALL/JMP. + */ + if ( a->cpuid == X86_FEATURE_ALWAYS && + *(int32_t *)(buf + 1) == -5 && + a->orig_len >= 6 && + orig[0] == 0xff && + orig[1] == (*buf & 1 ? 0x25 : 0x15) ) { - /* - * When building for CET-IBT, all function pointer targets - * should have an endbr64 instruction. - * - * If this is not the case, leave a warning because - * something is probably wrong with the build. A CET-IBT - * enabled system might have exploded already. - * - * Otherwise, skip the endbr64 instruction. This is a - * marginal perf improvement which saves on instruction - * decode bandwidth. - */ - if ( IS_ENABLED(CONFIG_XEN_IBT) ) + long disp = *(int32_t *)(orig + 2); + const uint8_t *dest = *(void **)(orig + 6 + disp); + + if ( dest ) { - if ( is_endbr64(dest) ) - dest += ENDBR64_LEN; - else - printk(XENLOG_WARNING - "altcall %ps dest %ps has no endbr64\n", - orig, dest); + /* + * When building for CET-IBT, all function pointer targets + * should have an endbr64 instruction. + * + * If this is not the case, leave a warning because + * something is probably wrong with the build. A CET-IBT + * enabled system might have exploded already. + * + * Otherwise, skip the endbr64 instruction. This is a + * marginal perf improvement which saves on instruction + * decode bandwidth. + */ + if ( IS_ENABLED(CONFIG_XEN_IBT) ) + { + if ( is_endbr64(dest) ) + dest += ENDBR64_LEN; + else + printk(XENLOG_WARNING + "altcall %ps dest %ps has no endbr64\n", + orig, dest); + } + + disp = dest - (orig + 5); + ASSERT(disp == (int32_t)disp); + *(int32_t *)(buf + 1) = disp; } - - disp = dest - (orig + 5); - ASSERT(disp == (int32_t)disp); - *(int32_t *)(buf + 1) = disp; - } - else if ( force ) - { - buf[0] = 0x0f; - buf[1] = 0x0b; - buf[2] = 0x0f; - buf[3] = 0xff; - buf[4] = 0xff; + else if ( force ) + { + buf[0] = 0x0f; + buf[1] = 0x0b; + buf[2] = 0x0f; + buf[3] = 0xff; + buf[4] = 0xff; + } + else + continue; } + else if ( force && system_state < SYS_STATE_active ) + ASSERT_UNREACHABLE(); else - continue; + *(int32_t *)(buf + 1) += repl - orig; } - else if ( force && system_state < SYS_STATE_active ) + else if ( force && system_state < SYS_STATE_active ) ASSERT_UNREACHABLE(); - else - *(int32_t *)(buf + 1) += repl - orig; } - else if ( force && system_state < SYS_STATE_active ) - ASSERT_UNREACHABLE(); a->priv = 1;
... to make subsequent patches legible. No functional change. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/alternative.c | 126 +++++++++++++++++++------------------ 1 file changed, 64 insertions(+), 62 deletions(-)