[PATCH 0/3] nVMX: Emulated Page Modification Logging for Nested Virtualization

[PATCH 0/3] nVMX: Emulated Page Modification Logging for Nested Virtualization

[Bandan Das]

These patches implement PML on top of EPT A/D emulation
(ae1e2d1082ae).

When dirty bit is being set, we write the gpa to the
buffer provided by L1. If the index overflows, we just
change the exit reason before running L1.

Bandan Das (3):
  kvm: x86: Add a hook for arch specific dirty logging emulation
  nVMX: Implement emulated Page Modification Logging
  nVMX: Advertise PML to L1 hypervisor

 arch/x86/include/asm/kvm_host.h |  2 +
 arch/x86/kvm/mmu.c              | 15 +++++++
 arch/x86/kvm/mmu.h              |  1 +
 arch/x86/kvm/paging_tmpl.h      |  4 ++
 arch/x86/kvm/vmx.c              | 87 ++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 107 insertions(+), 2 deletions(-)

-- 
2.9.3

[PATCH 1/3] kvm: x86: Add a hook for arch specific dirty logging emulation

[Bandan Das]

When KVM updates accessed/dirty bits, this hook can be used
to invoke an arch specific function that implements/emulates
dirty logging such as PML.

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |  2 ++
 arch/x86/kvm/mmu.c              | 15 +++++++++++++++
 arch/x86/kvm/mmu.h              |  1 +
 arch/x86/kvm/paging_tmpl.h      |  4 ++++
 4 files changed, 22 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index f5bddf92..9c761fe 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1020,6 +1020,8 @@ struct kvm_x86_ops {
 	void (*enable_log_dirty_pt_masked)(struct kvm *kvm,
 					   struct kvm_memory_slot *slot,
 					   gfn_t offset, unsigned long mask);
+	int (*write_log_dirty)(struct kvm_vcpu *vcpu);
+
 	/* pmu operations of sub-arch */
 	const struct kvm_pmu_ops *pmu_ops;
 
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 5586765..5d3376f 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1498,6 +1498,21 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
 		kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
 }
 
