Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1687190 > unrolled thread

[PATCH] KVM: VMX: Fix losing blocking by NMI in the guest interruptibility-state field

Started byWanpeng Li <kernellwp@gmail.com>
First post2017-07-14 11:40 +0200
Last post2017-07-14 14:30 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] KVM: VMX: Fix losing blocking by NMI in the guest interruptibility-state field Wanpeng Li <kernellwp@gmail.com> - 2017-07-14 11:40 +0200
    Re: [PATCH] KVM: VMX: Fix losing blocking by NMI in the guest  interruptibility-state field Paolo Bonzini <pbonzini@redhat.com> - 2017-07-14 13:40 +0200
      Re: [PATCH] KVM: VMX: Fix losing blocking by NMI in the guest  interruptibility-state field Wanpeng Li <kernellwp@gmail.com> - 2017-07-14 14:30 +0200

#1687190 — [PATCH] KVM: VMX: Fix losing blocking by NMI in the guest interruptibility-state field

FromWanpeng Li <kernellwp@gmail.com>
Date2017-07-14 11:40 +0200
Subject[PATCH] KVM: VMX: Fix losing blocking by NMI in the guest interruptibility-state field
Message-ID<u35eN-1tG-5@gated-at.bofh.it>
From: Wanpeng Li <wanpeng.li@hotmail.com>

Run kvm-unit-tests/eventinj.flat in L1 w/ ept=0 on both L0 and L1:

Before NMI IRET test
Sending NMI to self
NMI isr running stack 0x461000
Sending nested NMI to self
After nested NMI to self
Nested NMI isr running rip=40038e
After iret
After NMI to self
FAIL: NMI

Reference SDM 31.7.1.2:

 If the “virtual NMIs” VM-execution control is 1, bit 12 of the VM-exit 
 interruption-information field indicates that the VM exit was due to a fault 
 encountered during an execution of the IRET instruction that removed virtual-NMI 
 blocking. In particular, it provides this indication if the following are both 
 true:
 
  - Bit 31 (valid) in the IDT-vectoring information field is 0.
  - The value of bits 7:0 (vector) of the VM-exit interruption-information 
    field is not 8 (the VM exit is not due to a double-fault exception).
 
 If both are true and bit 12 of the VM-exit interruption-information field is 1, 
 there was virtual-NMI blocking before guest software executed the IRET instruction 
 that caused the fault that caused the VM exit. The VMM should set bit 3 (blocking 
 by NMI) in the interruptibility-state field (using VMREAD and VMWRITE) before 
 resuming guest software.

However, commit 0be9c7a89f750 (KVM: VMX: set "blocked by NMI" flag if EPT 
violation happens during IRET from NMI) just fixes the fault due to EPT violation. 
This patch tries to fix the fault due to the page fault of shadow page table. 

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 arch/x86/kvm/vmx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 84e62ac..32ca063 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5709,6 +5709,11 @@ static int handle_exception(struct kvm_vcpu *vcpu)
 	}
 
 	if (is_page_fault(intr_info)) {
+
+		if (!(to_vmx(vcpu)->idt_vectoring_info & VECTORING_INFO_VALID_MASK) &&
+			(intr_info & INTR_INFO_UNBLOCK_NMI))
+			vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO, GUEST_INTR_STATE_NMI);
+
 		cr2 = vmcs_readl(EXIT_QUALIFICATION);
 		/* EPT won't cause page fault directly */
 		WARN_ON_ONCE(!vcpu->arch.apf.host_apf_reason && enable_ept);
-- 
2.7.4

[toc] | [next] | [standalone]

#1687271 — Re: [PATCH] KVM: VMX: Fix losing blocking by NMI in the guest interruptibility-state field

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-07-14 13:40 +0200
SubjectRe: [PATCH] KVM: VMX: Fix losing blocking by NMI in the guest interruptibility-state field
Message-ID<u376V-2Kc-3@gated-at.bofh.it>
In reply to#1687190
On 14/07/2017 11:39, Wanpeng Li wrote:
> However, commit 0be9c7a89f750 (KVM: VMX: set "blocked by NMI" flag if EPT 
> violation happens during IRET from NMI) just fixes the fault due to EPT violation. 
> This patch tries to fix the fault due to the page fault of shadow page table. 
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
>  arch/x86/kvm/vmx.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 84e62ac..32ca063 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -5709,6 +5709,11 @@ static int handle_exception(struct kvm_vcpu *vcpu)
>  	}
>  
>  	if (is_page_fault(intr_info)) {
> +
> +		if (!(to_vmx(vcpu)->idt_vectoring_info & VECTORING_INFO_VALID_MASK) &&
> +			(intr_info & INTR_INFO_UNBLOCK_NMI))
> +			vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO, GUEST_INTR_STATE_NMI);
> +
>  		cr2 = vmcs_readl(EXIT_QUALIFICATION);
>  		/* EPT won't cause page fault directly */
>  		WARN_ON_ONCE(!vcpu->arch.apf.host_apf_reason && enable_ept);

