Why do we need KVM_REQ_GET_NESTED_STATE_PAGES after all

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I would like to raise a question about this elephant in the room which I wanted to understand for 
quite a long time.
 
For my nested AVIC work I once again need to change the KVM_REQ_GET_NESTED_STATE_PAGES code and once
again I am asking myself, maybe we can get rid of this code, after all?
  
And of course if we don't need it, getting rid of it would be a very happy day in my life 
(and likely in the life of all other KVM developers as well).
 
Needless to say that it already caused few CVE worthy issues which thankfully we patched before
it got to production, and on top of that, having different code flow for a very rare code path 
because nested migration only happens if you are lucky enough to have nested guest 
actually running during migration, which happens only once in a while, even when L1 is fully loaded. 
 
In my testing I always disable HLT exits to actually ensure that nested guest runs all the time,
sans vmexits.
 
====================================================================================================
 
So first of all what KVM_REQ_GET_NESTED_STATE_PAGES is:
 
KVM_REQ_GET_NESTED_STATE_PAGES exists to delay reading/writing/pinning of the guest memory, which is 
pointed by various fields of current vmcs/vmcb to next KVM_RUN, under an assumption that it is not 
safe to do in case when we do a non natural VM entry (which is either due to resume of nested
guest that was interrupted by SMM or when we set the nested state after migration.
 
The alleged reason for that is that either kvm's mmu is not in 'right state', or that userspace
VMM will do some modifications to the VM after the entry and before we actually enter the nested guest.
 
====================================================================================================
 
There are two cases where the KVM_REQ_GET_NESTED_STATE_PAGES is used:
 
1. Exit from SMM, back to an interrupted nested guest:
 
Note that on SVM we actually read the VMCB from the guest memory and HSAVE area,
already violating the rule of not reading guest memory.
On VMX at least we use the cached VMCS.
 
 
2. Loading the nested state.
 
In this case indeed both vmcb and vmcs come from the migration stream, thus we don't touch the 
guest memory when setting the nested state.
 
====================================================================================================
 
Now let see how the guest memory would be accessed on nested VM entry if we didn't use
KVM_REQ_GET_NESTED_STATE_PAGES:
 
First of all, AFAIK all SVM/VMX memory structures including the VMCB/VMCS itself are accessed 
by their physical address, thus we don't need to worry about guest's paging, and 
neither about nested NPT/EPT paging.
 
Shadow pages that MMU created are not relevant also because they are not used by KVM itself to
read guest's memory.
 
To access guest physical memory from KVM the following is done:
 
A. a set of memslots is chosen (calling __kvm_memslots())
 
   KVM keeps two sets of memory slots, one for 'Normal' address space and one for SMM address space.
 
   These memslots are setup by VMM, which usually updates it when the guest modifies
   virtual chipset's SMM related settings.
 
   Note that on standard Q35/i440fx chipsets which qemu emulates, those settings themselves are
   accessible through pci config space regardless of SMM, but have a lock bit which prevents non 
   SMM code to change them after VM's BIOS setup and locked them..
 
   Lots of places in the KVM hardcoded to use the 'Normal' memslots 
   (everyone who uses kvm_memslots for example).
   Others use the kvm_vcpu_memslots, which chooses memslots based on 'arch.hflags & HF_SMM_MASK'
 
   Thankfully we don't allow VMX/SVM in SMM (and I really hope that we never will), thus all the guest 
   memory structures including VMCB/VMCS would reside in the main guest memory and thus can be
   reached through 'Normal' memslots.
 
   From this we can deduce that loading of the nested state will never happen when the 
   VCPU is in SMM mode.
   KVM verifies this and denies setting the nested state if that is attempted.
 
   On returning from SMM to a nested guest, kvm_smm_changed is the first thing to be called,
   prior to calling vendor code which resumes the nested guest.
   kvm_smm_changed clears the HF_SMM_MASK flag, which selects Which memslots to take when doing 
   guest physical to host virtual translation.
 
   Based on this reasoning, we can say that nested guest entries we will never access the 
   SMM memslots, even if we attempt to access the guest memory during the setting of the nested state.
 
 
B. From the 'Normal' memslots, the host virtual address of the page containing the guest physical 
   memory is obtained.
   Page itself might be swapped out or not even present at all if post-copy migration is used.
 
   AFAIK, I don't see a reason why QEMU or any other KVM user would not upload the correct memory slot
   set, after a nested migration.
 
   In fact qemu rarely modifies the memory slots. It does have some memslots for devices which
   Have ram like memory but if the guest attempts to place there VM related structure, it is welcome
   to keep both pieces.
 
   Actual RAM memslots should only be modified during runtime when RAM hotplug happens or so,
   which even if happen after nested migration, would not be an issue as the fact that they 
   Happen after a migration means that guest VM structures couldn’t be in these memslots.
 
   Qemu resets and loads the state for all the devices, and they match the original devices on the 
   migration source, and seems to upload the nested state last.
   It might not migrate all ram, but rather register it with userfaultd, but that isn't an
   issue (see below)
 
   For returns from the SMM, the chances that this memslot map is not up to date are even lower.
   In fact, returning from smm usually won't even cause a userspace vmexit to let qemu mess with
   this map.
 
 
3. Once the host’s virtual address of the guest's page obtained, stock kernel paging 
   facilities are used to obtain the page (swap-in, userfaultd, etc) and
    then obtain its physical address.
 
   Once again on return from SMM, there should be no issues.
 
   After nested migration page might not be present but that will lead to either swapping it in,
   or asking qemu via userfaultd to bring it via userfaultd interface while doing post-copy migration.
   
   In regard to post-copy migration, other VMMs ought to work the same (use userfaultd).
   In theory a VMM could instead handle SIGSEGV, as an indication of a page fault but that would
   not really work due to many reasons.
   
   
So, having said all that, the only justification for KVM_REQ_GET_NESTED_STATE_PAGES
would be an VMM which first sets the CPU nested state and then setups the VMAs for guest memory
and uploads them to KVM via memslots.
 
Since nested migration is something relatively new, there is good chance that nobody does this,
and then requiring that guest memory is present before setting a nested state (or at least memory
which is pointed by the VMX/SVM structs) seems like a reasonable requirement.
 
On top of that, that simplifies the API usage by not delaying the error to the next VM run,
if truly there is an issue with this guest memory pointed by the nested state.
 
 
PS: Few technical notes:
 
====================================================================================================
A. Note on eVMCS usage of KVM_REQ_GET_NESTED_STATE_PAGES:
====================================================================================================
 
When eVMCS which is a guest memory page, representing a VM, similar to VMCB is used 
(this is HV specific PV feature), VMCS fields are loaded from it.
 
Thankfully after nested migration or return from SMM, we already have up to date vmcs12 either read
from the migration stream or saved in kernel memory.
 
We only need the guest physical address of this memory page, to map and pin it, so that later on,
after the nested entry/exit we could read/write it.
 
It address is (indirectly) obtained from msr HV_X64_MSR_VP_ASSIST_PAGE, which qemu restores after
it sets the nested state.
 
Currently to support this, setting nested state with eVMCS is allowed without this msr set,
and later KVM_REQ_GET_NESTED_STATE_PAGES actually maps the eVMCS page.
 
There is also a workaround that was relatively recently added to map eVMCS also when
KVM_REQ_GET_NESTED_STATE_PAGES haven’t got called after nested migration which can happen if we
have nested VM exit prior to entering the guest even once after setting the nested state.
Workaround was to also map eVMCS on nested VM exit.
 
IMHO the right way to fix this, assuming that we drop KVM_REQ_GET_NESTED_STATE_PAGES is to just
map the eVMCS when the HV_X64_MSR_VP_ASSIST_PAGE is set by qemu (host write) after nested state with
active eVMCS was set (that is we are nested but with vmptr == -1 )
 

====================================================================================================
B: Some digital archaeology for reference:
====================================================================================================

First of all we have those two commits:
 
commit a7d5c7ce41ac1e2537d78ddb57ef0ac4f737aa19
Author: Paolo Bonzini <pbonzini@xxxxxxxxxx>
Date:   Tue Sep 22 07:43:14 2020 -0400
 
	KVM: nSVM: delay MSR permission processing to first nested VM run
    
	Allow userspace to set up the memory map after KVM_SET_NESTED_STATE;
	to do so, move the call to nested_svm_vmrun_msrpm inside the
	KVM_REQ_GET_NESTED_STATE_PAGES handler (which is currently
	not used by nSVM).  This is similar to what VMX does already.
    
	Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
 
commit 729c15c20f1a7c9ad1d09a603ad1aa7fb60b1f88
Author: Paolo Bonzini <pbonzini@xxxxxxxxxx>
Date:   Tue Sep 22 06:53:57 2020 -0400
 
	KVM: x86: rename KVM_REQ_GET_VMCS12_PAGES
    
	We are going to use it for SVM too, so use a more generic name.
	Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
	

These two commits were added around the same time I  started fixing the SVM's nested migration
which was just implemented and didn't yet receive any real-world testing.

SVM's code used to not even read the nested msr bitmap which I fixed, and there is a good change
that the above commit was added just in case to make SVM's code do the same thing as VMX does.
	

If we go deeper, we get this commit, from which it all started:
 
 
commit 7f7f1ba33cf2c21d001821313088c231db42ff40
Author: Paolo Bonzini <pbonzini@xxxxxxxxxx>
Date:   Wed Jul 18 18:49:01 2018 +0200
 
	KVM: x86: do not load vmcs12 pages while still in SMM
    
	If the vCPU enters system management mode while running a nested guest,
	RSM starts processing the vmentry while still in SMM.  In that case,
	however, the pages pointed to by the vmcs12 might be incorrectly
	loaded from SMRAM.  To avoid this, delay the handling of the pages
	until just before the next vmentry.  This is done with a new request
	and a new entry in kvm_x86_ops, which we will be able to reuse for
	nested VMX state migration.
    
	Extracted from a patch by Jim Mattson and KarimAllah Ahmed.
    
	Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
 

That commit was part of initial patch series which implemented the nested migration,
and it could be very well that the above commit was just a precation.
https://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/msg1740085.html


Looking back at the old code before this commit, I wonder if that was needed even back then:
 
In the original code just prior to the this commit we had this weird code:
 
                vcpu->arch.hflags &= ~HF_SMM_MASK;
                ret = enter_vmx_non_root_mode(vcpu, NULL);
                vcpu->arch.hflags |= HF_SMM_MASK; <- why do we set it back 
 
 
Yet, it did clear the HF_SMM_MASK just prior to nested entry,
thus correct memslots should be accessed even back then, and I don't think that MSR bitmap would
be loaded from SMM memory.


====================================================================================================
C: POC
====================================================================================================

I attached the patch which removes the KVM_REQ_GET_NESTED_STATE_PAGES which I lightly tested.
I didn't test the eVMCS side of things. Works for me on SVM and VMX.

I also added debug prints to qemu and used virtio-mem which was suspected to set memslots
after setting nested state. It seems that it really doesn't.



Best regards,
   Maxim Levitsky

From 129302dceb659c9d561676faaf33cb3ec64d02c5 Mon Sep 17 00:00:00 2001
From: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
Date: Wed, 26 Jan 2022 17:20:18 +0200
Subject: [PATCH] KVM: x86: get rid of KVM_REQ_GET_NESTED_STATE_PAGES

TODO: add commit description

Signed-off-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
---
 arch/x86/include/asm/kvm_host.h |  5 +-
 arch/x86/kvm/hyperv.c           |  4 ++
 arch/x86/kvm/svm/nested.c       | 51 ++++-------------
 arch/x86/kvm/svm/svm.c          |  2 +-
 arch/x86/kvm/svm/svm.h          |  3 +-
 arch/x86/kvm/vmx/nested.c       | 99 +++++++++------------------------
 arch/x86/kvm/x86.c              |  6 --
 7 files changed, 46 insertions(+), 124 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 82bc3e3e9b935..5a58835b5af0d 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -92,7 +92,6 @@
 #define KVM_REQ_HV_EXIT			KVM_ARCH_REQ(21)
 #define KVM_REQ_HV_STIMER		KVM_ARCH_REQ(22)
 #define KVM_REQ_LOAD_EOI_EXITMAP	KVM_ARCH_REQ(23)
-#define KVM_REQ_GET_NESTED_STATE_PAGES	KVM_ARCH_REQ(24)
 #define KVM_REQ_APICV_UPDATE \
 	KVM_ARCH_REQ_FLAGS(25, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
 #define KVM_REQ_TLB_FLUSH_CURRENT	KVM_ARCH_REQ(26)
@@ -1502,12 +1501,14 @@ struct kvm_x86_nested_ops {
 	int (*set_state)(struct kvm_vcpu *vcpu,
 			 struct kvm_nested_state __user *user_kvm_nested_state,
 			 struct kvm_nested_state *kvm_state);
-	bool (*get_nested_state_pages)(struct kvm_vcpu *vcpu);
 	int (*write_log_dirty)(struct kvm_vcpu *vcpu, gpa_t l2_gpa);
 
 	int (*enable_evmcs)(struct kvm_vcpu *vcpu,
 			    uint16_t *vmcs_version);
 	uint16_t (*get_evmcs_version)(struct kvm_vcpu *vcpu);
+
+	bool (*get_evmcs_page)(struct kvm_vcpu *vcpu);
+
 };
 
 struct kvm_x86_init_ops {
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 6e38a7d22e97a..c15886efa9a66 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1494,6 +1494,10 @@ static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
 					    gfn_to_gpa(gfn) | KVM_MSR_ENABLED,
 					    sizeof(struct hv_vp_assist_page)))
 			return 1;
+
+		if (host && kvm_x86_ops.nested_ops->get_evmcs_page)
+			if (kvm_x86_ops.nested_ops->get_evmcs_page(vcpu))
+				return 1;
 		break;
 	}
 	case HV_X64_MSR_EOI:
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index edd6cf1344112..66fa2478d89f6 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -677,7 +677,7 @@ static void nested_svm_copy_common_state(struct vmcb *from_vmcb, struct vmcb *to
 }
 
 int enter_svm_guest_mode(struct kvm_vcpu *vcpu, u64 vmcb12_gpa,
-			 struct vmcb *vmcb12, bool from_vmrun)
+			 struct vmcb *vmcb12)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 	int ret;
@@ -707,15 +707,13 @@ int enter_svm_guest_mode(struct kvm_vcpu *vcpu, u64 vmcb12_gpa,
 	nested_vmcb02_prepare_save(svm, vmcb12);
 
 	ret = nested_svm_load_cr3(&svm->vcpu, svm->nested.save.cr3,
-				  nested_npt_enabled(svm), from_vmrun);
+				  nested_npt_enabled(svm), true);
 	if (ret)
 		return ret;
 
 	if (!npt_enabled)
 		vcpu->arch.mmu->inject_page_fault = svm_inject_page_fault_nested;
 
