kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] KVM: nVMX: Fix the NMI IDT-vectoring handling
@ 2016-09-22  9:55 Wanpeng Li
  2016-09-22  9:58 ` Paolo Bonzini
  2016-09-22 23:12 ` Radim Krčmář
  0 siblings, 2 replies; 3+ messages in thread
From: Wanpeng Li @ 2016-09-22  9:55 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Wanpeng Li, Paolo Bonzini, Radim Krčmář,
	Jan Kiszka, Bandan Das

From: Wanpeng Li <wanpeng.li@hotmail.com>

Run kvm-unit-tests/eventinj.flat in L1:

Sending NMI to self
After NMI to self
FAIL: NMI

This test scenario is to test whether VMM can handle NMI IDT-vectoring info correctly.

At the beginning, L2 writes LAPIC to send a self NMI, the EPT page tables on both L1 
and L0 are empty so:

- The L2 accesses memory can generate EPT violation which can be intercepted by L0.

  The EPT violation vmexit occurred during delivery of this NMI, and the NMI info is 
  recorded in vmcs02's IDT-vectoring info.

- L0 walks L1's EPT12 and L0 sees the mapping is invalid, it injects the EPT violation into L1.

  The vmcs02's IDT-vectoring info is reflected to vmcs12's IDT-vectoring info since 
  it is a nested vmexit. 
  
- L1 receives the EPT violation, then fixes its EPT12.
- L1 executes VMRESUME to resume L2 which generates vmexit and causes L1 exits to L0.
- L0 emulates VMRESUME which is called from L1, then return to L2.

  L0 merges the requirement of vmcs12's IDT-vectoring info and injects it to L2 through 
  vmcs02.

- The L2 re-executes the fault instruction and cause EPT violation again.
- Since the L1's EPT12 is valid, L0 can fix its EPT02
- L0 resume L2
	
  The EPT violation vmexit occurred during delivery of this NMI again, and the NMI info 
  is recorded in vmcs02's IDT-vectoring info. L0 should inject the NMI through vmentry 
  event injection since it is caused by EPT02's EPT violation.

