Commit c20d403f authored by Sean Christopherson's avatar Sean Christopherson Committed by Paolo Bonzini
Browse files

KVM: VMX: Make VMREAD error path play nice with noinstr 



Mark vmread_error_trampoline() as noinstr, and add a second trampoline
for the CONFIG_CC_HAS_ASM_GOTO_OUTPUT=n case to enable instrumentation
when handling VM-Fail on VMREAD.  VMREAD is used in various noinstr
flows, e.g. immediately after VM-Exit, and objtool rightly complains that
the call to the error trampoline leaves a no-instrumentation section
without annotating that it's safe to do so.

  vmlinux.o: warning: objtool: vmx_vcpu_enter_exit+0xc9:
  call to vmread_error_trampoline() leaves .noinstr.text section

Note, strictly speaking, enabling instrumentation in the VM-Fail path
isn't exactly safe, but if VMREAD fails the kernel/system is likely hosed
anyways, and logging that there is a fatal error is more important than
*maybe* encountering slightly unsafe instrumentation.

Reported-by: default avatarSu Hui <suhui@nfschina.com>
Signed-off-by: default avatarSean Christopherson <seanjc@google.com>
Message-Id: <20230721235637.2345403-2-seanjc@google.com>
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent 5e1fe4a2

arch/x86/kvm/vmx/vmenter.S

+4 −4
Original line number Diff line number Diff line
@@ -303,10 +303,8 @@ SYM_FUNC_START(vmx_do_nmi_irqoff) 
	VMX_DO_EVENT_IRQOFF call asm_exc_nmi_kvm_vmx

SYM_FUNC_END(vmx_do_nmi_irqoff)





.section .text, "ax"



#ifndef CONFIG_CC_HAS_ASM_GOTO_OUTPUT



/**

 * vmread_error_trampoline - Trampoline from inline asm to vmread_error()

 * @field:	VMCS field encoding that failed
@@ -335,7 +333,7 @@ SYM_FUNC_START(vmread_error_trampoline) 
	mov 3*WORD_SIZE(%_ASM_BP), %_ASM_ARG2

	mov 2*WORD_SIZE(%_ASM_BP), %_ASM_ARG1



	call vmread_error

	call vmread_error_trampoline2



	/* Zero out @fault, which will be popped into the result register. */

	_ASM_MOV $0, 3*WORD_SIZE(%_ASM_BP)
@@ -357,6 +355,8 @@ SYM_FUNC_START(vmread_error_trampoline) 
SYM_FUNC_END(vmread_error_trampoline)

#endif



.section .text, "ax"



SYM_FUNC_START(vmx_do_interrupt_irqoff)

	VMX_DO_EVENT_IRQOFF CALL_NOSPEC _ASM_ARG1

SYM_FUNC_END(vmx_do_interrupt_irqoff)

arch/x86/kvm/vmx/vmx.c

+14 −4
Original line number Diff line number Diff line
@@ -441,14 +441,24 @@ do { \ 
	pr_warn_ratelimited(fmt);	\

} while (0)



void vmread_error(unsigned long field, bool fault)

noinline void vmread_error(unsigned long field)

{

	if (fault)

		kvm_spurious_fault();

	else

	vmx_insn_failed("vmread failed: field=%lx\n", field);

}



#ifndef CONFIG_CC_HAS_ASM_GOTO_OUTPUT

noinstr void vmread_error_trampoline2(unsigned long field, bool fault)

{

	if (fault) {

		kvm_spurious_fault();

	} else {

		instrumentation_begin();

		vmread_error(field);

		instrumentation_end();

	}

}

#endif



noinline void vmwrite_error(unsigned long field, unsigned long value)

{

	vmx_insn_failed("vmwrite failed: field=%lx val=%lx err=%u\n",

arch/x86/kvm/vmx/vmx_ops.h

+8 −1
Original line number Diff line number Diff line
@@ -10,7 +10,7 @@ 
#include "vmcs.h"

#include "../x86.h"



void vmread_error(unsigned long field, bool fault);

void vmread_error(unsigned long field);

void vmwrite_error(unsigned long field, unsigned long value);

void vmclear_error(struct vmcs *vmcs, u64 phys_addr);

void vmptrld_error(struct vmcs *vmcs, u64 phys_addr);
@@ -31,6 +31,13 @@ void invept_error(unsigned long ext, u64 eptp, gpa_t gpa); 
 * void vmread_error_trampoline(unsigned long field, bool fault);

 */

extern unsigned long vmread_error_trampoline;



/*

 * The second VMREAD error trampoline, called from the assembly trampoline,

 * exists primarily to enable instrumentation for the VM-Fail path.

 */

void vmread_error_trampoline2(unsigned long field, bool fault);



#endif



static __always_inline void vmcs_check16(unsigned long field)