-	if (!from_vmrun)
-		kvm_make_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
 
 	svm_set_gif(svm, true);
 
@@ -783,7 +781,7 @@ int nested_svm_vmrun(struct kvm_vcpu *vcpu)
 
 	svm->nested.nested_run_pending = 1;
 
-	if (enter_svm_guest_mode(vcpu, vmcb12_gpa, vmcb12, true))
+	if (enter_svm_guest_mode(vcpu, vmcb12_gpa, vmcb12))
 		goto out_exit_err;
 
 	if (nested_svm_vmrun_msrpm(svm))
@@ -867,8 +865,6 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
 	svm->nested.vmcb12_gpa = 0;
 	WARN_ON_ONCE(svm->nested.nested_run_pending);
 
-	kvm_clear_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
-
 	/* in case we halted in L2 */
 	svm->vcpu.arch.mp_state = KVM_MP_STATE_RUNNABLE;
 
@@ -1066,8 +1062,6 @@ void svm_leave_nested(struct vcpu_svm *svm)
 		nested_svm_uninit_mmu_context(vcpu);
 		vmcb_mark_all_dirty(svm->vmcb);
 	}
-
-	kvm_clear_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
 }
 
 static int nested_svm_exit_handled_msr(struct vcpu_svm *svm)
@@ -1541,11 +1535,11 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 	 */
 
 	ret = nested_svm_load_cr3(&svm->vcpu, vcpu->arch.cr3,
-				  nested_npt_enabled(svm), false);
+				  nested_npt_enabled(svm),
+				  !vcpu->arch.pdptrs_from_userspace);
 	if (WARN_ON_ONCE(ret))
 		goto out_free;
 
