On 04/05/2017 20:22, Bandan Das wrote:
> Paolo Bonzini <pbonzini@xxxxxxxxxx> 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 <bsd@xxxxxxxxxx>
>>> ---
>>> 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
>>
>>> }
>>>
>>>
