mbox series

[v5,0/5] 86/paravirt: Get rid of paravirt patching

Message ID 20231129133332.31043-1-jgross@suse.com
Headers
Series 86/paravirt: Get rid of paravirt patching |

Message

Juergen Gross Nov. 29, 2023, 1:33 p.m. UTC 
  This is a small series getting rid of paravirt patching by switching
completely to alternative patching for the same functionality.

The basic idea is to add the capability to switch from indirect to
direct calls via a special alternative patching option.

This removes _some_ of the paravirt macro maze, but most of it needs
to stay due to the need of hiding the call instructions from the
compiler in order to avoid needless register save/restore.

What is going away is the nasty stacking of alternative and paravirt
patching and (of course) the special .parainstructions linker section.

I have tested the series on bare metal and as Xen PV domain to still
work.

Note that objtool might need some changes to cope with the new
indirect call patching mechanism. Additionally some paravirt handling
can probably be removed from it.

Changes in V5:
- addressed Boris' comments
- rebased on top of the tip/master branch

Changes in V4:
- addressed Boris' comments in patch 1
- fixed bugs found by kernel test robot (patch 2)

Changes in V3:
- split v2 patch 3 into 2 patches as requested by Peter and Ingo

Changes in V2:
- split last patch into 2
- rebase of patch 2 as suggested by Peter
- addressed Peter's comments for patch 3

Juergen Gross (5):
  x86/paravirt: introduce ALT_NOT_XEN
  x86/paravirt: move some functions and defines to alternative
  x86/alternative: add indirect call patching
  x86/paravirt: switch mixed paravirt/alternative calls to alternative_2
  x86/paravirt: remove no longer needed paravirt patching code

 arch/x86/include/asm/alternative.h        |  30 +++++-
 arch/x86/include/asm/paravirt.h           |  77 ++++---------
 arch/x86/include/asm/paravirt_types.h     |  85 +++++----------
 arch/x86/include/asm/qspinlock_paravirt.h |   4 +-
 arch/x86/include/asm/text-patching.h      |  12 ---
 arch/x86/kernel/alternative.c             | 125 ++++++++++------------
 arch/x86/kernel/callthunks.c              |  17 ++-
 arch/x86/kernel/kvm.c                     |   4 +-
 arch/x86/kernel/module.c                  |  20 +---
 arch/x86/kernel/paravirt.c                |  54 ++--------
 arch/x86/kernel/vmlinux.lds.S             |  13 ---
 arch/x86/tools/relocs.c                   |   2 +-
 arch/x86/xen/irq.c                        |   2 +-
 13 files changed, 161 insertions(+), 284 deletions(-)

Comments

Borislav Petkov Dec. 6, 2023, 11:06 a.m. UTC | #1 
On Wed, Nov 29, 2023 at 02:33:30PM +0100, Juergen Gross wrote:
> In order to prepare replacing of paravirt patching with alternative
> patching, add the capability to replace an indirect call with a direct
> one to alternative patching.
> 
> This is done via a new flag ALT_FLAG_CALL as the target of the call
> instruction needs to be evaluated using the value of the location
> addressed by the indirect call. For convenience add a macro for a
> default call instruction. In case it is being used without the new
> flag being set, it will result in a BUG() when being executed. As in
> most cases the feature used will be X86_FEATURE_ALWAYS add another
> macro ALT_CALL_ALWAYS usable for the flags parameter of the ALTERNATIVE
> macros.
> 
> For a complete replacement handle the special cases of calling a nop
> function and an indirect call of NULL the same way as paravirt does.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

That SOB chain basically says that you're PeterZ and you're sending this
patch. :)

Are you trying to say that he co-developed it or suggested it or
Originally-by?

Documentation/process/submitting-patches.rst has it all.

Also, what I did ontop of this is below as we must dump the full flags
now with the debug output since you're adding more flags than ALT_NOT.

Also, the naked magic numbers need explanation.

Please include into your next submission.

Thx.

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index e7e53b9e801b..888205234f15 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -395,6 +395,9 @@ noinstr void BUG_func(void)
 }
 EXPORT_SYMBOL_GPL(BUG_func);
 
+#define CALL_RIP_REL_PREFIX	0xff
+#define CALL_RIP_REL_MODRM	0x15
+
 /*
  * Rewrite the "call BUG_func" replacement to point to the target of the
  * indirect pv_ops call "call *disp(%ip)".
@@ -409,11 +412,14 @@ static int alt_replace_call(u8 *instr, u8 *insn_buff, struct alt_instr *a)
 		BUG();
 	}
 
-	if (a->instrlen != 6 || instr[0] != 0xff || instr[1] != 0x15) {
+	if (a->instrlen != 6 ||
+	    instr[0] != CALL_RIP_REL_PREFIX ||
+	    instr[1] != CALL_RIP_REL_MODRM) {
 		pr_err("ALT_FLAG_DIRECT_CALL set for unrecognized indirect call\n");
 		BUG();
 	}
 
+	/* Skip CALL_RIP_REL_PREFIX and CALL_RIP_REL_MODRM */
 	disp = *(s32 *)(instr + 2);
 #ifdef CONFIG_X86_64
 	/* ff 15 00 00 00 00   call   *0x0(%rip) */
@@ -493,12 +499,11 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start,
 			continue;
 		}
 
-		DPRINTK(ALT, "feat: %s%d*32+%d, old: (%pS (%px) len: %d), repl: (%px, len: %d)",
-			(a->flags & ALT_FLAG_NOT) ? "!" : "",
+		DPRINTK(ALT, "feat: %d32+%d, old: (%pS (%px) len: %d), repl: (%px, len: %d) flags: 0x%x",
 			a->cpuid >> 5,
 			a->cpuid & 0x1f,
 			instr, instr, a->instrlen,
-			replacement, a->replacementlen);
+			replacement, a->replacementlen, a->flags);
 
 		memcpy(insn_buff, replacement, a->replacementlen);
 		insn_buff_sz = a->replacementlen;