-
 	/*
 	 * All checks done, we can enter guest mode. Userspace provides
 	 * vmcb12.control, which will be combined with L1 and stored into
@@ -1570,32 +1564,6 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 
 	svm_switch_vmcb(svm, &svm->nested.vmcb02);
 	nested_vmcb02_prepare_control(svm);
-	kvm_make_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
-
-	ret = 0;
-out_free:
-	kfree(save);
-	kfree(ctl);
-
-	return ret;
-}
-
-static bool svm_get_nested_state_pages(struct kvm_vcpu *vcpu)
-{
-	struct vcpu_svm *svm = to_svm(vcpu);
-
-	if (WARN_ON(!is_guest_mode(vcpu)))
-		return true;
-
-	if (!vcpu->arch.pdptrs_from_userspace &&
-	    !nested_npt_enabled(svm) && is_pae_paging(vcpu))
-		/*
-		 * Reload the guest's PDPTRs since after a migration
-		 * the guest CR3 might be restored prior to setting the nested
-		 * state which can lead to a load of wrong PDPTRs.
-		 */
-		if (CC(!load_pdptrs(vcpu, vcpu->arch.cr3)))
-			return false;
 
 	if (!nested_svm_vmrun_msrpm(svm)) {
 		vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
@@ -1605,13 +1573,18 @@ static bool svm_get_nested_state_pages(struct kvm_vcpu *vcpu)
 		return false;
 	}
 