vmx_recover_nmi_blocking is supposed to do the same.  EPT and PML-full exits
need separate code because they store bit 12 in the exit qualification rather
than the VM-exit interruption info.  I think the bug is in the handling of
vmx->nmi_known_unmasked. 

The following patch fixes it for me, can you test it too?

Thanks,

Paolo

--------- 8< -------------------
From: Paolo Bonzini <pbonzini@redhat.com
Subject: [PATCH] KVM: nVMX: track NMI blocking state separately for each VMCS

vmx_recover_nmi_blocking is using a cached value of the guest 
interruptibility info, which is stored in vmx->nmi_known_unmasked.
vmx_recover_nmi_blocking is run for both normal and nested guests,
so the cached value must be per-VMCS.

This fixes eventinj.flat in a nested non-EPT environment.  With EPT it 
works, because the EPT violation handler doesn't have the 
vmx->nmi_known_unmasked optimization (it is unnecessary because, unlike 
vmx_recover_nmi_blocking, it can just look at the exit qualification).

Thanks to Wanpeng Li for debugging the testcase and providing an initial
patch.

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


diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 32db3f5dce7f..504df356a10c 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -198,7 +198,8 @@ struct loaded_vmcs {
 	struct vmcs *vmcs;
 	struct vmcs *shadow_vmcs;
 	int cpu;
-	int launched;
+	bool launched;
+	bool nmi_known_unmasked;
 	struct list_head loaded_vmcss_on_cpu_link;
 };
 
@@ -5497,10 +5498,8 @@ static void vmx_inject_nmi(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 
-	if (!is_guest_mode(vcpu)) {
-		++vcpu->stat.nmi_injections;
-		vmx->nmi_known_unmasked = false;
-	}
+	++vcpu->stat.nmi_injections;
+	vmx->loaded_vmcs->nmi_known_unmasked = false;
 
 	if (vmx->rmode.vm86_active) {
 		if (kvm_inject_realmode_interrupt(vcpu, NMI_VECTOR, 0) != EMULATE_DONE)
@@ -5514,16 +5513,21 @@ static void vmx_inject_nmi(struct kvm_vcpu *vcpu)
 
 static bool vmx_get_nmi_mask(struct kvm_vcpu *vcpu)
 {
-	if (to_vmx(vcpu)->nmi_known_unmasked)
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
+	bool masked;
+
+	if (vmx->loaded_vmcs->nmi_known_unmasked)
 		return false;
-	return vmcs_read32(GUEST_INTERRUPTIBILITY_INFO)	& GUEST_INTR_STATE_NMI;
+	masked = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & GUEST_INTR_STATE_NMI;
+	vmx->loaded_vmcs->nmi_known_unmasked = !masked;
+	return masked;
 }
 
 static void vmx_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 
-	vmx->nmi_known_unmasked = !masked;
+	vmx->loaded_vmcs->nmi_known_unmasked = !masked;
 	if (masked)
 		vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO,
 			      GUEST_INTR_STATE_NMI);
@@ -8719,7 +8723,7 @@ static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx)
 
 	idtv_info_valid = vmx->idt_vectoring_info & VECTORING_INFO_VALID_MASK;
 
