From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [PATCH] KVM: nVMX: fix nested EPT detection Date: Wed, 22 Mar 2017 18:42:02 +0100 Message-ID: <20170322174201.GC1910@potion> References: <1490202597-5926-1-git-send-email-lprosek@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: kvm@vger.kernel.org, pbonzini@redhat.com, wanpeng.li@hotmail.com To: Ladi Prosek Return-path: Received: from mx1.redhat.com ([209.132.183.28]:57878 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935672AbdCVRmM (ORCPT ); Wed, 22 Mar 2017 13:42:12 -0400 Content-Disposition: inline In-Reply-To: <1490202597-5926-1-git-send-email-lprosek@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 2017-03-22 18:09+0100, Ladi Prosek: > The nested_ept_enabled flag introduced in commit 7ca29de2136 was not > computed correctly. We are interested only in L1's EPT state, not the > the combined L0+L1 value. > > In particular, if L0 uses EPT but L1 does not, nested_ept_enabled must > be false to make sure that PDPSTRs are loaded based on CR3 as usual, > because the special case described in 26.3.2.4 Loading Page-Directory- > Pointer-Table Entries does not apply. > > Reported-by: Wanpeng Li > Signed-off-by: Ladi Prosek Adding a standardized "Fixes:" tag even if you mention the commit in text allows simpler automated backports to stable kernels, Fixes: 7ca29de21362 ("KVM: nVMX: fix CR3 load if L2 uses PAE paging and EPT") > --- > arch/x86/kvm/vmx.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 98e82ee..a525c72 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -10105,8 +10105,11 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, > SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY | > SECONDARY_EXEC_APIC_REGISTER_VIRT); > if (nested_cpu_has(vmcs12, > - CPU_BASED_ACTIVATE_SECONDARY_CONTROLS)) > + CPU_BASED_ACTIVATE_SECONDARY_CONTROLS)) { > exec_control |= vmcs12->secondary_vm_exec_control; > + nested_ept_enabled = (vmcs12->secondary_vm_exec_control & > + SECONDARY_EXEC_ENABLE_EPT) != 0; I'd prefer to get rid of nested_ept_enabled and directly call 'nested_cpu_has_ept(vmcs12)' instead, like the other place in prepare_vmcs02() that asks whether we have nested ept. The change looks correct, Reviewed-by: Radim Krčmář Thanks. > + } > > if (exec_control & SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY) { > vmcs_write64(EOI_EXIT_BITMAP0, > @@ -10121,8 +10124,6 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, > vmcs12->guest_intr_status); > } > > - nested_ept_enabled = (exec_control & SECONDARY_EXEC_ENABLE_EPT) != 0; > - > /* > * Write an illegal value to APIC_ACCESS_ADDR. Later, > * nested_get_vmcs12_pages will either fix it up or > -- > 2.7.4 >