-	return true;
+	ret = 0;
+out_free:
+	kfree(save);
+	kfree(ctl);
+
+	return ret;
 }
 
+
 struct kvm_x86_nested_ops svm_nested_ops = {
 	.check_events = svm_check_nested_events,
 	.triple_fault = nested_svm_triple_fault,
-	.get_nested_state_pages = svm_get_nested_state_pages,
 	.get_state = svm_get_nested_state,
 	.set_state = svm_set_nested_state,
 };
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 0e6ad19d205c9..00a9396f726b9 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4334,7 +4334,7 @@ static int svm_leave_smm(struct kvm_vcpu *vcpu, const char *smstate)
 	vmcb12 = map.hva;
 	nested_copy_vmcb_control_to_cache(svm, &vmcb12->control);
 	nested_copy_vmcb_save_to_cache(svm, &vmcb12->save);
-	ret = enter_svm_guest_mode(vcpu, vmcb12_gpa, vmcb12, false);
+	ret = enter_svm_guest_mode(vcpu, vmcb12_gpa, vmcb12);
 
 	if (ret)
 		goto unmap_save;
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 3acef0dfc9b94..5979f41632756 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -552,8 +552,7 @@ static inline bool nested_exit_on_nmi(struct vcpu_svm *svm)
 	return vmcb12_is_intercept(&svm->nested.ctl, INTERCEPT_NMI);
 }
 
