[PATCH v3 4/4] x86/alternative: Complicate optimize_nops() some more

From: Peter Zijlstra <peterz@infradead.org>
To: x86@kernel.org
Cc: linux-kernel@vger.kernel.org, peterz@infradead.org,
	mhiramat@kernel.org, kirill.shutemov@linux.intel.com,
	Andrew.Cooper3@citrix.com, jpoimboe@redhat.com
Subject: [PATCH v3 4/4] x86/alternative: Complicate optimize_nops() some more
Date: Wed, 08 Feb 2023 18:10:54 +0100	[thread overview]
Message-ID: <20230208171431.433132442@infradead.org> (raw)
In-Reply-To: 20230208171050.490809180@infradead.org

Because:

  SMP alternatives: ffffffff810026dc: [2:44) optimized NOPs: eb 2a eb 28 cc cc
    cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
    cc cc cc cc cc cc cc cc cc cc cc cc cc

is quite daft, make things more complicated and have the NOP runlength
detection eat the preceding JMP if they both end at the same target.

  SMP alternatives: ffffffff810026dc: [0:44) optimized NOPs: eb 2a cc cc cc cc
    cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc
    cc cc cc cc cc cc cc cc cc cc cc cc cc

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kernel/alternative.c |   59 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 49 insertions(+), 10 deletions(-)

--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -126,6 +126,7 @@ static void __init_or_module add_nops(vo
 	}
 }
 
+/* Fill the buffer with a single effective instruction of size @len */
 static void __init_or_module add_nop(u8 *instr, unsigned int len)
 {
 	u8 *target = instr + len;
@@ -158,6 +159,9 @@ extern struct alt_instr __alt_instructio
 extern s32 __smp_locks[], __smp_locks_end[];
 void text_poke_early(void *addr, const void *opcode, size_t len);
 
+/*
+ * Matches NOP and NOPL, not any of the other possible NOPs.
+ */
 static bool insn_is_nop(struct insn *insn)
 {
 	if (insn->opcode.bytes[0] == 0x90)
@@ -171,6 +175,10 @@ static bool insn_is_nop(struct insn *ins
 	return false;
 }
 
+/*
+ * Find the offset of the first non-nop instruction starting at @offset
+ * but no further than @len.
+ */
 static int skip_nops(u8 *instr, int offset, int len)
 {
 	struct insn insn;
@@ -187,11 +195,46 @@ static int skip_nops(u8 *instr, int offs
 }
 
 /*
+ * Optimize a sequence of NOPs, possibly preceded by a unconditional jump
+ * to the end of the NOP sequence into a single 'NOP'.
+ */
+static bool __optimize_nops(u8 *instr, size_t len, struct insn *insn,
+			    int *next, int *prev, int *target)
+{
+	int i = *next - insn->length;
+
+	switch (insn->opcode.bytes[0]) {
+	case JMP8_INSN_OPCODE:
+	case JMP32_INSN_OPCODE:
+		*prev = i;
+		*target = *next + insn->immediate.value;
+		return false;
+	}
+
+	if (insn_is_nop(insn)) {
+		int nop = i;
+
+		*next = skip_nops(instr, *next, len);
+		if (*target && *next == *target)
+			nop = *prev;
+
+		add_nop(instr + nop, *next - nop);
+		DUMP_BYTES(ALT, instr, len, "%px: [%d:%d) optimized NOPs: ", instr, nop, *next);
+		return true;
+	}
+
+	*target = 0;
+	return false;
+}
+
+/*
  * "noinline" to cause control flow change and thus invalidate I$ and
  * cause refetch after modification.
  */
 static void __init_or_module noinline optimize_nops(u8 *instr, size_t len)
 {
+	int prev, target = 0;
+
 	for (int next, i = 0; i < len; i = next) {
 		struct insn insn;
 
@@ -200,11 +243,7 @@ static void __init_or_module noinline op
 
 		next = i + insn.length;
 
-		if (insn_is_nop(&insn)) {
-			next = skip_nops(instr, next, len);
-			add_nop(instr + i, next - i);
-			DUMP_BYTES(ALT, instr, len, "%px: [%d:%d) optimized NOPs: ", instr, i, next);
-		}
+		__optimize_nops(instr, len, &insn, &next, &prev, &target);
 	}
 }
 
@@ -275,6 +314,8 @@ bool need_reloc(unsigned long offset, u8
 static void __init_or_module noinline
 apply_relocation(u8 *buf, size_t len, u8 *dest, u8 *src, size_t src_len)
 {
+	int prev, target = 0;
+
 	for (int next, i = 0; i < len; i = next) {
 		struct insn insn;
 
@@ -283,6 +324,9 @@ apply_relocation(u8 *buf, size_t len, u8
 
 		next = i + insn.length;
 
+		if (__optimize_nops(buf, len, &insn, &next, &prev, &target))
+			continue;
+
 		switch (insn.opcode.bytes[0]) {
 		case 0x0f:
 			if (insn.opcode.bytes[1] < 0x80 ||
@@ -324,11 +368,6 @@ apply_relocation(u8 *buf, size_t len, u8
 					    src - dest);
 			}
 		}
-
-		if (insn_is_nop(&insn)) {
-			next = skip_nops(buf, next, len);
-			add_nop(buf + i, next - i);
-		}
 	}
 }
 


