diff mbox series

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

Message ID 20191015001633.8603-3-krish.sadhukhan@oracle.com (mailing list archive)
State New, archived
Headers show
Series : kvm-unit-test: nVMX: Test deferring of error from VM-entry MSR-load area | expand

Commit Message

Krish Sadhukhan Oct. 15, 2019, 12:16 a.m. UTC
..as both VMX_ENTRY_FAILURE and VMX_FAIL_STATE together comprise the
    exit eeason when VM-entry fails due invalid guest state.

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
---
 x86/vmx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Jim Mattson Oct. 16, 2019, 5:55 p.m. UTC | #1
On Mon, Oct 14, 2019 at 5:52 PM Krish Sadhukhan
<krish.sadhukhan@oracle.com> 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@oracle.com>
> Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
> ---
>  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

>
>                 print_vmentry_failure_info(failure);
>                 abort();
> --
> 2.20.1
>
Sean Christopherson Oct. 17, 2019, 6:24 p.m. UTC | #2
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@oracle.com> 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@oracle.com>
> > Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
> > ---
> >  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();
diff mbox series

Patch

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))) {
 
 		print_vmentry_failure_info(failure);
 		abort();