-int enter_svm_guest_mode(struct kvm_vcpu *vcpu,
-			 u64 vmcb_gpa, struct vmcb *vmcb12, bool from_vmrun);
+int enter_svm_guest_mode(struct kvm_vcpu *vcpu, u64 vmcb_gpa, struct vmcb *vmcb12);
 void svm_leave_nested(struct vcpu_svm *svm);
 void svm_free_nested(struct vcpu_svm *svm);
 int svm_allocate_nested(struct vcpu_svm *svm);
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 5a2595a6bf08c..cb259b58a9110 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -293,8 +293,6 @@ static void free_nested(struct kvm_vcpu *vcpu)
 	if (!vmx->nested.vmxon && !vmx->nested.smm.vmxon)
 		return;
 
-	kvm_clear_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
-
 	vmx->nested.vmxon = false;
 	vmx->nested.smm.vmxon = false;
 	vmx->nested.vmxon_ptr = INVALID_GPA;
@@ -2592,7 +2590,8 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
 
 	/* Shadow page tables on either EPT or shadow page tables. */
 	if (nested_vmx_load_cr3(vcpu, vmcs12->guest_cr3, nested_cpu_has_ept(vmcs12),
-				from_vmentry, entry_failure_code))
+				from_vmentry || !vcpu->arch.pdptrs_from_userspace,
+				entry_failure_code))
 		return -EINVAL;
 
 	/*
@@ -3124,7 +3123,7 @@ static int nested_vmx_check_vmentry_hw(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-static bool nested_get_evmcs_page(struct kvm_vcpu *vcpu)
+bool nested_get_evmcs_page(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 
@@ -3160,18 +3159,6 @@ static bool nested_get_vmcs12_pages(struct kvm_vcpu *vcpu)
 	struct page *page;
 	u64 hpa;
 
-	if (!vcpu->arch.pdptrs_from_userspace &&
-	    !nested_cpu_has_ept(vmcs12) && is_pae_paging(vcpu)) {
-		/*
-		 * Reload the guest's PDPTRs since after a migration
-		 * the guest CR3 might be restored prior to setting the nested
-		 * state which can lead to a load of wrong PDPTRs.
-		 */
-		if (CC(!load_pdptrs(vcpu, vcpu->arch.cr3)))
-			return false;
-	}
-
-
 	if (nested_cpu_has2(vmcs12, SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)) {
 		/*
 		 * Translate L1 physical address to host physical
@@ -3253,25 +3240,6 @@ static bool nested_get_vmcs12_pages(struct kvm_vcpu *vcpu)
 	return true;
 }
 
-static bool vmx_get_nested_state_pages(struct kvm_vcpu *vcpu)
-{
-	if (!nested_get_evmcs_page(vcpu)) {
-		pr_debug_ratelimited("%s: enlightened vmptrld failed\n",
-				     __func__);
-		vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
-		vcpu->run->internal.suberror =
-			KVM_INTERNAL_ERROR_EMULATION;
-		vcpu->run->internal.ndata = 0;
-
-		return false;
-	}
-
-	if (is_guest_mode(vcpu) && !nested_get_vmcs12_pages(vcpu))
-		return false;
-
-	return true;
-}
-
 static int nested_vmx_write_pml_buffer(struct kvm_vcpu *vcpu, gpa_t gpa)
 {
 	struct vmcs12 *vmcs12;
@@ -3401,12 +3369,12 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
 
 	prepare_vmcs02_early(vmx, &vmx->vmcs01, vmcs12);
 
-	if (from_vmentry) {
-		if (unlikely(!nested_get_vmcs12_pages(vcpu))) {
-			vmx_switch_vmcs(vcpu, &vmx->vmcs01);
-			return NVMX_VMENTRY_KVM_INTERNAL_ERROR;
-		}
+	if (unlikely(!nested_get_vmcs12_pages(vcpu))) {
+		vmx_switch_vmcs(vcpu, &vmx->vmcs01);
+		return NVMX_VMENTRY_KVM_INTERNAL_ERROR;
+	}
 
+	if (from_vmentry) {
 		if (nested_vmx_check_vmentry_hw(vcpu)) {
 			vmx_switch_vmcs(vcpu, &vmx->vmcs01);
 			return NVMX_VMENTRY_VMFAIL;
@@ -3428,24 +3396,14 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
 		goto vmentry_fail_vmexit_guest_mode;
 	}
 
-	if (from_vmentry) {
-		failed_index = nested_vmx_load_msr(vcpu,
-						   vmcs12->vm_entry_msr_load_addr,
-						   vmcs12->vm_entry_msr_load_count);
-		if (failed_index) {
-			exit_reason.basic = EXIT_REASON_MSR_LOAD_FAIL;
-			vmcs12->exit_qualification = failed_index;
-			goto vmentry_fail_vmexit_guest_mode;
-		}
-	} else {
-		/*
-		 * The MMU is not initialized to point at the right entities yet and
-		 * "get pages" would need to read data from the guest (i.e. we will
-		 * need to perform gpa to hpa translation). Request a call
-		 * to nested_get_vmcs12_pages before the next VM-entry.  The MSRs
-		 * have already been set at vmentry time and should not be reset.
-		 */
-		kvm_make_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
+
+	failed_index = nested_vmx_load_msr(vcpu,
+					   vmcs12->vm_entry_msr_load_addr,
+					   vmcs12->vm_entry_msr_load_count);
+	if (failed_index) {
+		exit_reason.basic = EXIT_REASON_MSR_LOAD_FAIL;
+		vmcs12->exit_qualification = failed_index;
+		goto vmentry_fail_vmexit_guest_mode;
 	}
 
 	/*
@@ -4515,16 +4473,6 @@ void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason,
 	/* Similarly, triple faults in L2 should never escape. */
 	WARN_ON_ONCE(kvm_check_request(KVM_REQ_TRIPLE_FAULT, vcpu));
 
-	if (kvm_check_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu)) {
-		/*
-		 * KVM_REQ_GET_NESTED_STATE_PAGES is also used to map
-		 * Enlightened VMCS after migration and we still need to
-		 * do that when something is forcing L2->L1 exit prior to
-		 * the first L2 run.
-		 */
-		(void)nested_get_evmcs_page(vcpu);
-	}
-
 	/* Service pending TLB flush requests for L2 before switching to L1. */
 	kvm_service_local_tlb_flush_requests(vcpu);
 
