[RFC PATCH 0/3] x86/paravirt: Get rid of paravirt patching

[RFC PATCH 0/3] x86/paravirt: Get rid of paravirt patching

[Juergen Gross]

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.

RFC because I'm quite sure there will be some objtool work needed
(at least removing the specific paravirt handling).

Juergen Gross (3):
  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

 arch/x86/include/asm/alternative.h        | 26 +++++-
 arch/x86/include/asm/paravirt.h           | 39 ++-------
 arch/x86/include/asm/paravirt_types.h     | 68 +++-------------
 arch/x86/include/asm/qspinlock_paravirt.h |  4 +-
 arch/x86/include/asm/text-patching.h      | 12 ---
 arch/x86/kernel/alternative.c             | 99 +++++++++++------------
 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, 111 insertions(+), 249 deletions(-)

-- 
2.35.3

[RFC PATCH 1/3] x86/paravirt: move some functions and defines to alternative

[Juergen Gross]

As a preparation for replacing paravirt patching completely by
alternative patching, move some backend functions and #defines to
alternative code and header.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/include/asm/alternative.h        | 16 ++++++++++++
 arch/x86/include/asm/paravirt.h           | 12 ---------
 arch/x86/include/asm/paravirt_types.h     |  4 +--
 arch/x86/include/asm/qspinlock_paravirt.h |  4 +--
 arch/x86/kernel/alternative.c             | 10 ++++++++
 arch/x86/kernel/kvm.c                     |  4 +--
 arch/x86/kernel/paravirt.c                | 30 +++++++----------------
 arch/x86/xen/irq.c                        |  2 +-
 8 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index d7da28fada87..a5a4944ce5d1 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -335,6 +335,22 @@ static inline int alternatives_text_reserved(void *start, void *end)
  */
 #define ASM_NO_INPUT_CLOBBER(clbr...) "i" (0) : clbr
 
+/* Macro for creating assembler functions avoiding any C magic. */
+#define DEFINE_ASM_FUNC(func, instr, sec)		\
+	asm (".pushsection " #sec ", \"ax\"\n"		\
+	     ".global " #func "\n\t"			\
+	     ".type " #func ", @function\n\t"		\
+	     ASM_FUNC_ALIGN "\n"			\
+	     #func ":\n\t"				\
+	     ASM_ENDBR					\
+	     instr "\n\t"				\
+	     ASM_RET					\
+	     ".size " #func ", . - " #func "\n\t"	\
+	     ".popsection")
+
+void x86_BUG(void);
+void x86_nop(void);
+
 #else /* __ASSEMBLY__ */
 
 #ifdef CONFIG_SMP
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index b49778664d2b..3474dac4607d 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -726,18 +726,6 @@ static __always_inline unsigned long arch_local_irq_save(void)
 #undef PVOP_VCALL4
 #undef PVOP_CALL4
 
-#define DEFINE_PARAVIRT_ASM(func, instr, sec)		\
-	asm (".pushsection " #sec ", \"ax\"\n"		\
-	     ".global " #func "\n\t"			\
-	     ".type " #func ", @function\n\t"		\
-	     ASM_FUNC_ALIGN "\n"			\
-	     #func ":\n\t"				\
-	     ASM_ENDBR					\
-	     instr "\n\t"				\
-	     ASM_RET					\
-	     ".size " #func ", . - " #func "\n\t"	\
-	     ".popsection")
-
 extern void default_banner(void);
 
 #else  /* __ASSEMBLY__ */
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 4acbcddddc29..fe58f1882e9c 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -557,8 +557,6 @@ void paravirt_enter_lazy_mmu(void);
 void paravirt_leave_lazy_mmu(void);
 void paravirt_flush_lazy_mmu(void);
 
-void _paravirt_nop(void);
-void paravirt_BUG(void);
 unsigned long paravirt_ret0(void);
 #ifdef CONFIG_PARAVIRT_XXL
 u64 _paravirt_ident_64(u64);
@@ -568,7 +566,7 @@ void pv_native_irq_enable(void);
 unsigned long pv_native_read_cr2(void);
 #endif
 
-#define paravirt_nop	((void *)_paravirt_nop)
+#define paravirt_nop	((void *)x86_nop)
 
 extern struct paravirt_patch_site __parainstructions[],
 	__parainstructions_end[];
