| Message ID | 20180612225244.71856-9-junaids@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 13/06/2018 00:52, Junaid Shahid wrote: > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index aa7da1d8ece2..e4ee76bee3dd 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -2122,16 +2122,22 @@ static void shrink_halt_poll_ns(struct kvm_vcpu *vcpu) > > static int kvm_vcpu_check_block(struct kvm_vcpu *vcpu) > { > + int ret = -EINTR; > + int idx = srcu_read_lock(&vcpu->kvm->srcu); > + > if (kvm_arch_vcpu_runnable(vcpu)) { > kvm_make_request(KVM_REQ_UNHALT, vcpu); > - return -EINTR; > + goto out; > } > if (kvm_cpu_has_pending_timer(vcpu)) > - return -EINTR; > + goto out; > if (signal_pending(current)) > - return -EINTR; > + goto out; > > - return 0; > + ret = 0; > +out: > + srcu_read_unlock(&vcpu->kvm->srcu, idx); > + return ret; > } What needs kvm->srcu here? Thanks, Paolo
On 06/13/2018 04:28 AM, Paolo Bonzini wrote: > > What needs kvm->srcu here? > That is needed because of the call to kvm_arch_vcpu_runnable(). That function may call vmx_check_nested_events(), which can call nested_vmx_vmexit(). That in turn would eventually call fast_cr3_switch(), which needs the kvm->srcu in order to verify (via mmu_check_root) that the new CR3 belongs to a valid memslot.
On 14/06/2018 01:31, Junaid Shahid wrote: > On 06/13/2018 04:28 AM, Paolo Bonzini wrote: >> >> What needs kvm->srcu here? >> > > That is needed because of the call to kvm_arch_vcpu_runnable(). That > function may call vmx_check_nested_events(), which can call > nested_vmx_vmexit(). That in turn would eventually call > fast_cr3_switch(), which needs the kvm->srcu in order to verify (via > mmu_check_root) that the new CR3 belongs to a valid memslot. > Hmm that's not very intuitive, and it should already need SRCU for nested_mark_vmcs12_pages_dirty (called by vmx_complete_nested_posted_interrupt) actually. vmx_check_nested_events() is called via kvm_vcpu_running, but really all the conditions that are checked by vmx_check_nested_events would also be checked by kvm_vcpu_has_events (well, except for vmx->nested.preemption_timer_expired), so perhaps we can spare it. I'll take a look. Paolo
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index 845ca328cf38..94d4cf4cb743 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -4043,7 +4043,7 @@ static bool fast_cr3_switch(struct kvm_vcpu *vcpu, gpa_t new_cr3, return false; swap(mmu->root_hpa, mmu->prev_root.hpa); - mmu->prev_root.cr3 = kvm_read_cr3(vcpu); + mmu->prev_root.cr3 = mmu->get_cr3(vcpu); if (new_cr3 == prev_cr3 && VALID_PAGE(mmu->root_hpa) && @@ -4075,6 +4075,7 @@ void kvm_mmu_new_cr3(struct kvm_vcpu *vcpu, gpa_t new_cr3, if (!fast_cr3_switch(vcpu, new_cr3, new_role)) kvm_mmu_free_roots(vcpu, false); } +EXPORT_SYMBOL_GPL(kvm_mmu_new_cr3); static unsigned long get_cr3(struct kvm_vcpu *vcpu) { diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 48989f78be60..9c85c6249280 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -10564,7 +10564,9 @@ static int nested_ept_init_mmu_context(struct kvm_vcpu *vcpu) if (!valid_ept_address(vcpu, nested_ept_get_cr3(vcpu))) return 1; - kvm_mmu_unload(vcpu); + kvm_mmu_new_cr3(vcpu, nested_ept_get_cr3(vcpu), + kvm_mmu_calc_shadow_ept_root_page_role(vcpu, + nested_ept_ad_enabled(vcpu))); kvm_init_shadow_ept_mmu(vcpu, to_vmx(vcpu)->nested.msrs.ept_caps & VMX_EPT_EXECUTE_ONLY_BIT, @@ -11119,12 +11121,16 @@ static int nested_vmx_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3, bool ne return 1; } } - - vcpu->arch.cr3 = cr3; - __set_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail); } - kvm_mmu_reset_context(vcpu); + if (!nested_ept) + kvm_mmu_new_cr3(vcpu, cr3, kvm_mmu_calc_root_page_role(vcpu)); + + vcpu->arch.cr3 = cr3; + __set_bit(VCPU_EXREG_CR3, (ulong *)&vcpu->arch.regs_avail); + + kvm_init_mmu(vcpu, false); + return 0; } diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index aa7da1d8ece2..e4ee76bee3dd 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2122,16 +2122,22 @@ static void shrink_halt_poll_ns(struct kvm_vcpu *vcpu) static int kvm_vcpu_check_block(struct kvm_vcpu *vcpu) { + int ret = -EINTR; + int idx = srcu_read_lock(&vcpu->kvm->srcu); + if (kvm_arch_vcpu_runnable(vcpu)) { kvm_make_request(KVM_REQ_UNHALT, vcpu); - return -EINTR; + goto out; } if (kvm_cpu_has_pending_timer(vcpu)) - return -EINTR; + goto out; if (signal_pending(current)) - return -EINTR; + goto out; - return 0; + ret = 0; +out: + srcu_read_unlock(&vcpu->kvm->srcu, idx); + return ret; } /*
Use the fast CR3 switch mechanism to locklessly change the MMU root page when switching between L1 and L2. The switch from L2 to L1 should always go through the fast path, while the switch from L1 to L2 should go through the fast path if L1's CR3/EPTP for L2 hasn't changed since the last time. Signed-off-by: Junaid Shahid <junaids@google.com> --- arch/x86/kvm/mmu.c | 3 ++- arch/x86/kvm/vmx.c | 16 +++++++++++----- virt/kvm/kvm_main.c | 14 ++++++++++---- 3 files changed, 23 insertions(+), 10 deletions(-)