[PATCH 01/43] KVM: VMX: Fix handling of #MC that occurs during VM-Entry

[PATCH 01/43] KVM: VMX: Fix handling of #MC that occurs during VM-Entry

[Paolo Bonzini]

From: Sean Christopherson <sean.j.christopherson@intel.com>

A previous fix to prevent KVM from consuming stale VMCS state after a
failed VM-Entry inadvertantly blocked KVM's handling of machine checks
that occur during VM-Entry.

Per Intel's SDM, a #MC during VM-Entry is handled in one of three ways,
depending on when the #MC is recognoized.  As it pertains to this bug
fix, the third case explicitly states EXIT_REASON_MCE_DURING_VMENTRY
is handled like any other VM-Exit during VM-Entry, i.e. sets bit 31 to
indicate the VM-Entry failed.

If a machine-check event occurs during a VM entry, one of the following occurs:
 - The machine-check event is handled as if it occurred before the VM entry:
        ...
 - The machine-check event is handled after VM entry completes:
        ...
 - A VM-entry failure occurs as described in Section 26.7. The basic
   exit reason is 41, for "VM-entry failure due to machine-check event".

Explicitly handle EXIT_REASON_MCE_DURING_VMENTRY as a one-off case in
vmx_vcpu_run() instead of binning it into vmx_complete_atomic_exit().
Doing so allows vmx_vcpu_run() to handle VMX_EXIT_REASONS_FAILED_VMENTRY
in a sane fashion and also simplifies vmx_complete_atomic_exit() since
VMCS.VM_EXIT_INTR_INFO is guaranteed to be fresh.

