From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755915AbdEDSXS (ORCPT ); Thu, 4 May 2017 14:23:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47056 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751738AbdEDSWk (ORCPT ); Thu, 4 May 2017 14:22:40 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com F37FF4AEBD Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=bsd@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com F37FF4AEBD From: Bandan Das To: Paolo Bonzini Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3] nVMX: Advertise PML to L1 hypervisor References: <20170503221457.18869-1-bsd@redhat.com> <20170503221457.18869-4-bsd@redhat.com> Date: Thu, 04 May 2017 14:22:38 -0400 In-Reply-To: (Paolo Bonzini's message of "Thu, 4 May 2017 09:03:49 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Thu, 04 May 2017 18:22:40 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Paolo Bonzini writes: > On 04/05/2017 00:14, Bandan Das wrote: >> Advertise the PML bit in vmcs12 but clear it out >> before running L2 since we don't depend on hardware support >> for PML emulation. >> >> Signed-off-by: Bandan Das >> --- >> arch/x86/kvm/vmx.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index 5e5abb7..df71116 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -2763,8 +2763,11 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx) >> vmx->nested.nested_vmx_ept_caps |= VMX_EPT_EXTENT_GLOBAL_BIT | >> VMX_EPT_EXTENT_CONTEXT_BIT | VMX_EPT_2MB_PAGE_BIT | >> VMX_EPT_1GB_PAGE_BIT; >> - if (enable_ept_ad_bits) >> + if (enable_ept_ad_bits) { >> + vmx->nested.nested_vmx_secondary_ctls_high |= >> + SECONDARY_EXEC_ENABLE_PML; >> vmx->nested.nested_vmx_ept_caps |= VMX_EPT_AD_BIT; >> + } >> } else >> vmx->nested.nested_vmx_ept_caps = 0; >> >> @@ -10080,6 +10083,7 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, >> if (exec_control & SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES) >> vmcs_write64(APIC_ACCESS_ADDR, -1ull); >> >> + exec_control &= ~SECONDARY_EXEC_ENABLE_PML; >> vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_control); > > L0 is still using its own page modification log when running L2, so you > have to clear the bit here instead: > > exec_control |= vmcs12->secondary_vm_exec_control; > Oops, good catch, thank you! > and set up PML_ADDRESS and GUEST_PML_INDEX. Though, the lack of > PML_ADDRESS and GUEST_PML_INDEX initialization is a pre-existing bug. A little further down I see that these fields are being reset as part of commit 1fb883bb827: ... if (enable_pml) { /* * Conceptually we want to copy the PML address and index from * vmcs01 here, and then back to vmcs01 on nested vmexit. But, * since we always flush the log on each vmexit, this happens * to be equivalent to simply resetting the fields in vmcs02. */ ASSERT(vmx->pml_pg); vmcs_write64(PML_ADDRESS, page_to_phys(vmx->pml_pg)); vmcs_write16(GUEST_PML_INDEX, PML_ENTITY_NUM - 1); } Or are you referring to a different place, these fields need to be set ? > Paolo > >> } >> >>