However, vmx_inject_nmi() refuses to inject NMI from IDT-vectoring info if vCPU is in 
guest mode, this patch fix it by permitting to inject NMI from IDT-vectoring if it is 
the L0's responsibility to inject NMI from IDT-vectoring info to L2.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Bandan Das <bsd@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
v1 -> v2: 
 * move the if (vmx->rmode.vm86_active) part out if (!is_guest_mode())

 arch/x86/kvm/vmx.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 813658d..5429a43 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5309,29 +5309,30 @@ static void vmx_inject_nmi(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 
-	if (is_guest_mode(vcpu))
-		return;
+	if (!is_guest_mode(vcpu)) {
+		if (!cpu_has_virtual_nmis()) {
+			/*
+			 * Tracking the NMI-blocked state in software is built upon
+			 * finding the next open IRQ window. This, in turn, depends on
+			 * well-behaving guests: They have to keep IRQs disabled at
+			 * least as long as the NMI handler runs. Otherwise we may
+			 * cause NMI nesting, maybe breaking the guest. But as this is
+			 * highly unlikely, we can live with the residual risk.
+			 */
+			vmx->soft_vnmi_blocked = 1;
+			vmx->vnmi_blocked_time = 0;
+		}
 
-	if (!cpu_has_virtual_nmis()) {
-		/*
-		 * Tracking the NMI-blocked state in software is built upon
-		 * finding the next open IRQ window. This, in turn, depends on
-		 * well-behaving guests: They have to keep IRQs disabled at
-		 * least as long as the NMI handler runs. Otherwise we may
-		 * cause NMI nesting, maybe breaking the guest. But as this is
-		 * highly unlikely, we can live with the residual risk.
-		 */
-		vmx->soft_vnmi_blocked = 1;
-		vmx->vnmi_blocked_time = 0;
+		++vcpu->stat.nmi_injections;
+		vmx->nmi_known_unmasked = false;
 	}
 
-	++vcpu->stat.nmi_injections;
-	vmx->nmi_known_unmasked = false;
 	if (vmx->rmode.vm86_active) {
 		if (kvm_inject_realmode_interrupt(vcpu, NMI_VECTOR, 0) != EMULATE_DONE)
 			kvm_make_request(KVM_REQ_TRIPLE_FAULT, vcpu);
 		return;
 	}
+
 	vmcs_write32(VM_ENTRY_INTR_INFO_FIELD,
 			INTR_TYPE_NMI_INTR | INTR_INFO_VALID_MASK | NMI_VECTOR);
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] KVM: nVMX: Fix the NMI IDT-vectoring handling
  2016-09-22  9:55 [PATCH v2] KVM: nVMX: Fix the NMI IDT-vectoring handling Wanpeng Li
@ 2016-09-22  9:58 ` Paolo Bonzini
  2016-09-22 23:12 ` Radim Krčmář
  1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2016-09-22  9:58 UTC (permalink / raw)
  To: Wanpeng Li, linux-kernel, kvm
  Cc: Wanpeng Li, Radim Krčmář, Jan Kiszka, Bandan Das



On 22/09/2016 11:55, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> Run kvm-unit-tests/eventinj.flat in L1:
> 
> Sending NMI to self
> After NMI to self
> FAIL: NMI
> 
> This test scenario is to test whether VMM can handle NMI IDT-vectoring info correctly.
> 
> At the beginning, L2 writes LAPIC to send a self NMI, the EPT page tables on both L1 
> and L0 are empty so:
> 
> - The L2 accesses memory can generate EPT violation which can be intercepted by L0.
> 
>   The EPT violation vmexit occurred during delivery of this NMI, and the NMI info is 
>   recorded in vmcs02's IDT-vectoring info.
> 
> - L0 walks L1's EPT12 and L0 sees the mapping is invalid, it injects the EPT violation into L1.
> 
>   The vmcs02's IDT-vectoring info is reflected to vmcs12's IDT-vectoring info since 
>   it is a nested vmexit. 
>   
> - L1 receives the EPT violation, then fixes its EPT12.
> - L1 executes VMRESUME to resume L2 which generates vmexit and causes L1 exits to L0.
> - L0 emulates VMRESUME which is called from L1, then return to L2.
> 
>   L0 merges the requirement of vmcs12's IDT-vectoring info and injects it to L2 through 
>   vmcs02.
> 
> - The L2 re-executes the fault instruction and cause EPT violation again.
> - Since the L1's EPT12 is valid, L0 can fix its EPT02
> - L0 resume L2
> 	
>   The EPT violation vmexit occurred during delivery of this NMI again, and the NMI info 
>   is recorded in vmcs02's IDT-vectoring info. L0 should inject the NMI through vmentry 
>   event injection since it is caused by EPT02's EPT violation.
> 
> However, vmx_inject_nmi() refuses to inject NMI from IDT-vectoring info if vCPU is in 
> guest mode, this patch fix it by permitting to inject NMI from IDT-vectoring if it is 
> the L0's responsibility to inject NMI from IDT-vectoring info to L2.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Bandan Das <bsd@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
> v1 -> v2: 
>  * move the if (vmx->rmode.vm86_active) part out if (!is_guest_mode())

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

> 
>  arch/x86/kvm/vmx.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 813658d..5429a43 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -5309,29 +5309,30 @@ static void vmx_inject_nmi(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_vmx *vmx = to_vmx(vcpu);
>  
> -	if (is_guest_mode(vcpu))
> -		return;
> +	if (!is_guest_mode(vcpu)) {
> +		if (!cpu_has_virtual_nmis()) {
> +			/*
> +			 * Tracking the NMI-blocked state in software is built upon
> +			 * finding the next open IRQ window. This, in turn, depends on
> +			 * well-behaving guests: They have to keep IRQs disabled at
> +			 * least as long as the NMI handler runs. Otherwise we may
> +			 * cause NMI nesting, maybe breaking the guest. But as this is
> +			 * highly unlikely, we can live with the residual risk.
> +			 */
> +			vmx->soft_vnmi_blocked = 1;
> +			vmx->vnmi_blocked_time = 0;
> +		}
>  
> -	if (!cpu_has_virtual_nmis()) {
> -		/*
> -		 * Tracking the NMI-blocked state in software is built upon
> -		 * finding the next open IRQ window. This, in turn, depends on
> -		 * well-behaving guests: They have to keep IRQs disabled at
> -		 * least as long as the NMI handler runs. Otherwise we may
> -		 * cause NMI nesting, maybe breaking the guest. But as this is
> -		 * highly unlikely, we can live with the residual risk.
> -		 */
> -		vmx->soft_vnmi_blocked = 1;
> -		vmx->vnmi_blocked_time = 0;
> +		++vcpu->stat.nmi_injections;
> +		vmx->nmi_known_unmasked = false;
>  	}
>  
> -	++vcpu->stat.nmi_injections;
> -	vmx->nmi_known_unmasked = false;
>  	if (vmx->rmode.vm86_active) {
>  		if (kvm_inject_realmode_interrupt(vcpu, NMI_VECTOR, 0) != EMULATE_DONE)
>  			kvm_make_request(KVM_REQ_TRIPLE_FAULT, vcpu);
>  		return;
>  	}
> +
>  	vmcs_write32(VM_ENTRY_INTR_INFO_FIELD,
>  			INTR_TYPE_NMI_INTR | INTR_INFO_VALID_MASK | NMI_VECTOR);
>  }
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] KVM: nVMX: Fix the NMI IDT-vectoring handling
  2016-09-22  9:55 [PATCH v2] KVM: nVMX: Fix the NMI IDT-vectoring handling Wanpeng Li
  2016-09-22  9:58 ` Paolo Bonzini
@ 2016-09-22 23:12 ` Radim Krčmář
  1 sibling, 0 replies; 3+ messages in thread
From: Radim Krčmář @ 2016-09-22 23:12 UTC (permalink / raw)
  To: Wanpeng Li
  Cc: linux-kernel, kvm, Wanpeng Li, Paolo Bonzini, Jan Kiszka,
	Bandan Das

2016-09-22 17:55+0800, Wanpeng Li:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> Run kvm-unit-tests/eventinj.flat in L1:
> 
> Sending NMI to self
> After NMI to self
> FAIL: NMI
> 
> This test scenario is to test whether VMM can handle NMI IDT-vectoring info correctly.
> 
> At the beginning, L2 writes LAPIC to send a self NMI, the EPT page tables on both L1 
> and L0 are empty so:
> 
> - The L2 accesses memory can generate EPT violation which can be intercepted by L0.
> 
>   The EPT violation vmexit occurred during delivery of this NMI, and the NMI info is 
>   recorded in vmcs02's IDT-vectoring info.
> 
> - L0 walks L1's EPT12 and L0 sees the mapping is invalid, it injects the EPT violation into L1.
> 
>   The vmcs02's IDT-vectoring info is reflected to vmcs12's IDT-vectoring info since 
>   it is a nested vmexit. 
>   
> - L1 receives the EPT violation, then fixes its EPT12.
> - L1 executes VMRESUME to resume L2 which generates vmexit and causes L1 exits to L0.
> - L0 emulates VMRESUME which is called from L1, then return to L2.
> 
>   L0 merges the requirement of vmcs12's IDT-vectoring info and injects it to L2 through 
>   vmcs02.
> 
> - The L2 re-executes the fault instruction and cause EPT violation again.
> - Since the L1's EPT12 is valid, L0 can fix its EPT02
> - L0 resume L2
> 	
>   The EPT violation vmexit occurred during delivery of this NMI again, and the NMI info 
>   is recorded in vmcs02's IDT-vectoring info. L0 should inject the NMI through vmentry 
>   event injection since it is caused by EPT02's EPT violation.
> 
> However, vmx_inject_nmi() refuses to inject NMI from IDT-vectoring info if vCPU is in 
> guest mode, this patch fix it by permitting to inject NMI from IDT-vectoring if it is 
> the L0's responsibility to inject NMI from IDT-vectoring info to L2.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Bandan Das <bsd@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---

Applied to kvm/queue, thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-09-22 23:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-22  9:55 [PATCH v2] KVM: nVMX: Fix the NMI IDT-vectoring handling Wanpeng Li
2016-09-22  9:58 ` Paolo Bonzini
2016-09-22 23:12 ` Radim Krčmář

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).