Fixes: b060ca3b2e9e7 ("kvm: vmx: Handle VMLAUNCH/VMRESUME failure properly")
Cc: Jim Mattson <jmattson@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/vmx/vmx.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 5d903f8909d1..1b3ca0582a0c 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6107,28 +6107,21 @@ static void vmx_apicv_post_state_restore(struct kvm_vcpu *vcpu)
 
 static void vmx_complete_atomic_exit(struct vcpu_vmx *vmx)
 {
-	u32 exit_intr_info = 0;
-	u16 basic_exit_reason = (u16)vmx->exit_reason;
-
-	if (!(basic_exit_reason == EXIT_REASON_MCE_DURING_VMENTRY
-	      || basic_exit_reason == EXIT_REASON_EXCEPTION_NMI))
+	if (vmx->exit_reason != EXIT_REASON_EXCEPTION_NMI)
 		return;
 
-	if (!(vmx->exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY))
-		exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
-	vmx->exit_intr_info = exit_intr_info;
+	vmx->exit_intr_info = vmcs_read32(VM_EXIT_INTR_INFO);
 
 	/* if exit due to PF check for async PF */
-	if (is_page_fault(exit_intr_info))
+	if (is_page_fault(vmx->exit_intr_info))
 		vmx->vcpu.arch.apf.host_apf_reason = kvm_read_and_reset_pf_reason();
 
 	/* Handle machine checks before interrupts are enabled */
-	if (basic_exit_reason == EXIT_REASON_MCE_DURING_VMENTRY ||
-	    is_machine_check(exit_intr_info))
+	if (is_machine_check(vmx->exit_intr_info))
 		kvm_machine_check();
 
 	/* We need to handle NMIs before interrupts are enabled */
-	if (is_nmi(exit_intr_info)) {
+	if (is_nmi(vmx->exit_intr_info)) {
 		kvm_before_interrupt(&vmx->vcpu);
 		asm("int $2");
 		kvm_after_interrupt(&vmx->vcpu);
@@ -6535,6 +6528,9 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
 	vmx->idt_vectoring_info = 0;
 
 	vmx->exit_reason = vmx->fail ? 0xdead : vmcs_read32(VM_EXIT_REASON);
+	if ((u16)vmx->exit_reason == EXIT_REASON_MCE_DURING_VMENTRY)
+		kvm_machine_check();
+
 	if (vmx->fail || (vmx->exit_reason & VMX_EXIT_REASONS_FAILED_VMENTRY))
 		return;
 
-- 
1.8.3.1

[PATCH 15/43] KVM: VMX: Always signal #GP on WRMSR to MSR_IA32_CR_PAT with bad value

[Paolo Bonzini]

From: Sean Christopherson <sean.j.christopherson@intel.com>

The behavior of WRMSR is in no way dependent on whether or not KVM
consumes the value.

Fixes: 4566654bb9be9 ("KVM: vmx: Inject #GP on invalid PAT CR")
Cc: stable@vger.kernel.org
Cc: Nadav Amit <nadav.amit@gmail.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/vmx/vmx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 1a87a91e98dc..091610684d28 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1894,9 +1894,10 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 					      MSR_TYPE_W);
 		break;
 	case MSR_IA32_CR_PAT:
+		if (!kvm_pat_valid(data))
+			return 1;
+
 		if (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_IA32_PAT) {
-			if (!kvm_pat_valid(data))
-				return 1;
 			vmcs_write64(GUEST_IA32_PAT, data);
 			vcpu->arch.pat = data;
 			break;
-- 
1.8.3.1

[PATCH 16/43] KVM: nVMX: Always sync GUEST_BNDCFGS when it comes from vmcs01

[Paolo Bonzini]

From: Sean Christopherson <sean.j.christopherson@intel.com>

If L1 does not set VM_ENTRY_LOAD_BNDCFGS, then L1's BNDCFGS value must
be propagated to vmcs02 since KVM always runs with VM_ENTRY_LOAD_BNDCFGS
when MPX is supported.  Because the value effectively comes from vmcs01,
vmcs02 must be updated even if vmcs12 is clean.

Fixes: 62cf9bd8118c4 ("KVM: nVMX: Fix emulation of VM_ENTRY_LOAD_BNDCFGS")
Cc: stable@vger.kernel.org
Cc: Liran Alon <liran.alon@oracle.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/vmx/nested.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index fb7eddd64714..f2be64256f15 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -2228,13 +2228,9 @@ static void prepare_vmcs02_rare(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12)
 
 	set_cr4_guest_host_mask(vmx);
 
-	if (kvm_mpx_supported()) {
-		if (vmx->nested.nested_run_pending &&
-			(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS))
-			vmcs_write64(GUEST_BNDCFGS, vmcs12->guest_bndcfgs);
-		else
-			vmcs_write64(GUEST_BNDCFGS, vmx->nested.vmcs01_guest_bndcfgs);
-	}
+	if (kvm_mpx_supported() && vmx->nested.nested_run_pending &&
+	    (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS))
+		vmcs_write64(GUEST_BNDCFGS, vmcs12->guest_bndcfgs);
 }
 
 /*
@@ -2266,6 +2262,9 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
 		kvm_set_dr(vcpu, 7, vcpu->arch.dr7);
 		vmcs_write64(GUEST_IA32_DEBUGCTL, vmx->nested.vmcs01_debugctl);
 	}
+	if (kvm_mpx_supported() && (!vmx->nested.nested_run_pending ||
+	    !(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS)))
+		vmcs_write64(GUEST_BNDCFGS, vmx->nested.vmcs01_guest_bndcfgs);
 	vmx_set_rflags(vcpu, vmcs12->guest_rflags);
 
 	/* EXCEPTION_BITMAP and CR0_GUEST_HOST_MASK should basically be the
-- 
1.8.3.1

[PATCH 22/43] KVM: nVMX: Don't dump VMCS if virtual APIC page can't be mapped

[Paolo Bonzini]

From: Sean Christopherson <sean.j.christopherson@intel.com>

... as a malicious userspace can run a toy guest to generate invalid
virtual-APIC page addresses in L1, i.e. flood the kernel log with error
messages.

Fixes: 690908104e39d ("KVM: nVMX: allow tests to use bad virtual-APIC page address")
Cc: stable@vger.kernel.org
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/vmx/nested.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 9478d8947595..0f4cb473bd36 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -2880,9 +2880,6 @@ static void nested_get_vmcs12_pages(struct kvm_vcpu *vcpu)
 			 */
 			vmcs_clear_bits(CPU_BASED_VM_EXEC_CONTROL,
 					CPU_BASED_TPR_SHADOW);
-		} else {
-			printk("bad virtual-APIC page address\n");
-			dump_vmcs();
 		}
 	}
 
