On 04/05/2017 20:22, Bandan Das wrote: > Paolo Bonzini <[email protected]> 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 <[email protected]> >>> --- >>> 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 ?
Oh, I missed that, I was looking at an old tree. That's better, thanks. :) Paolo > > >> Paolo >> >>> } >>> >>>