diff --git a/arch/x86/include/asm/qspinlock_paravirt.h b/arch/x86/include/asm/qspinlock_paravirt.h
index 42b17cf10b10..2189b3379b1c 100644
--- a/arch/x86/include/asm/qspinlock_paravirt.h
+++ b/arch/x86/include/asm/qspinlock_paravirt.h
@@ -54,8 +54,8 @@ __PV_CALLEE_SAVE_REGS_THUNK(__pv_queued_spin_unlock_slowpath, ".spinlock.text");
 	"pop    %rdx\n\t"						\
 	FRAME_END
 
-DEFINE_PARAVIRT_ASM(__raw_callee_save___pv_queued_spin_unlock,
-		    PV_UNLOCK_ASM, .spinlock.text);
+DEFINE_ASM_FUNC(__raw_callee_save___pv_queued_spin_unlock,
+		PV_UNLOCK_ASM, .spinlock.text);
 
 #else /* CONFIG_64BIT */
 
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index f615e0cb6d93..b7c70479fe58 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -253,6 +253,16 @@ static void __init_or_module noinline optimize_nops(u8 *instr, size_t len)
 	}
 }
 
+/* Low-level backend functions usable from alternative code replacements. */
+DEFINE_ASM_FUNC(x86_nop, "", .entry.text);
+EXPORT_SYMBOL_GPL(x86_nop);
+
+noinstr void x86_BUG(void)
+{
+	BUG();
+}
+EXPORT_SYMBOL_GPL(x86_BUG);
+
 /*
  * Replace instructions with better alternatives for this CPU type. This runs
  * before SMP is initialized to avoid SMP problems with self modifying code.
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 1cceac5984da..d025fc630115 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -803,8 +803,8 @@ extern bool __raw_callee_save___kvm_vcpu_is_preempted(long);
  "cmpb   $0, " __stringify(KVM_STEAL_TIME_preempted) "+steal_time(%rax)\n\t" \
  "setne  %al\n\t"
 
-DEFINE_PARAVIRT_ASM(__raw_callee_save___kvm_vcpu_is_preempted,
-		    PV_VCPU_PREEMPTED_ASM, .text);
+DEFINE_ASM_FUNC(__raw_callee_save___kvm_vcpu_is_preempted,
+		PV_VCPU_PREEMPTED_ASM, .text);
 #endif
 
 static void __init kvm_guest_init(void)
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index ac10b46c5832..dfad56679f88 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -34,14 +34,8 @@
 #include <asm/io_bitmap.h>
 #include <asm/gsseg.h>
 
-/*
- * nop stub, which must not clobber anything *including the stack* to
- * avoid confusing the entry prologues.
- */
-DEFINE_PARAVIRT_ASM(_paravirt_nop, "", .entry.text);
-
 /* stub always returning 0. */
-DEFINE_PARAVIRT_ASM(paravirt_ret0, "xor %eax,%eax", .entry.text);
+DEFINE_ASM_FUNC(paravirt_ret0, "xor %eax,%eax", .entry.text);
 
 void __init default_banner(void)
 {
@@ -49,12 +43,6 @@ void __init default_banner(void)
 	       pv_info.name);
 }
 
-/* Undefined instruction for dealing with missing ops pointers. */
-noinstr void paravirt_BUG(void)
-{
-	BUG();
-}
-
 static unsigned paravirt_patch_call(void *insn_buff, const void *target,
 				    unsigned long addr, unsigned len)
 {
@@ -64,11 +52,11 @@ static unsigned paravirt_patch_call(void *insn_buff, const void *target,
 }
 
 #ifdef CONFIG_PARAVIRT_XXL
-DEFINE_PARAVIRT_ASM(_paravirt_ident_64, "mov %rdi, %rax", .text);
-DEFINE_PARAVIRT_ASM(pv_native_save_fl, "pushf; pop %rax", .noinstr.text);
-DEFINE_PARAVIRT_ASM(pv_native_irq_disable, "cli", .noinstr.text);
-DEFINE_PARAVIRT_ASM(pv_native_irq_enable, "sti", .noinstr.text);
-DEFINE_PARAVIRT_ASM(pv_native_read_cr2, "mov %cr2, %rax", .noinstr.text);
+DEFINE_ASM_FUNC(_paravirt_ident_64, "mov %rdi, %rax", .text);
+DEFINE_ASM_FUNC(pv_native_save_fl, "pushf; pop %rax", .noinstr.text);
+DEFINE_ASM_FUNC(pv_native_irq_disable, "cli", .noinstr.text);
+DEFINE_ASM_FUNC(pv_native_irq_enable, "sti", .noinstr.text);
+DEFINE_ASM_FUNC(pv_native_read_cr2, "mov %cr2, %rax", .noinstr.text);
 #endif
 
 DEFINE_STATIC_KEY_TRUE(virt_spin_lock_key);
@@ -90,9 +78,9 @@ unsigned int paravirt_patch(u8 type, void *insn_buff, unsigned long addr,
 	unsigned ret;
 
 	if (opfunc == NULL)
-		/* If there's no function, patch it with paravirt_BUG() */
-		ret = paravirt_patch_call(insn_buff, paravirt_BUG, addr, len);
-	else if (opfunc == _paravirt_nop)
+		/* If there's no function, patch it with x86_BUG() */
+		ret = paravirt_patch_call(insn_buff, x86_BUG, addr, len);
+	else if (opfunc == x86_nop)
 		ret = 0;
 	else
 		/* Otherwise call the function. */
diff --git a/arch/x86/xen/irq.c b/arch/x86/xen/irq.c
index 6092fea7d651..5d132f5a5d7d 100644
--- a/arch/x86/xen/irq.c
+++ b/arch/x86/xen/irq.c
@@ -45,7 +45,7 @@ static const typeof(pv_ops) xen_irq_ops __initconst = {
 		/* Initial interrupt flag handling only called while interrupts off. */
 		.save_fl = __PV_IS_CALLEE_SAVE(paravirt_ret0),
 		.irq_disable = __PV_IS_CALLEE_SAVE(paravirt_nop),
-		.irq_enable = __PV_IS_CALLEE_SAVE(paravirt_BUG),
+		.irq_enable = __PV_IS_CALLEE_SAVE(x86_BUG),
 
 		.safe_halt = xen_safe_halt,
 		.halt = xen_halt,
-- 
2.35.3

Re: [RFC PATCH 0/3] x86/paravirt: Get rid of paravirt patching

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 1966 bytes --]

Any comments?

On 08.06.23 16:03, Juergen Gross wrote:
> 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.
> 
> RFC because I'm quite sure there will be some objtool work needed
> (at least removing the specific paravirt handling).
> 
> Juergen Gross (3):
>    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
> 
>   arch/x86/include/asm/alternative.h        | 26 +++++-
>   arch/x86/include/asm/paravirt.h           | 39 ++-------
>   arch/x86/include/asm/paravirt_types.h     | 68 +++-------------
>   arch/x86/include/asm/qspinlock_paravirt.h |  4 +-
>   arch/x86/include/asm/text-patching.h      | 12 ---
>   arch/x86/kernel/alternative.c             | 99 +++++++++++------------
>   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, 111 insertions(+), 249 deletions(-)
> 


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [RFC PATCH 0/3] x86/paravirt: Get rid of paravirt patching

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 2237 bytes --]

PING!

On 10.07.23 14:29, Juergen Gross wrote:
> Any comments?
> 
> On 08.06.23 16:03, Juergen Gross wrote:
>> 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.
>>
>> RFC because I'm quite sure there will be some objtool work needed
>> (at least removing the specific paravirt handling).
>>
>> Juergen Gross (3):
>>    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
>>
>>   arch/x86/include/asm/alternative.h        | 26 +++++-
>>   arch/x86/include/asm/paravirt.h           | 39 ++-------
>>   arch/x86/include/asm/paravirt_types.h     | 68 +++-------------
>>   arch/x86/include/asm/qspinlock_paravirt.h |  4 +-
>>   arch/x86/include/asm/text-patching.h      | 12 ---
>>   arch/x86/kernel/alternative.c             | 99 +++++++++++------------
>>   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, 111 insertions(+), 249 deletions(-)
>>
> 


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [RFC PATCH 1/3] x86/paravirt: move some functions and defines to alternative

[Peter Zijlstra]

On Thu, Jun 08, 2023 at 04:03:31PM +0200, Juergen Gross wrote:
> As a preparation for replacing paravirt patching completely by
> alternative patching, move some backend functions and #defines to
> alternative code and header.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>