Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
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 OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
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
Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
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
> ---
> 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
Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
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
Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
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 --- 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 OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] x86/paravirt: convert simple paravirt functions to asm
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 > --- > 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