@@ -6348,14 +6296,17 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu,
 
 		set_current_vmptr(vmx, kvm_state->hdr.vmx.vmcs12_pa);
 	} else if (kvm_state->flags & KVM_STATE_NESTED_EVMCS) {
+
+		vmx->nested.hv_evmcs_vmptr = EVMPTR_MAP_PENDING;
+
 		/*
 		 * nested_vmx_handle_enlightened_vmptrld() cannot be called
-		 * directly from here as HV_X64_MSR_VP_ASSIST_PAGE may not be
-		 * restored yet. EVMCS will be mapped from
-		 * nested_get_vmcs12_pages().
+		 * directly from here if HV_X64_MSR_VP_ASSIST_PAGE is not
+		 * restored yet. EVMCS will be mapped when it is.
 		 */
-		vmx->nested.hv_evmcs_vmptr = EVMPTR_MAP_PENDING;
-		kvm_make_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
+		if (kvm_hv_assist_page_enabled(vcpu))
+			nested_get_evmcs_page(vcpu);
+
 	} else {
 		return -EINVAL;
 	}
@@ -6778,8 +6729,8 @@ struct kvm_x86_nested_ops vmx_nested_ops = {
 	.triple_fault = nested_vmx_triple_fault,
 	.get_state = vmx_get_nested_state,
 	.set_state = vmx_set_nested_state,
-	.get_nested_state_pages = vmx_get_nested_state_pages,
 	.write_log_dirty = nested_vmx_write_pml_buffer,
 	.enable_evmcs = nested_enable_evmcs,
 	.get_evmcs_version = nested_get_evmcs_version,
+	.get_evmcs_page = nested_get_evmcs_page,
 };
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 3ae401113a80b..283a3f52aa135 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9727,12 +9727,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 			r = -EIO;
 			goto out;
 		}
-		if (kvm_check_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu)) {
-			if (unlikely(!kvm_x86_ops.nested_ops->get_nested_state_pages(vcpu))) {
-				r = 0;
-				goto out;
-			}
-		}
 		if (kvm_check_request(KVM_REQ_MMU_RELOAD, vcpu))
 			kvm_mmu_unload(vcpu);
 		if (kvm_check_request(KVM_REQ_MIGRATE_TIMER, vcpu))
-- 
2.26.3


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux