All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/paravirt: convert simple paravirt functions to asm
@ 2023-03-08 15:42 ` Juergen Gross
  0 siblings, 0 replies; 14+ messages in thread
From: Juergen Gross via Virtualization @ 2023-03-08 15:42 UTC (permalink / raw)
  To: linux-kernel, x86, virtualization
  Cc: Juergen Gross, H. Peter Anvin, VMware PV-Drivers Reviewers,
	Dave Hansen, Ingo Molnar, Borislav Petkov, Alexey Makhalov,
	Thomas Gleixner

All functions referenced via __PV_IS_CALLEE_SAVE() need to be assembler
functions, as those functions calls are hidden from gcc. In case the
kernel is compiled with "-fzero-call-used-regs" the compiler will
clobber caller-saved registers at the end of C functions, which will
result in unexpectedly zeroed registers at the call site of the
related paravirt functions.

Replace the C functions with DEFINE_PARAVIRT_ASM() constructs using
the same instructions as the related paravirt calls in the
PVOP_ALT_[V]CALLEE*() macros.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/include/asm/paravirt_types.h |  8 +++++++-
 arch/x86/kernel/paravirt.c            | 27 ++++++---------------------
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 8c1da419260f..49f5c6955229 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -562,8 +562,14 @@ void paravirt_flush_lazy_mmu(void);
 
 void _paravirt_nop(void);
 void paravirt_BUG(void);
-u64 _paravirt_ident_64(u64);
 unsigned long paravirt_ret0(void);
+#ifdef CONFIG_PARAVIRT_XXL
+u64 _paravirt_ident_64(u64);
+unsigned long pv_native_save_fl(void);
+void pv_native_irq_disable(void);
+void pv_native_irq_enable(void);
+unsigned long pv_native_read_cr2(void);
+#endif
 
 #define paravirt_nop	((void *)_paravirt_nop)
 
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 42e182868873..d25ac4b08c41 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -64,11 +64,11 @@ static unsigned paravirt_patch_call(void *insn_buff, const void *target,
 }
 
 #ifdef CONFIG_PARAVIRT_XXL
-/* identity function, which can be inlined */
-u64 notrace _paravirt_ident_64(u64 x)
-{
-	return x;
-}
+DEFINE_PARAVIRT_ASM(_paravirt_ident_64, "mov %rdi, %rax", .text);
+DEFINE_PARAVIRT_ASM(pv_native_save_fl, "pushf; pop %rax", .text);
+DEFINE_PARAVIRT_ASM(pv_native_irq_disable, "cli", .text);
+DEFINE_PARAVIRT_ASM(pv_native_irq_enable, "sti", .text);
+DEFINE_PARAVIRT_ASM(pv_native_read_cr2, "mov %cr2, %rax", .text);
 #endif
 
 DEFINE_STATIC_KEY_TRUE(virt_spin_lock_key);
@@ -197,11 +197,6 @@ void paravirt_end_context_switch(struct task_struct *next)
 		arch_enter_lazy_mmu_mode();
 }
 
-static noinstr unsigned long pv_native_read_cr2(void)
-{
-	return native_read_cr2();
-}
-
 static noinstr void pv_native_write_cr2(unsigned long val)
 {
 	native_write_cr2(val);
@@ -222,16 +217,6 @@ noinstr void pv_native_wbinvd(void)
 	native_wbinvd();
 }
 
-static noinstr void pv_native_irq_enable(void)
-{
-	native_irq_enable();
-}
-
-static noinstr void pv_native_irq_disable(void)
-{
-	native_irq_disable();
-}
-
 static noinstr void pv_native_safe_halt(void)
 {
 	native_safe_halt();
@@ -298,7 +283,7 @@ struct paravirt_patch_template pv_ops = {
 	.cpu.end_context_switch		= paravirt_nop,
 
 	/* Irq ops. */
-	.irq.save_fl		= __PV_IS_CALLEE_SAVE(native_save_fl),
+	.irq.save_fl		= __PV_IS_CALLEE_SAVE(pv_native_save_fl),
 	.irq.irq_disable	= __PV_IS_CALLEE_SAVE(pv_native_irq_disable),
 	.irq.irq_enable		= __PV_IS_CALLEE_SAVE(pv_native_irq_enable),
 	.irq.safe_halt		= pv_native_safe_halt,
-- 
2.35.3

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH] x86/paravirt: convert simple paravirt functions to asm
@ 2023-03-08 15:42 ` Juergen Gross
  0 siblings, 0 replies; 14+ messages in thread
