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 ?


> Paolo
>
>>      }
>>  
>> 

Reply via email to