+/**
+ * kvm_arch_write_log_dirty - emulate dirty page logging
+ * @vcpu: Guest mode vcpu
+ *
+ * Emulate arch specific page modification logging for the
+ * nested hypervisor
+ */
+int kvm_arch_write_log_dirty(struct kvm_vcpu *vcpu)
+{
+	if (kvm_x86_ops->write_log_dirty)
+		return kvm_x86_ops->write_log_dirty(vcpu);
+
+	return 0;
+}
+
 bool kvm_mmu_slot_gfn_write_protect(struct kvm *kvm,
 				    struct kvm_memory_slot *slot, u64 gfn)
 {
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index d8ccb32..2797580 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -202,4 +202,5 @@ void kvm_mmu_gfn_disallow_lpage(struct kvm_memory_slot *slot, gfn_t gfn);
 void kvm_mmu_gfn_allow_lpage(struct kvm_memory_slot *slot, gfn_t gfn);
 bool kvm_mmu_slot_gfn_write_protect(struct kvm *kvm,
 				    struct kvm_memory_slot *slot, u64 gfn);
+int kvm_arch_write_log_dirty(struct kvm_vcpu *vcpu);
 #endif
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 314d207..5624174 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -226,6 +226,10 @@ static int FNAME(update_accessed_dirty_bits)(struct kvm_vcpu *vcpu,
 		if (level == walker->level && write_fault &&
 				!(pte & PT_GUEST_DIRTY_MASK)) {
 			trace_kvm_mmu_set_dirty_bit(table_gfn, index, sizeof(pte));
+#if PTTYPE == PTTYPE_EPT
+			if (kvm_arch_write_log_dirty(vcpu))
+				return -EINVAL;
+#endif
 			pte |= PT_GUEST_DIRTY_MASK;
 		}
 		if (pte == orig_pte)
-- 
2.9.3

[PATCH 2/3] nVMX: Implement emulated Page Modification Logging

[Bandan Das]

With EPT A/D enabled, processor access to L2 guest
paging structures will result in a write violation.
When this happens, write the GUEST_PHYSICAL_ADDRESS
to the pml buffer provided by L1 if the access is
write and the dirty bit is being set.

This patch also adds necessary checks during VMEntry if L1
has enabled PML. If the PML index overflows, we change the
exit reason and run L1 to simulate a PML full event.

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 arch/x86/kvm/vmx.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 80 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 29940b6..5e5abb7 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -248,6 +248,7 @@ struct __packed vmcs12 {
 	u64 xss_exit_bitmap;
 	u64 guest_physical_address;
 	u64 vmcs_link_pointer;
+	u64 pml_address;
 	u64 guest_ia32_debugctl;
 	u64 guest_ia32_pat;
 	u64 guest_ia32_efer;
@@ -369,6 +370,7 @@ struct __packed vmcs12 {
 	u16 guest_ldtr_selector;
 	u16 guest_tr_selector;
 	u16 guest_intr_status;
+	u16 guest_pml_index;
 	u16 host_es_selector;
 	u16 host_cs_selector;
 	u16 host_ss_selector;
@@ -407,6 +409,7 @@ struct nested_vmx {
 	/* Has the level1 guest done vmxon? */
 	bool vmxon;
 	gpa_t vmxon_ptr;
+	bool pml_full;
 
 	/* The guest-physical address of the current VMCS L1 keeps for L2 */
 	gpa_t current_vmptr;
@@ -742,6 +745,7 @@ static const unsigned short vmcs_field_to_offset_table[] = {
 	FIELD(GUEST_LDTR_SELECTOR, guest_ldtr_selector),
 	FIELD(GUEST_TR_SELECTOR, guest_tr_selector),
 	FIELD(GUEST_INTR_STATUS, guest_intr_status),
+	FIELD(GUEST_PML_INDEX, guest_pml_index),
 	FIELD(HOST_ES_SELECTOR, host_es_selector),
 	FIELD(HOST_CS_SELECTOR, host_cs_selector),
 	FIELD(HOST_SS_SELECTOR, host_ss_selector),
@@ -767,6 +771,7 @@ static const unsigned short vmcs_field_to_offset_table[] = {
 	FIELD64(XSS_EXIT_BITMAP, xss_exit_bitmap),
 	FIELD64(GUEST_PHYSICAL_ADDRESS, guest_physical_address),
 	FIELD64(VMCS_LINK_POINTER, vmcs_link_pointer),
+	FIELD64(PML_ADDRESS, pml_address),
 	FIELD64(GUEST_IA32_DEBUGCTL, guest_ia32_debugctl),
 	FIELD64(GUEST_IA32_PAT, guest_ia32_pat),
 	FIELD64(GUEST_IA32_EFER, guest_ia32_efer),
@@ -1349,6 +1354,11 @@ static inline bool nested_cpu_has_xsaves(struct vmcs12 *vmcs12)
 		vmx_xsaves_supported();
 }
 
+static inline bool nested_cpu_has_pml(struct vmcs12 *vmcs12)
+{
+	return nested_cpu_has2(vmcs12, SECONDARY_EXEC_ENABLE_PML);
+}
+
 static inline bool nested_cpu_has_virt_x2apic_mode(struct vmcs12 *vmcs12)
 {
 	return nested_cpu_has2(vmcs12, SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE);
@@ -9368,12 +9378,21 @@ static void nested_ept_inject_page_fault(struct kvm_vcpu *vcpu,
 		struct x86_exception *fault)
 {
 	struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
 	u32 exit_reason;
 
-	if (fault->error_code & PFERR_RSVD_MASK)
+	if (vmx->nested.pml_full) {
+		exit_reason = EXIT_REASON_PML_FULL;
+		vmx->nested.pml_full = false;
+	} else if (fault->error_code & PFERR_RSVD_MASK)
 		exit_reason = EXIT_REASON_EPT_MISCONFIG;
 	else
 		exit_reason = EXIT_REASON_EPT_VIOLATION;
+	/*
+	 * The definition of bit 12 for EPT violations and PML
+	 * full event is the same, so pass it through since
+	 * the rest of the bits are undefined.
+	 */
 	nested_vmx_vmexit(vcpu, exit_reason, 0, vcpu->arch.exit_qualification);
 	vmcs12->guest_physical_address = fault->address;
 }
@@ -9717,6 +9736,22 @@ static int nested_vmx_check_msr_switch_controls(struct kvm_vcpu *vcpu,
 	return 0;
 }
 
+static int nested_vmx_check_pml_controls(struct kvm_vcpu *vcpu,
+					 struct vmcs12 *vmcs12)
+{
+	u64 address = vmcs12->pml_address;
+	int maxphyaddr = cpuid_maxphyaddr(vcpu);
+
+	if (nested_cpu_has2(vmcs12, SECONDARY_EXEC_ENABLE_PML)) {
+		if (!nested_cpu_has_ept(vmcs12) ||
+		    !IS_ALIGNED(address, 4096)  ||
+		    address >> maxphyaddr)
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int nested_vmx_msr_check_common(struct kvm_vcpu *vcpu,
 				       struct vmx_msr_entry *e)
 {
@@ -10240,6 +10275,9 @@ static int check_vmentry_prereqs(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 	if (nested_vmx_check_msr_switch_controls(vcpu, vmcs12))
 		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
 
+	if (nested_vmx_check_pml_controls(vcpu, vmcs12))
+		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
+
 	if (!vmx_control_verify(vmcs12->cpu_based_vm_exec_control,
 				vmx->nested.nested_vmx_procbased_ctls_low,
 				vmx->nested.nested_vmx_procbased_ctls_high) ||
@@ -11134,6 +11172,46 @@ static void vmx_flush_log_dirty(struct kvm *kvm)
 	kvm_flush_pml_buffers(kvm);
 }
 
+static int vmx_write_pml_buffer(struct kvm_vcpu *vcpu)
+{
+	struct vmcs12 *vmcs12;
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
+	gpa_t gpa;
+	struct page *page = NULL;
+	u64 *pml_address;
+
+	if (is_guest_mode(vcpu)) {
+		WARN_ON_ONCE(vmx->nested.pml_full);
+
+		/*
+		 * Check if PML is enabled for the nested guest.
+		 * Whether eptp bit 6 is set is already checked
+		 * as part of A/D emulation.
+		 */
+		vmcs12 = get_vmcs12(vcpu);
+		if (!nested_cpu_has_pml(vmcs12))
+			return 0;
+
+		if (vmcs12->guest_pml_index > PML_ENTITY_NUM) {
+			vmx->nested.pml_full = true;
+			return 1;
+		}
+
+		gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS) & ~0xFFFull;
+
+		page = nested_get_page(vcpu, vmcs12->pml_address);
+		if (!page)
+			return 0;
+
+		pml_address = kmap(page);
+		pml_address[vmcs12->guest_pml_index--] = gpa;
+		kunmap(page);
+		nested_release_page_clean(page);
+	}
+
+	return 0;
+}
+
 static void vmx_enable_log_dirty_pt_masked(struct kvm *kvm,
 					   struct kvm_memory_slot *memslot,
 					   gfn_t offset, unsigned long mask)
@@ -11493,6 +11571,7 @@ static struct kvm_x86_ops vmx_x86_ops __ro_after_init = {
 	.slot_disable_log_dirty = vmx_slot_disable_log_dirty,
 	.flush_log_dirty = vmx_flush_log_dirty,
 	.enable_log_dirty_pt_masked = vmx_enable_log_dirty_pt_masked,
+	.write_log_dirty = vmx_write_pml_buffer,
 
 	.pre_block = vmx_pre_block,
 	.post_block = vmx_post_block,
-- 
2.9.3

Re: [PATCH 2/3] nVMX: Implement emulated Page Modification Logging

[Paolo Bonzini]

On 04/05/2017 00:14, Bandan Das wrote:
> +	if (vmx->nested.pml_full) {
> +		exit_reason = EXIT_REASON_PML_FULL;
> +		vmx->nested.pml_full = false;
> +	} else if (fault->error_code & PFERR_RSVD_MASK)
>  		exit_reason = EXIT_REASON_EPT_MISCONFIG;
>  	else
>  		exit_reason = EXIT_REASON_EPT_VIOLATION;
> +	/*
> +	 * The definition of bit 12 for EPT violations and PML
> +	 * full event is the same, so pass it through since
> +	 * the rest of the bits are undefined.
> +	 */

Please zero all other bits instead.  It's as easy as adding an "u64
exit_qualification" local variable.

Paolo

>  	nested_vmx_vmexit(vcpu, exit_reason, 0, vcpu->arch.exit_qualification);

Re: [PATCH 2/3] nVMX: Implement emulated Page Modification Logging

[Bandan Das]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 04/05/2017 00:14, Bandan Das wrote:
>> +	if (vmx->nested.pml_full) {
>> +		exit_reason = EXIT_REASON_PML_FULL;
>> +		vmx->nested.pml_full = false;
>> +	} else if (fault->error_code & PFERR_RSVD_MASK)
>>  		exit_reason = EXIT_REASON_EPT_MISCONFIG;
>>  	else
>>  		exit_reason = EXIT_REASON_EPT_VIOLATION;
>> +	/*
>> +	 * The definition of bit 12 for EPT violations and PML
>> +	 * full event is the same, so pass it through since
>> +	 * the rest of the bits are undefined.
>> +	 */
>
> Please zero all other bits instead.  It's as easy as adding an "u64
> exit_qualification" local variable.

Will do, thanks for the review.

Bandan

> Paolo
>
>>  	nested_vmx_vmexit(vcpu, exit_reason, 0, vcpu->arch.exit_qualification);

[PATCH 3/3] nVMX: Advertise PML to L1 hypervisor

[Bandan Das]

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@redhat.com>
---
 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);
 	}
 
-- 
2.9.3

Re: [PATCH 3/3] nVMX: Advertise PML to L1 hypervisor

[Paolo Bonzini]

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@redhat.com>
> ---
>  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;

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.

Paolo

>  	}
>  
> 

Re: [PATCH 3/3] nVMX: Advertise PML to L1 hypervisor

[Bandan Das]

Paolo Bonzini <pbonzini@redhat.com> 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@redhat.com>
>> ---
>>  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
>
>>  	}
>>  
>> 

Re: [PATCH 3/3] nVMX: Advertise PML to L1 hypervisor

[Paolo Bonzini]

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