From: Juergen Gross @ 2023-03-08 15:42 UTC (permalink / raw)
  To: linux-kernel, x86, virtualization
  Cc: Juergen Gross, Srivatsa S. Bhat (VMware),
	Alexey Makhalov, VMware PV-Drivers Reviewers, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin

All functions referenced via __PV_IS_CALLEE_SAVE() need to be assembler
functions, as those functions calls are hidden from gcc. In case the
kernel is compiled with "-fzero-call-used-regs" the compiler will
clobber caller-saved registers at the end of C functions, which will
result in unexpectedly zeroed registers at the call site of the
related paravirt functions.

Replace the C functions with DEFINE_PARAVIRT_ASM() constructs using
the same instructions as the related paravirt calls in the
PVOP_ALT_[V]CALLEE*() macros.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/include/asm/paravirt_types.h |  8 +++++++-
 arch/x86/kernel/paravirt.c            | 27 ++++++---------------------
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 8c1da419260f..49f5c6955229 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -562,8 +562,14 @@ void paravirt_flush_lazy_mmu(void);
 
 void _paravirt_nop(void);
 void paravirt_BUG(void);
-u64 _paravirt_ident_64(u64);
 unsigned long paravirt_ret0(void);
+#ifdef CONFIG_PARAVIRT_XXL
+u64 _paravirt_ident_64(u64);
+unsigned long pv_native_save_fl(void);
+void pv_native_irq_disable(void);
+void pv_native_irq_enable(void);
+unsigned long pv_native_read_cr2(void);
+#endif
 
 #define paravirt_nop	((void *)_paravirt_nop)
 
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 42e182868873..d25ac4b08c41 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -64,11 +64,11 @@ static unsigned paravirt_patch_call(void *insn_buff, const void *target,
 }
 
 #ifdef CONFIG_PARAVIRT_XXL
-/* identity function, which can be inlined */
-u64 notrace _paravirt_ident_64(u64 x)
-{
-	return x;
-}
+DEFINE_PARAVIRT_ASM(_paravirt_ident_64, "mov %rdi, %rax", .text);
+DEFINE_PARAVIRT_ASM(pv_native_save_fl, "pushf; pop %rax", .text);
+DEFINE_PARAVIRT_ASM(pv_native_irq_disable, "cli", .text);
+DEFINE_PARAVIRT_ASM(pv_native_irq_enable, "sti", .text);
+DEFINE_PARAVIRT_ASM(pv_native_read_cr2, "mov %cr2, %rax", .text);
 #endif
 
 DEFINE_STATIC_KEY_TRUE(virt_spin_lock_key);
@@ -197,11 +197,6 @@ void paravirt_end_context_switch(struct task_struct *next)
 		arch_enter_lazy_mmu_mode();
 }
 
-static noinstr unsigned long pv_native_read_cr2(void)
-{
-	return native_read_cr2();
-}
-
 static noinstr void pv_native_write_cr2(unsigned long val)
 {
 	native_write_cr2(val);
@@ -222,16 +217,6 @@ noinstr void pv_native_wbinvd(void)
 	native_wbinvd();
 }
 