-	if (vmx->nmi_known_unmasked)
+	if (vmx->loaded_vmcs->nmi_known_unmasked)
 		return;
 	/*
 	 * Can't use vmx->exit_intr_info since we're not sure what
@@ -8743,7 +8747,7 @@ static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx)
 		vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO,
 			      GUEST_INTR_STATE_NMI);
 	else
-		vmx->nmi_known_unmasked =
+		vmx->loaded_vmcs->nmi_known_unmasked =
 			!(vmcs_read32(GUEST_INTERRUPTIBILITY_INFO)
 			  & GUEST_INTR_STATE_NMI);
 }

[toc] | [prev] | [next] | [standalone]

#1687333 — Re: [PATCH] KVM: VMX: Fix losing blocking by NMI in the guest interruptibility-state field

FromWanpeng Li <kernellwp@gmail.com>
Date2017-07-14 14:30 +0200
SubjectRe: [PATCH] KVM: VMX: Fix losing blocking by NMI in the guest interruptibility-state field
Message-ID<u37Tk-3mW-15@gated-at.bofh.it>
In reply to#1687271
2017-07-14 19:36 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
> On 14/07/2017 11:39, Wanpeng Li wrote:
>> However, commit 0be9c7a89f750 (KVM: VMX: set "blocked by NMI" flag if EPT
>> violation happens during IRET from NMI) just fixes the fault due to EPT violation.
>> This patch tries to fix the fault due to the page fault of shadow page table.
>>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Radim Krčmář <rkrcmar@redhat.com>
>> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
>> ---
>>  arch/x86/kvm/vmx.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 84e62ac..32ca063 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -5709,6 +5709,11 @@ static int handle_exception(struct kvm_vcpu *vcpu)
>>       }
>>
>>       if (is_page_fault(intr_info)) {
>> +
>> +             if (!(to_vmx(vcpu)->idt_vectoring_info & VECTORING_INFO_VALID_MASK) &&
>> +                     (intr_info & INTR_INFO_UNBLOCK_NMI))
>> +                     vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO, GUEST_INTR_STATE_NMI);
>> +
>>               cr2 = vmcs_readl(EXIT_QUALIFICATION);
>>               /* EPT won't cause page fault directly */
>>               WARN_ON_ONCE(!vcpu->arch.apf.host_apf_reason && enable_ept);
>
> vmx_recover_nmi_blocking is supposed to do the same.  EPT and PML-full exits
> need separate code because they store bit 12 in the exit qualification rather
> than the VM-exit interruption info.  I think the bug is in the handling of
> vmx->nmi_known_unmasked.
>
> The following patch fixes it for me, can you test it too?
>
> Thanks,
>
> Paolo
>
> --------- 8< -------------------
> From: Paolo Bonzini <pbonzini@redhat.com
> Subject: [PATCH] KVM: nVMX: track NMI blocking state separately for each VMCS
>
> vmx_recover_nmi_blocking is using a cached value of the guest
> interruptibility info, which is stored in vmx->nmi_known_unmasked.
> vmx_recover_nmi_blocking is run for both normal and nested guests,
> so the cached value must be per-VMCS.
>
> This fixes eventinj.flat in a nested non-EPT environment.  With EPT it
> works, because the EPT violation handler doesn't have the
> vmx->nmi_known_unmasked optimization (it is unnecessary because, unlike
> vmx_recover_nmi_blocking, it can just look at the exit qualification).
>
> Thanks to Wanpeng Li for debugging the testcase and providing an initial
> patch.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Looks good to me, thanks for the patch.

Regards,
Wanpeng Li

>
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 32db3f5dce7f..504df356a10c 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -198,7 +198,8 @@ struct loaded_vmcs {
>         struct vmcs *vmcs;
>         struct vmcs *shadow_vmcs;
>         int cpu;
> -       int launched;
> +       bool launched;
> +       bool nmi_known_unmasked;
>         struct list_head loaded_vmcss_on_cpu_link;
>  };
>
> @@ -5497,10 +5498,8 @@ static void vmx_inject_nmi(struct kvm_vcpu *vcpu)
>  {
>         struct vcpu_vmx *vmx = to_vmx(vcpu);
>
> -       if (!is_guest_mode(vcpu)) {
> -               ++vcpu->stat.nmi_injections;
> -               vmx->nmi_known_unmasked = false;
> -       }
> +       ++vcpu->stat.nmi_injections;
> +       vmx->loaded_vmcs->nmi_known_unmasked = false;
>
>         if (vmx->rmode.vm86_active) {
>                 if (kvm_inject_realmode_interrupt(vcpu, NMI_VECTOR, 0) != EMULATE_DONE)
> @@ -5514,16 +5513,21 @@ static void vmx_inject_nmi(struct kvm_vcpu *vcpu)
>
>  static bool vmx_get_nmi_mask(struct kvm_vcpu *vcpu)
>  {
> -       if (to_vmx(vcpu)->nmi_known_unmasked)
> +       struct vcpu_vmx *vmx = to_vmx(vcpu);
> +       bool masked;
> +
> +       if (vmx->loaded_vmcs->nmi_known_unmasked)
>                 return false;
> -       return vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & GUEST_INTR_STATE_NMI;
> +       masked = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & GUEST_INTR_STATE_NMI;
> +       vmx->loaded_vmcs->nmi_known_unmasked = !masked;
> +       return masked;
>  }
>
>  static void vmx_set_nmi_mask(struct kvm_vcpu *vcpu, bool masked)
>  {
>         struct vcpu_vmx *vmx = to_vmx(vcpu);
>
> -       vmx->nmi_known_unmasked = !masked;
> +       vmx->loaded_vmcs->nmi_known_unmasked = !masked;
>         if (masked)
>                 vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO,
>                               GUEST_INTR_STATE_NMI);
> @@ -8719,7 +8723,7 @@ static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx)
>
>         idtv_info_valid = vmx->idt_vectoring_info & VECTORING_INFO_VALID_MASK;
>
> -       if (vmx->nmi_known_unmasked)
> +       if (vmx->loaded_vmcs->nmi_known_unmasked)
>                 return;
>         /*
>          * Can't use vmx->exit_intr_info since we're not sure what
> @@ -8743,7 +8747,7 @@ static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx)
>                 vmcs_set_bits(GUEST_INTERRUPTIBILITY_INFO,
>                               GUEST_INTR_STATE_NMI);
>         else
> -               vmx->nmi_known_unmasked =
> +               vmx->loaded_vmcs->nmi_known_unmasked =
>                         !(vmcs_read32(GUEST_INTERRUPTIBILITY_INFO)
>                           & GUEST_INTR_STATE_NMI);
>  }

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web