-- 
1.8.3.1

Re: [PATCH 01/43] KVM: VMX: Fix handling of #MC that occurs during VM-Entry

[Jim Mattson]

On Thu, Jun 13, 2019 at 10:03 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> From: Sean Christopherson <sean.j.christopherson@intel.com>
>
> A previous fix to prevent KVM from consuming stale VMCS state after a
> failed VM-Entry inadvertantly blocked KVM's handling of machine checks
> that occur during VM-Entry.
>
> Per Intel's SDM, a #MC during VM-Entry is handled in one of three ways,
> depending on when the #MC is recognoized.  As it pertains to this bug
> fix, the third case explicitly states EXIT_REASON_MCE_DURING_VMENTRY
> is handled like any other VM-Exit during VM-Entry, i.e. sets bit 31 to
> indicate the VM-Entry failed.
>
> If a machine-check event occurs during a VM entry, one of the following occurs:
>  - The machine-check event is handled as if it occurred before the VM entry:
>         ...
>  - The machine-check event is handled after VM entry completes:
>         ...
>  - A VM-entry failure occurs as described in Section 26.7. The basic
>    exit reason is 41, for "VM-entry failure due to machine-check event".
>
> Explicitly handle EXIT_REASON_MCE_DURING_VMENTRY as a one-off case in
> vmx_vcpu_run() instead of binning it into vmx_complete_atomic_exit().
> Doing so allows vmx_vcpu_run() to handle VMX_EXIT_REASONS_FAILED_VMENTRY
> in a sane fashion and also simplifies vmx_complete_atomic_exit() since
> VMCS.VM_EXIT_INTR_INFO is guaranteed to be fresh.
>
> Fixes: b060ca3b2e9e7 ("kvm: vmx: Handle VMLAUNCH/VMRESUME failure properly")

I'm never going to live down that subject line, am I? :-)

Reviewed-by: Jim Mattson <jmattson@google.com>

Re: [PATCH 16/43] KVM: nVMX: Always sync GUEST_BNDCFGS when it comes from vmcs01

[Liran Alon]

You should apply something as the following instead of the original fix by Sean
to play nicely on upstream without additional dependency:

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index f1a69117ac0f..3fc44852ed4f 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -2234,12 +2234,9 @@ static void prepare_vmcs02_full(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12)

        set_cr4_guest_host_mask(vmx);

-       if (kvm_mpx_supported()) {
-               if (vmx->nested.nested_run_pending &&
-                       (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS))
-                       vmcs_write64(GUEST_BNDCFGS, vmcs12->guest_bndcfgs);
-               else
-                       vmcs_write64(GUEST_BNDCFGS, vmx->nested.vmcs01_guest_bndcfgs);
+       if (kvm_mpx_supported() && vmx->nested.nested_run_pending &&
+               (vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS)) {
+               vmcs_write64(GUEST_BNDCFGS, vmcs12->guest_bndcfgs);
        }
 }

@@ -2283,6 +2280,10 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
                kvm_set_dr(vcpu, 7, vcpu->arch.dr7);
                vmcs_write64(GUEST_IA32_DEBUGCTL, vmx->nested.vmcs01_debugctl);
        }