-static noinstr void pv_native_irq_enable(void)
-{
-	native_irq_enable();
-}
-
-static noinstr void pv_native_irq_disable(void)
-{
-	native_irq_disable();
-}
-
 static noinstr void pv_native_safe_halt(void)
 {
 	native_safe_halt();
@@ -298,7 +283,7 @@ struct paravirt_patch_template pv_ops = {
 	.cpu.end_context_switch		= paravirt_nop,
 
 	/* Irq ops. */
-	.irq.save_fl		= __PV_IS_CALLEE_SAVE(native_save_fl),
+	.irq.save_fl		= __PV_IS_CALLEE_SAVE(pv_native_save_fl),
 	.irq.irq_disable	= __PV_IS_CALLEE_SAVE(pv_native_irq_disable),
 	.irq.irq_enable		= __PV_IS_CALLEE_SAVE(pv_native_irq_enable),
 	.irq.safe_halt		= pv_native_safe_halt,
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
  2023-03-08 15:42 ` Juergen Gross
@ 2023-03-09 13:39   ` Borislav Petkov
  -1 siblings, 0 replies; 14+ messages in thread
From: Borislav Petkov @ 2023-03-09 13:39 UTC (permalink / raw)
  To: Juergen Gross
  Cc: linux-kernel, x86, virtualization, Srivatsa S. Bhat (VMware),
	Alexey Makhalov, VMware PV-Drivers Reviewers, Thomas Gleixner,
	Ingo Molnar, Dave Hansen, H. Peter Anvin

On Wed, Mar 08, 2023 at 04:42:10PM +0100, Juergen Gross wrote:
> All functions referenced via __PV_IS_CALLEE_SAVE() need to be assembler
> functions, as those functions calls are hidden from gcc. In case the
> kernel is compiled with "-fzero-call-used-regs" the compiler will
> clobber caller-saved registers at the end of C functions, which will
> result in unexpectedly zeroed registers at the call site of the
> related paravirt functions.
> 
> Replace the C functions with DEFINE_PARAVIRT_ASM() constructs using
> the same instructions as the related paravirt calls in the
> PVOP_ALT_[V]CALLEE*() macros.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  arch/x86/include/asm/paravirt_types.h |  8 +++++++-
>  arch/x86/kernel/paravirt.c            | 27 ++++++---------------------
>  2 files changed, 13 insertions(+), 22 deletions(-)

Right, works with my particular reproducer.

Turning them into asm prevents the compiler from doing the
callee-clobbered zeroing and that's fine as this whole paravirt gunk is
hiding the "CALL" insn from it and you putting them in asm is in line
with this.

And a negative diffstat..

So yeah, I'll queue it soon unless someone objects.

Long term, I think we should continue switching all that pv stuff to
using the alternatives.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
@ 2023-03-09 13:39   ` Borislav Petkov
  0 siblings, 0 replies; 14+ messages in thread
From: Borislav Petkov @ 2023-03-09 13:39 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Dave Hansen, H. Peter Anvin, VMware PV-Drivers Reviewers, x86,
	linux-kernel, virtualization, Ingo Molnar, Alexey Makhalov,
	Thomas Gleixner

On Wed, Mar 08, 2023 at 04:42:10PM +0100, Juergen Gross wrote:
> All functions referenced via __PV_IS_CALLEE_SAVE() need to be assembler
> functions, as those functions calls are hidden from gcc. In case the
> kernel is compiled with "-fzero-call-used-regs" the compiler will
> clobber caller-saved registers at the end of C functions, which will
> result in unexpectedly zeroed registers at the call site of the
> related paravirt functions.
> 
> Replace the C functions with DEFINE_PARAVIRT_ASM() constructs using
> the same instructions as the related paravirt calls in the
> PVOP_ALT_[V]CALLEE*() macros.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  arch/x86/include/asm/paravirt_types.h |  8 +++++++-
>  arch/x86/kernel/paravirt.c            | 27 ++++++---------------------
>  2 files changed, 13 insertions(+), 22 deletions(-)

Right, works with my particular reproducer.

Turning them into asm prevents the compiler from doing the
callee-clobbered zeroing and that's fine as this whole paravirt gunk is
hiding the "CALL" insn from it and you putting them in asm is in line
with this.

And a negative diffstat..

So yeah, I'll queue it soon unless someone objects.

Long term, I think we should continue switching all that pv stuff to
using the alternatives.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
  2023-03-09 13:39   ` Borislav Petkov
@ 2023-03-10  6:24     ` Juergen Gross via Virtualization
  -1 siblings, 0 replies; 14+ messages in thread
From: Juergen Gross @ 2023-03-10  6:24 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-kernel, x86, virtualization, Srivatsa S. Bhat (VMware),
	Alexey Makhalov, VMware PV-Drivers Reviewers, Thomas Gleixner,
	Ingo Molnar, Dave Hansen, H. Peter Anvin


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

On 09.03.23 14:39, Borislav Petkov wrote:
> On Wed, Mar 08, 2023 at 04:42:10PM +0100, Juergen Gross wrote:
>> All functions referenced via __PV_IS_CALLEE_SAVE() need to be assembler
>> functions, as those functions calls are hidden from gcc. In case the
>> kernel is compiled with "-fzero-call-used-regs" the compiler will
>> clobber caller-saved registers at the end of C functions, which will
>> result in unexpectedly zeroed registers at the call site of the
>> related paravirt functions.
>>
>> Replace the C functions with DEFINE_PARAVIRT_ASM() constructs using
>> the same instructions as the related paravirt calls in the
>> PVOP_ALT_[V]CALLEE*() macros.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   arch/x86/include/asm/paravirt_types.h |  8 +++++++-
>>   arch/x86/kernel/paravirt.c            | 27 ++++++---------------------
>>   2 files changed, 13 insertions(+), 22 deletions(-)
> 
> Right, works with my particular reproducer.
> 
> Turning them into asm prevents the compiler from doing the
> callee-clobbered zeroing and that's fine as this whole paravirt gunk is
> hiding the "CALL" insn from it and you putting them in asm is in line
> with this.
> 
> And a negative diffstat..
> 
> So yeah, I'll queue it soon unless someone objects.

Thanks.

> Long term, I think we should continue switching all that pv stuff to
> using the alternatives.

The "normal" cases not using alternatives should rather be switched to
static calls.

Whether it is possible to mix a static call with alternatives needs to
be evaluated.


Juergen

[-- 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 --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
@ 2023-03-10  6:24     ` Juergen Gross via Virtualization
  0 siblings, 0 replies; 14+ messages in thread
From: Juergen Gross via Virtualization @ 2023-03-10  6:24 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Dave Hansen, H. Peter Anvin, VMware PV-Drivers Reviewers, x86,
	linux-kernel, virtualization, Ingo Molnar, Alexey Makhalov,
	Thomas Gleixner


[-- Attachment #1.1.1.1: Type: text/plain, Size: 1603 bytes --]

On 09.03.23 14:39, Borislav Petkov wrote:
> On Wed, Mar 08, 2023 at 04:42:10PM +0100, Juergen Gross wrote:
>> All functions referenced via __PV_IS_CALLEE_SAVE() need to be assembler
>> functions, as those functions calls are hidden from gcc. In case the
>> kernel is compiled with "-fzero-call-used-regs" the compiler will
>> clobber caller-saved registers at the end of C functions, which will
>> result in unexpectedly zeroed registers at the call site of the
>> related paravirt functions.
>>
>> Replace the C functions with DEFINE_PARAVIRT_ASM() constructs using
>> the same instructions as the related paravirt calls in the
>> PVOP_ALT_[V]CALLEE*() macros.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>   arch/x86/include/asm/paravirt_types.h |  8 +++++++-
>>   arch/x86/kernel/paravirt.c            | 27 ++++++---------------------
>>   2 files changed, 13 insertions(+), 22 deletions(-)
> 
> Right, works with my particular reproducer.
> 
> Turning them into asm prevents the compiler from doing the
> callee-clobbered zeroing and that's fine as this whole paravirt gunk is
> hiding the "CALL" insn from it and you putting them in asm is in line
> with this.
> 
> And a negative diffstat..
> 
> So yeah, I'll queue it soon unless someone objects.

Thanks.

> Long term, I think we should continue switching all that pv stuff to
> using the alternatives.

The "normal" cases not using alternatives should rather be switched to
static calls.

Whether it is possible to mix a static call with alternatives needs to
be evaluated.


Juergen

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

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

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
  2023-03-10  6:24     ` Juergen Gross via Virtualization
@ 2023-03-12 20:47       ` Borislav Petkov
  -1 siblings, 0 replies; 14+ messages in thread
From: Borislav Petkov @ 2023-03-12 20:47 UTC (permalink / raw)
  To: Juergen Gross
  Cc: linux-kernel, x86, virtualization, Srivatsa S. Bhat (VMware),
	Alexey Makhalov, VMware PV-Drivers Reviewers, Thomas Gleixner,
	Ingo Molnar, Dave Hansen, H. Peter Anvin

On Fri, Mar 10, 2023 at 07:24:17AM +0100, Juergen Gross wrote:
> The "normal" cases not using alternatives should rather be switched to
> static calls.

Or that.

> Whether it is possible to mix a static call with alternatives needs to
> be evaluated.

I'd prefer not to mix them. Either should be fine and if neither have
the required functionality, then it should be added depending on which
- static calls or alternatives - would make things simpler.

I'd love to get rid of the whole paravirt glue and use the facilities we
have in the tree instead.

But no hurry - it should be nice and clean work. :-)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
@ 2023-03-12 20:47       ` Borislav Petkov
  0 siblings, 0 replies; 14+ messages in thread
From: Borislav Petkov @ 2023-03-12 20:47 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Dave Hansen, H. Peter Anvin, VMware PV-Drivers Reviewers, x86,
	linux-kernel, virtualization, Ingo Molnar, Alexey Makhalov,
	Thomas Gleixner

On Fri, Mar 10, 2023 at 07:24:17AM +0100, Juergen Gross wrote:
> The "normal" cases not using alternatives should rather be switched to
> static calls.

Or that.

> Whether it is possible to mix a static call with alternatives needs to
> be evaluated.

I'd prefer not to mix them. Either should be fine and if neither have
the required functionality, then it should be added depending on which
- static calls or alternatives - would make things simpler.

I'd love to get rid of the whole paravirt glue and use the facilities we
have in the tree instead.

But no hurry - it should be nice and clean work. :-)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
  2023-03-08 15:42 ` Juergen Gross
@ 2023-03-16 16:50   ` Borislav Petkov
  -1 siblings, 0 replies; 14+ messages in thread
From: Borislav Petkov @ 2023-03-16 16:50 UTC (permalink / raw)
  To: Juergen Gross
  Cc: linux-kernel, x86, virtualization, Srivatsa S. Bhat (VMware),
	Alexey Makhalov, VMware PV-Drivers Reviewers, Thomas Gleixner,
	Ingo Molnar, Dave Hansen, H. Peter Anvin

On Wed, Mar 08, 2023 at 04:42:10PM +0100, Juergen Gross wrote:
> All functions referenced via __PV_IS_CALLEE_SAVE() need to be assembler
> functions, as those functions calls are hidden from gcc. In case the
> kernel is compiled with "-fzero-call-used-regs" the compiler will
> clobber caller-saved registers at the end of C functions, which will
> result in unexpectedly zeroed registers at the call site of the
> related paravirt functions.
> 
> Replace the C functions with DEFINE_PARAVIRT_ASM() constructs using
> the same instructions as the related paravirt calls in the
> PVOP_ALT_[V]CALLEE*() macros.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  arch/x86/include/asm/paravirt_types.h |  8 +++++++-
>  arch/x86/kernel/paravirt.c            | 27 ++++++---------------------
>  2 files changed, 13 insertions(+), 22 deletions(-)

objtool's not happy with this for whatever reason. I'll look later as to
why. .config is allmodconfig with this patch ontop of tip:x86/paravirt:

vmlinux.o: warning: objtool: pv_ops[31]: pv_native_irq_disable
vmlinux.o: warning: objtool: default_idle+0x1e: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[31]: pv_native_irq_disable
vmlinux.o: warning: objtool: mwait_idle+0x5d: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[31]: pv_native_irq_disable
vmlinux.o: warning: objtool: cpu_idle_poll.isra.0+0x94: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[31]: pv_native_irq_disable
vmlinux.o: warning: objtool: intel_idle_irq+0xab: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[31]: pv_native_irq_disable
vmlinux.o: warning: objtool: acpi_safe_halt+0x2a: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[31]: pv_native_irq_disable
vmlinux.o: warning: objtool: poll_idle+0x86: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[42]: pv_native_read_cr2
vmlinux.o: warning: objtool: exc_double_fault+0x3b: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[42]: pv_native_read_cr2
vmlinux.o: warning: objtool: exc_nmi+0x188: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: __sev_put_ghcb+0x11: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: __sev_get_ghcb+0x13: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[42]: pv_native_read_cr2
vmlinux.o: warning: objtool: exc_page_fault+0x1e: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: lockdep_hardirqs_on+0xd0: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: lockdep_hardirqs_off+0xe7: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: look_up_lock_class+0x52: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[32]: pv_native_irq_enable
vmlinux.o: warning: objtool: lock_is_held_type+0x143: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: ct_kernel_enter.constprop.0+0x37: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[32]: pv_native_irq_enable
vmlinux.o: warning: objtool: ct_idle_exit+0x51: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: ct_idle_enter+0xe: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: check_preemption_disabled+0x4c: call to {dynamic}() leaves .noinstr.text section

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
@ 2023-03-16 16:50   ` Borislav Petkov
  0 siblings, 0 replies; 14+ messages in thread
From: Borislav Petkov @ 2023-03-16 16:50 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Dave Hansen, H. Peter Anvin, VMware PV-Drivers Reviewers, x86,
	linux-kernel, virtualization, Ingo Molnar, Alexey Makhalov,
	Thomas Gleixner

On Wed, Mar 08, 2023 at 04:42:10PM +0100, Juergen Gross wrote:
> All functions referenced via __PV_IS_CALLEE_SAVE() need to be assembler
> functions, as those functions calls are hidden from gcc. In case the
> kernel is compiled with "-fzero-call-used-regs" the compiler will
> clobber caller-saved registers at the end of C functions, which will
> result in unexpectedly zeroed registers at the call site of the
> related paravirt functions.
> 
> Replace the C functions with DEFINE_PARAVIRT_ASM() constructs using
> the same instructions as the related paravirt calls in the
> PVOP_ALT_[V]CALLEE*() macros.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  arch/x86/include/asm/paravirt_types.h |  8 +++++++-
>  arch/x86/kernel/paravirt.c            | 27 ++++++---------------------
>  2 files changed, 13 insertions(+), 22 deletions(-)

objtool's not happy with this for whatever reason. I'll look later as to
why. .config is allmodconfig with this patch ontop of tip:x86/paravirt:

vmlinux.o: warning: objtool: pv_ops[31]: pv_native_irq_disable
vmlinux.o: warning: objtool: default_idle+0x1e: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[31]: pv_native_irq_disable
vmlinux.o: warning: objtool: mwait_idle+0x5d: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[31]: pv_native_irq_disable
vmlinux.o: warning: objtool: cpu_idle_poll.isra.0+0x94: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[31]: pv_native_irq_disable
vmlinux.o: warning: objtool: intel_idle_irq+0xab: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[31]: pv_native_irq_disable
vmlinux.o: warning: objtool: acpi_safe_halt+0x2a: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[31]: pv_native_irq_disable
vmlinux.o: warning: objtool: poll_idle+0x86: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[42]: pv_native_read_cr2
vmlinux.o: warning: objtool: exc_double_fault+0x3b: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[42]: pv_native_read_cr2
vmlinux.o: warning: objtool: exc_nmi+0x188: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: __sev_put_ghcb+0x11: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: __sev_get_ghcb+0x13: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[42]: pv_native_read_cr2
vmlinux.o: warning: objtool: exc_page_fault+0x1e: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: lockdep_hardirqs_on+0xd0: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: lockdep_hardirqs_off+0xe7: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: look_up_lock_class+0x52: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[32]: pv_native_irq_enable
vmlinux.o: warning: objtool: lock_is_held_type+0x143: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: ct_kernel_enter.constprop.0+0x37: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[32]: pv_native_irq_enable
vmlinux.o: warning: objtool: ct_idle_exit+0x51: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: ct_idle_enter+0xe: call to {dynamic}() leaves .noinstr.text section
vmlinux.o: warning: objtool: pv_ops[30]: pv_native_save_fl
vmlinux.o: warning: objtool: check_preemption_disabled+0x4c: call to {dynamic}() leaves .noinstr.text section

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
  2023-03-08 15:42 ` Juergen Gross
@ 2023-03-16 20:14   ` Peter Zijlstra
  -1 siblings, 0 replies; 14+ messages in thread
From: Peter Zijlstra @ 2023-03-16 20:14 UTC (permalink / raw)
  To: Juergen Gross
  Cc: linux-kernel, x86, virtualization, Srivatsa S. Bhat (VMware),
	Alexey Makhalov, VMware PV-Drivers Reviewers, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin

On Wed, Mar 08, 2023 at 04:42:10PM +0100, Juergen Gross wrote:

> +DEFINE_PARAVIRT_ASM(pv_native_irq_disable, "cli", .text);
> +DEFINE_PARAVIRT_ASM(pv_native_irq_enable, "sti", .text);
> +DEFINE_PARAVIRT_ASM(pv_native_read_cr2, "mov %cr2, %rax", .text);

per these v, the above ^ should be in .noinstr.text

> -static noinstr unsigned long pv_native_read_cr2(void)
> -static noinstr void pv_native_irq_enable(void)
> -static noinstr void pv_native_irq_disable(void)



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
@ 2023-03-16 20:14   ` Peter Zijlstra
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Zijlstra @ 2023-03-16 20:14 UTC (permalink / raw)
  To: Juergen Gross
  Cc: Dave Hansen, H. Peter Anvin, VMware PV-Drivers Reviewers, x86,
	linux-kernel, virtualization, Ingo Molnar, Borislav Petkov,
	Alexey Makhalov, Thomas Gleixner

On Wed, Mar 08, 2023 at 04:42:10PM +0100, Juergen Gross wrote:

> +DEFINE_PARAVIRT_ASM(pv_native_irq_disable, "cli", .text);
> +DEFINE_PARAVIRT_ASM(pv_native_irq_enable, "sti", .text);
> +DEFINE_PARAVIRT_ASM(pv_native_read_cr2, "mov %cr2, %rax", .text);

per these v, the above ^ should be in .noinstr.text

> -static noinstr unsigned long pv_native_read_cr2(void)
> -static noinstr void pv_native_irq_enable(void)
> -static noinstr void pv_native_irq_disable(void)


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
  2023-03-16 20:14   ` Peter Zijlstra
@ 2023-03-17  5:28     ` Juergen Gross via Virtualization
  -1 siblings, 0 replies; 14+ messages in thread
From: Juergen Gross @ 2023-03-17  5:28 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: linux-kernel, x86, virtualization, Srivatsa S. Bhat (VMware),
	Alexey Makhalov, VMware PV-Drivers Reviewers, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Dave Hansen, H. Peter Anvin


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

On 16.03.23 21:14, Peter Zijlstra wrote:
> On Wed, Mar 08, 2023 at 04:42:10PM +0100, Juergen Gross wrote:
> 
>> +DEFINE_PARAVIRT_ASM(pv_native_irq_disable, "cli", .text);
>> +DEFINE_PARAVIRT_ASM(pv_native_irq_enable, "sti", .text);
>> +DEFINE_PARAVIRT_ASM(pv_native_read_cr2, "mov %cr2, %rax", .text);
> 
> per these v, the above ^ should be in .noinstr.text

Yes, and I'm inclined to even put pv_native_save_fl into the noinstr
section. After paravirt patching it isn't called anymore anyway.


Juergen

[-- 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 --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
@ 2023-03-17  5:28     ` Juergen Gross via Virtualization
  0 siblings, 0 replies; 14+ messages in thread
From: Juergen Gross via Virtualization @ 2023-03-17  5:28 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Dave Hansen, H. Peter Anvin, VMware PV-Drivers Reviewers, x86,
	linux-kernel, virtualization, Ingo Molnar, Borislav Petkov,
	Alexey Makhalov, Thomas Gleixner


[-- Attachment #1.1.1.1: Type: text/plain, Size: 518 bytes --]

On 16.03.23 21:14, Peter Zijlstra wrote:
> On Wed, Mar 08, 2023 at 04:42:10PM +0100, Juergen Gross wrote:
> 
>> +DEFINE_PARAVIRT_ASM(pv_native_irq_disable, "cli", .text);
>> +DEFINE_PARAVIRT_ASM(pv_native_irq_enable, "sti", .text);
>> +DEFINE_PARAVIRT_ASM(pv_native_read_cr2, "mov %cr2, %rax", .text);
> 
> per these v, the above ^ should be in .noinstr.text

Yes, and I'm inclined to even put pv_native_save_fl into the noinstr
section. After paravirt patching it isn't called anymore anyway.


Juergen

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

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

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-03-17  5:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-08 15:42 [PATCH] x86/paravirt: convert simple paravirt functions to asm Juergen Gross via Virtualization
2023-03-08 15:42 ` Juergen Gross
2023-03-09 13:39 ` Borislav Petkov
2023-03-09 13:39   ` Borislav Petkov
2023-03-10  6:24   ` Juergen Gross
2023-03-10  6:24     ` Juergen Gross via Virtualization
2023-03-12 20:47     ` Borislav Petkov
2023-03-12 20:47       ` Borislav Petkov
2023-03-16 16:50 ` Borislav Petkov
2023-03-16 16:50   ` Borislav Petkov
2023-03-16 20:14 ` Peter Zijlstra
2023-03-16 20:14   ` Peter Zijlstra
2023-03-17  5:28   ` Juergen Gross
2023-03-17  5:28     ` Juergen Gross via Virtualization

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.