Re: [PATCH 2/4] kvm-unit-test: nVMX: __enter_guest() needs to also check for VMX_FAIL_STATE

[Sean Christopherson <sean.j.christopherson@xxxxxxxxx>]

On Wed, Oct 16, 2019 at 10:55:55AM -0700, Jim Mattson wrote:
> On Mon, Oct 14, 2019 at 5:52 PM Krish Sadhukhan
> <krish.sadhukhan@xxxxxxxxxx> wrote:
> >
> >   ..as both VMX_ENTRY_FAILURE and VMX_FAIL_STATE together comprise the
> >     exit eeason when VM-entry fails due invalid guest state.
> 
> Nit: s/reason/reason/
> 
> > Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
> > Reviewed-by: Karl Heubaum <karl.heubaum@xxxxxxxxxx>
> > ---
> >  x86/vmx.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/x86/vmx.c b/x86/vmx.c
> > index 647ab49..4ce0fb5 100644
> > --- a/x86/vmx.c
> > +++ b/x86/vmx.c
> > @@ -1848,7 +1848,8 @@ static void __enter_guest(u8 abort_flag, struct vmentry_failure *failure)
> >         vmx_enter_guest(failure);
> >         if ((abort_flag & ABORT_ON_EARLY_VMENTRY_FAIL && failure->early) ||
> >             (abort_flag & ABORT_ON_INVALID_GUEST_STATE &&
> > -           vmcs_read(EXI_REASON) & VMX_ENTRY_FAILURE)) {
> > +           (vmcs_read(EXI_REASON) & (VMX_ENTRY_FAILURE | VMX_FAIL_STATE))
> > +           == (VMX_ENTRY_FAILURE | VMX_FAIL_STATE))) {
> This shouldn't be a bitwise comparison. It should just be a value comparison:
> 
> vmcs_read(EXI_REASON) == VMX_ENTRY_FAILURE | VMX_FAIL_STATE

Hmm, but then we miss failed VM-Enter if something goes truly haywire,
e.g. KVM signals a failed VM-Enter due to #MC.

I'd do something like this:

	u32 exit_reason;

	...

	vmx_enter_guest(failure);

	if (failure->early) {
		if (abort_flag & ABORT_ON_EARLY_VMENTRY_FAIL)
			goto do_abort;
		return;
	}
	exit_reason = vmcs_read(EXI_REASON);
	if (exit_reason & VMX_ENTRY_FAILURE) {
		if ((abort_flag & ABORT_ON_INVALID_GUEST_STATE) ||
		    exit_reason != (VMX_ENTRY_FAILURE | VMX_FAIL_STATE))
			goto do_abort;
		return;
	}

	launched = 1;
	check_for_guest_termination();
	return;

do_abort:
	print_vmentry_failure_info(failure);
	abort();