+       if (kvm_mpx_supported() && (!vmx->nested.nested_run_pending ||
+               !(vmcs12->vm_entry_controls & VM_ENTRY_LOAD_BNDCFGS))) {
+               vmcs_write64(GUEST_BNDCFGS, vmx->nested.vmcs01_guest_bndcfgs);
+       }
        vmx_set_rflags(vcpu, vmcs12->guest_rflags);

        /* EXCEPTION_BITMAP and CR0_GUEST_HOST_MASK should basically be the

-Liran

> On 16 Jun 2019, at 1:16, Sasha Levin <sashal@kernel.org> wrote:
> 
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: 62cf9bd8118c KVM: nVMX: Fix emulation of VM_ENTRY_LOAD_BNDCFGS.
> 
> The bot has tested the following trees: v5.1.9, v4.19.50.
> 
> v5.1.9: Build OK!
> v4.19.50: Failed to apply! Possible dependencies:
>    09abb5e3e5e5 ("KVM: nVMX: call kvm_skip_emulated_instruction in nested_vmx_{fail,succeed}")
>    09abe3200266 ("KVM: nVMX: split pieces of prepare_vmcs02() to prepare_vmcs02_early()")
>    1438921c6dc1 ("KVM: nVMX: Flush TLB entries tagged by dest EPTP on L1<->L2 transitions")
>    199b118ab3d5 ("KVM: VMX: Alphabetize the includes in vmx.c")
>    1abf23fb42f5 ("KVM: nVMX: use vm_exit_controls_init() to write exit controls for vmcs02")
>    327c072187f7 ("KVM: nVMX: Flush linear and combined mappings on VPID02 related flushes")
>    3d5bdae8b164 ("KVM: nVMX: Use correct VPID02 when emulating L1 INVVPID")
>    3df5c37e55c8 ("KVM: nVMX: try to set EFER bits correctly when initializing controls")
>    55d2375e58a6 ("KVM: nVMX: Move nested code to dedicated files")
>    5b8ba41dafd7 ("KVM: nVMX: move vmcs12 EPTP consistency check to check_vmentry_prereqs()")
>    609363cf81fc ("KVM: nVMX: Move vmcs12 code to dedicated files")
>    75edce8a4548 ("KVM: VMX: Move eVMCS code to dedicated files")
>    7671ce21b13b ("KVM: nVMX: move check_vmentry_postreqs() call to nested_vmx_enter_non_root_mode()")
>    945679e301ea ("KVM: nVMX: add enlightened VMCS state")
>    a633e41e7362 ("KVM: nVMX: assimilate nested_vmx_entry_failure() into nested_vmx_enter_non_root_mode()")
>    a821bab2d1ee ("KVM: VMX: Move VMX specific files to a "vmx" subdirectory")
>    b8bbab928fb1 ("KVM: nVMX: implement enlightened VMPTRLD and VMCLEAR")
>    d63907dc7dd1 ("KVM: nVMX: rename enter_vmx_non_root_mode to nested_vmx_enter_non_root_mode")
>    efebf0aaec3d ("KVM: nVMX: Do not flush TLB on L1<->L2 transitions if L1 uses VPID and EPT")
> 
> 
> How should we proceed with this patch?
> 
> --
> Thanks,
> Sasha

Re: [PATCH 22/43] KVM: nVMX: Don't dump VMCS if virtual APIC page can't be mapped

[Radim Krčmář]

2019-06-13 19:03+0200, Paolo Bonzini:
> From: Sean Christopherson <sean.j.christopherson@intel.com>
> 
> ... as a malicious userspace can run a toy guest to generate invalid
> virtual-APIC page addresses in L1, i.e. flood the kernel log with error
> messages.
> 
> Fixes: 690908104e39d ("KVM: nVMX: allow tests to use bad virtual-APIC page address")
> Cc: stable@vger.kernel.org
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

Makes me wonder why it looks like this in kvm/queue. :)

  commit 1971a835297f9098ce5a735d38916830b8313a65
  Author:     Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
  AuthorDate: Tue May 7 09:06:26 2019 -0700
  Commit:     Paolo Bonzini <pbonzini@redhat.com>
  CommitDate: Thu Jun 13 16:23:13 2019 +0200
  
      KVM: nVMX: Don't dump VMCS if virtual APIC page can't be mapped
      
      ... as a malicious userspace can run a toy guest to generate invalid
      virtual-APIC page addresses in L1, i.e. flood the kernel log with error
      messages.
      
      Fixes: 690908104e39d ("KVM: nVMX: allow tests to use bad virtual-APIC page address")
      Cc: stable@xxxxxxxxxxxxxxx
      Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
      Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
      Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [PATCH 22/43] KVM: nVMX: Don't dump VMCS if virtual APIC page can't be mapped

[Sean Christopherson]

On Mon, Jun 17, 2019 at 09:17:24PM +0200, Radim Krčmář wrote:
> 2019-06-13 19:03+0200, Paolo Bonzini:
> > From: Sean Christopherson <sean.j.christopherson@intel.com>
> > 
> > ... as a malicious userspace can run a toy guest to generate invalid
> > virtual-APIC page addresses in L1, i.e. flood the kernel log with error
> > messages.
> > 
> > Fixes: 690908104e39d ("KVM: nVMX: allow tests to use bad virtual-APIC page address")
> > Cc: stable@vger.kernel.org
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> 
> Makes me wonder why it looks like this in kvm/queue. :)

Presumably something is wonky in Paolo's workflow, this happened before.

commit d69129b4e46a7b61dc956af038d143eb791f22c7
Author: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
Date:   Wed May 8 07:32:15 2019 -0700

    KVM: nVMX: Disable intercept for FS/GS base MSRs in vmcs02 when possible

    If L1 is using an MSR bitmap, unconditionally merge the MSR bitmaps from
    L0 and L1 for MSR_{KERNEL,}_{FS,GS}_BASE.  KVM unconditionally exposes
    MSRs L1.  If KVM is also running in L1 then it's highly likely L1 is
    also exposing the MSRs to L2, i.e. KVM doesn't need to intercept L2
    accesses.

    Based on code from Jintack Lim.

    Cc: Jintack Lim <jintack@xxxxxxxxxxxxxxx>
    Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

> 
>   commit 1971a835297f9098ce5a735d38916830b8313a65
>   Author:     Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
>   AuthorDate: Tue May 7 09:06:26 2019 -0700
>   Commit:     Paolo Bonzini <pbonzini@redhat.com>
>   CommitDate: Thu Jun 13 16:23:13 2019 +0200
>   
>       KVM: nVMX: Don't dump VMCS if virtual APIC page can't be mapped
>       
>       ... as a malicious userspace can run a toy guest to generate invalid
>       virtual-APIC page addresses in L1, i.e. flood the kernel log with error
>       messages.
>       
>       Fixes: 690908104e39d ("KVM: nVMX: allow tests to use bad virtual-APIC page address")
>       Cc: stable@xxxxxxxxxxxxxxx
>       Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>       Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
>       Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [PATCH 22/43] KVM: nVMX: Don't dump VMCS if virtual APIC page can't be mapped

[Paolo Bonzini]

On 17/06/19 22:07, Sean Christopherson wrote:
> On Mon, Jun 17, 2019 at 09:17:24PM +0200, Radim Krčmář wrote:
>> 2019-06-13 19:03+0200, Paolo Bonzini:
>>> From: Sean Christopherson <sean.j.christopherson@intel.com>
>>>
>>> ... as a malicious userspace can run a toy guest to generate invalid
>>> virtual-APIC page addresses in L1, i.e. flood the kernel log with error
>>> messages.
>>>
>>> Fixes: 690908104e39d ("KVM: nVMX: allow tests to use bad virtual-APIC page address")
>>> Cc: stable@vger.kernel.org
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>
>> Makes me wonder why it looks like this in kvm/queue. :)
> 
> Presumably something is wonky in Paolo's workflow, this happened before.

It's more my non-workflow... when I cannot find a patch for some reason
(deleted by mistake, eaten by Gmane, etc.), I search it with Google and
sometimes spinics.net comes up which mangles the domain.  I should just
subscribe to kvm@vger.kernel.org since Gmane has gotten less reliable,
or set up a Patchew instance for it.

Paolo