[1/2] kvm: nVMX: NMI-window exiting should wake L2 from HLT
diff mbox series

Message ID 20181121002458.239673-1-jmattson@google.com
State New
Headers show
Series
  • [1/2] kvm: nVMX: NMI-window exiting should wake L2 from HLT
Related show

Commit Message

Jim Mattson Nov. 21, 2018, 12:24 a.m. UTC
According to the SDM, "NMI-window exiting" VM-exits wake a logical
processor from the same inactive states as would an NMI. Specifically,
they wake a logical processor from the shutdown state and from the
states entered using the HLT and MWAIT instructions.

Fixes: 6dfacadd5858 ("KVM: nVMX: Add support for activity state HLT")
Signed-off-by: Jim Mattson <jmattson@google.com>
Reviewed-by: Peter Shier <pshier@google.com>
---
 arch/x86/kvm/vmx.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

Comments

Sean Christopherson Nov. 26, 2018, 4:37 p.m. UTC | #1
On Tue, Nov 20, 2018 at 04:24:57PM -0800, Jim Mattson wrote:
> According to the SDM, "NMI-window exiting" VM-exits wake a logical
> processor from the same inactive states as would an NMI. Specifically,
> they wake a logical processor from the shutdown state and from the
> states entered using the HLT and MWAIT instructions.
> 
> Fixes: 6dfacadd5858 ("KVM: nVMX: Add support for activity state HLT")
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Reviewed-by: Peter Shier <pshier@google.com>
> ---
>  arch/x86/kvm/vmx.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index ccc6a01eb4f4..5f5c79e72a49 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -13421,6 +13421,17 @@ static int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
>  	return 1;
>  }
>  
> +static bool nested_vmx_nmi_window_exit(struct kvm_vcpu *vcpu)
> +{
> +	struct vmcs12 *vmcs12 = get_vmcs12(vcpu);

Any reason not to pass vmcs12 directly to the function?  vcpu isn't
used othwerwise and the call site already has and is using vmcs12.

> +	return (vmcs12->cpu_based_vm_exec_control &
> +		CPU_BASED_VIRTUAL_NMI_PENDING) &&
> +		vmcs12->guest_activity_state != GUEST_ACTIVITY_WAIT_SIPI &&
> +		!(vmcs12->guest_interruptibility_info &
> +		  (GUEST_INTR_STATE_NMI | GUEST_INTR_STATE_MOV_SS));
> +}
> +
>  /*
>   * nested_vmx_run() handles a nested entry, i.e., a VMLAUNCH or VMRESUME on L1
>   * for running an L2 nested guest.
> @@ -13512,11 +13523,13 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
>  	nested_cache_shadow_vmcs12(vcpu, vmcs12);
>  
>  	/*
> -	 * If we're entering a halted L2 vcpu and the L2 vcpu won't be woken
> -	 * by event injection, halt vcpu.
> +	 * If we're entering a halted L2 vcpu and the L2 vcpu won't be
> +	 * awakened by event injection or by an NMI-window VM-exit,
> +	 * halt the vcpu.
>  	 */
>  	if ((vmcs12->guest_activity_state == GUEST_ACTIVITY_HLT) &&
> -	    !(vmcs12->vm_entry_intr_info_field & INTR_INFO_VALID_MASK)) {
> +	    !(vmcs12->vm_entry_intr_info_field & INTR_INFO_VALID_MASK) &&
> +	    !nested_vmx_nmi_window_exit(vcpu)) {
>  		vmx->nested.nested_run_pending = 0;
>  		return kvm_vcpu_halt(vcpu);
>  	}
> -- 
> 2.19.1.1215.g8438c0b245-goog
>
Jim Mattson Nov. 26, 2018, 6:34 p.m. UTC | #2
On Mon, Nov 26, 2018 at 8:37 AM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> On Tue, Nov 20, 2018 at 04:24:57PM -0800, Jim Mattson wrote:
> > According to the SDM, "NMI-window exiting" VM-exits wake a logical
> > processor from the same inactive states as would an NMI. Specifically,
> > they wake a logical processor from the shutdown state and from the
> > states entered using the HLT and MWAIT instructions.
> >
> > Fixes: 6dfacadd5858 ("KVM: nVMX: Add support for activity state HLT")
> > Signed-off-by: Jim Mattson <jmattson@google.com>
> > Reviewed-by: Peter Shier <pshier@google.com>
> > ---
> >  arch/x86/kvm/vmx.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index ccc6a01eb4f4..5f5c79e72a49 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -13421,6 +13421,17 @@ static int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
> >       return 1;
> >  }
> >
> > +static bool nested_vmx_nmi_window_exit(struct kvm_vcpu *vcpu)
> > +{
> > +     struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
>
> Any reason not to pass vmcs12 directly to the function?  vcpu isn't
> used othwerwise and the call site already has and is using vmcs12.

It seemed potentially more versatile this way, but since there is only
one caller at the moment, I'll switch it to vmcs12 in v2.

Patch
diff mbox series

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ccc6a01eb4f4..5f5c79e72a49 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -13421,6 +13421,17 @@  static int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
 	return 1;
 }
 
+static bool nested_vmx_nmi_window_exit(struct kvm_vcpu *vcpu)
+{
+	struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
+
+	return (vmcs12->cpu_based_vm_exec_control &
+		CPU_BASED_VIRTUAL_NMI_PENDING) &&
+		vmcs12->guest_activity_state != GUEST_ACTIVITY_WAIT_SIPI &&
+		!(vmcs12->guest_interruptibility_info &
+		  (GUEST_INTR_STATE_NMI | GUEST_INTR_STATE_MOV_SS));
+}
+
 /*
  * nested_vmx_run() handles a nested entry, i.e., a VMLAUNCH or VMRESUME on L1
  * for running an L2 nested guest.
@@ -13512,11 +13523,13 @@  static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
 	nested_cache_shadow_vmcs12(vcpu, vmcs12);
 
 	/*
-	 * If we're entering a halted L2 vcpu and the L2 vcpu won't be woken
-	 * by event injection, halt vcpu.
+	 * If we're entering a halted L2 vcpu and the L2 vcpu won't be
+	 * awakened by event injection or by an NMI-window VM-exit,
+	 * halt the vcpu.
 	 */
 	if ((vmcs12->guest_activity_state == GUEST_ACTIVITY_HLT) &&
-	    !(vmcs12->vm_entry_intr_info_field & INTR_INFO_VALID_MASK)) {
+	    !(vmcs12->vm_entry_intr_info_field & INTR_INFO_VALID_MASK) &&
+	    !nested_vmx_nmi_window_exit(vcpu)) {
 		vmx->nested.nested_run_pending = 0;
 		return kvm_vcpu_halt(vcpu);
 	}