[patch 0/5] VMX EPT misconfigurtion handler

[patch 0/5] VMX EPT misconfigurtion handler

[Marcelo Tosatti]

>From the Intel docs:
An EPT misconfiguration occurs when, in the course of translation
a guest-physical address, the logical processor encounters an EPT
paging-structure entry that contains an unsupported value.

Handle this event and print useful information for diagnostics.

[patch 1/5] KVM: VMX: more MSR_IA32_VMX_EPT_VPID_CAP capability bits

[Marcelo Tosatti]

[-- Attachment #1: vmx-ept-check-supported-bits --]
[-- Type: text/plain, Size: 1763 bytes --]

Required for EPT misconfiguration handler.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/include/asm/vmx.h
===================================================================
--- kvm.orig/arch/x86/include/asm/vmx.h
+++ kvm/arch/x86/include/asm/vmx.h
@@ -351,9 +351,16 @@ enum vmcs_field {
 #define VMX_EPT_EXTENT_INDIVIDUAL_ADDR		0
 #define VMX_EPT_EXTENT_CONTEXT			1
 #define VMX_EPT_EXTENT_GLOBAL			2
+
+#define VMX_EPT_EXECUTE_ONLY_BIT		(1ull)
+#define VMX_EPT_PAGE_WALK_4_BIT			(1ull << 6)
+#define VMX_EPTP_UC_BIT				(1ull << 8)
+#define VMX_EPTP_WB_BIT				(1ull << 14)
+#define VMX_EPT_2MB_PAGE_BIT			(1ull << 16)
 #define VMX_EPT_EXTENT_INDIVIDUAL_BIT		(1ull << 24)
 #define VMX_EPT_EXTENT_CONTEXT_BIT		(1ull << 25)
 #define VMX_EPT_EXTENT_GLOBAL_BIT		(1ull << 26)
+
 #define VMX_EPT_DEFAULT_GAW			3
 #define VMX_EPT_MAX_GAW				0x4
 #define VMX_EPT_MT_EPTE_SHIFT			3
Index: kvm/arch/x86/kvm/vmx.c
===================================================================
--- kvm.orig/arch/x86/kvm/vmx.c
+++ kvm/arch/x86/kvm/vmx.c
@@ -258,6 +258,26 @@ static inline bool cpu_has_vmx_flexprior
 		cpu_has_vmx_virtualize_apic_accesses();
 }
 
+static inline bool cpu_has_vmx_ept_execute_only(void)
+{
+	return !!(vmx_capability.ept & VMX_EPT_EXECUTE_ONLY_BIT);
+}
+
+static inline bool cpu_has_vmx_eptp_uncacheable(void)
+{
+	return !!(vmx_capability.ept & VMX_EPTP_UC_BIT);
+}
+
+static inline bool cpu_has_vmx_eptp_writeback(void)
+{
+	return !!(vmx_capability.ept & VMX_EPTP_WB_BIT);
+}
+
+static inline bool cpu_has_vmx_ept_2m_page(void)
+{
+	return !!(vmx_capability.ept & VMX_EPT_2MB_PAGE_BIT);
+}
+
 static inline int cpu_has_vmx_invept_individual_addr(void)
 {
 	return !!(vmx_capability.ept & VMX_EPT_EXTENT_INDIVIDUAL_BIT);

[patch 2/5] KVM: MMU: make for_each_shadow_entry aware of largepages

[Marcelo Tosatti]

[-- Attachment #1: kvm-shadow-iterator-largepage-descend --]
[-- Type: text/plain, Size: 697 bytes --]

This way there is no need to add explicit checks in every
for_each_shadow_entry user.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/mmu.c
===================================================================
--- kvm.orig/arch/x86/kvm/mmu.c
+++ kvm/arch/x86/kvm/mmu.c
@@ -1273,6 +1273,11 @@ static bool shadow_walk_okay(struct kvm_
 {
 	if (iterator->level < PT_PAGE_TABLE_LEVEL)
 		return false;
+
+	if (iterator->level == PT_PAGE_TABLE_LEVEL)
+		if (is_large_pte(*iterator->sptep))
+			return false;
+
 	iterator->index = SHADOW_PT_INDEX(iterator->addr, iterator->level);
 	iterator->sptep	= ((u64 *)__va(iterator->shadow_addr)) + iterator->index;
 	return true;

[patch 3/5] KVM: MMU: add kvm_mmu_shadow_walk helper

[Marcelo Tosatti]

[-- Attachment #1: kvm_mmu_walk_shadow --]
[-- Type: text/plain, Size: 1421 bytes --]

Required by EPT misconfiguration handler.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/mmu.c
===================================================================
--- kvm.orig/arch/x86/kvm/mmu.c
+++ kvm/arch/x86/kvm/mmu.c
@@ -3013,6 +3013,26 @@ out:
 	return r;
 }
 
+void kvm_mmu_shadow_walk(struct kvm_vcpu *vcpu, u64 addr,
+			 struct mmu_shadow_walk *walk)
+{
+	struct kvm_shadow_walk_iterator iterator;
+
+	spin_lock(&vcpu->kvm->mmu_lock);
+	for_each_shadow_entry(vcpu, addr, iterator) {
+		int err;
+
+		err = walk->fn(vcpu, iterator.sptep, iterator.level, walk);
+		if (err)
+			break;
+
+		if (!is_shadow_present_pte(*iterator.sptep))
+			break;
+	}
+	spin_unlock(&vcpu->kvm->mmu_lock);
+}
+EXPORT_SYMBOL(kvm_mmu_shadow_walk);
+
 #ifdef AUDIT
 
 static const char *audit_msg;
Index: kvm/arch/x86/kvm/mmu.h
===================================================================
--- kvm.orig/arch/x86/kvm/mmu.h
+++ kvm/arch/x86/kvm/mmu.h
@@ -37,6 +37,14 @@
 #define PT32_ROOT_LEVEL 2
 #define PT32E_ROOT_LEVEL 3
 
+struct mmu_shadow_walk {
+	int (*fn) (struct kvm_vcpu *vcpu, u64 *sptep, int level,
+		   struct mmu_shadow_walk *walk);
+};
+
+void kvm_mmu_shadow_walk(struct kvm_vcpu *vcpu, u64 addr,
+			 struct mmu_shadow_walk *walk);
+
 static inline void kvm_mmu_free_some_pages(struct kvm_vcpu *vcpu)
 {
 	if (unlikely(vcpu->kvm->arch.n_free_mmu_pages < KVM_MIN_FREE_MMU_PAGES))

[patch 4/5] KVM: VMX: EPT misconfiguration handler

[Marcelo Tosatti]

[-- Attachment #1: vmx-misconfig-handler --]
[-- Type: text/plain, Size: 3355 bytes --]

Handler for EPT misconfiguration which checks for valid state 
in the shadow pagetables, printing the spte on each level.

The separate WARN_ONs are useful for kerneloops.org.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/vmx.c
===================================================================
--- kvm.orig/arch/x86/kvm/vmx.c
+++ kvm/arch/x86/kvm/vmx.c
@@ -3173,6 +3173,100 @@ static int handle_ept_violation(struct k
 	return kvm_mmu_page_fault(vcpu, gpa & PAGE_MASK, 0);
 }
 
+static u64 ept_rsvd_mask(u64 *sptep, int level)
+{
+	int i;
+	u64 mask = 0;
+
+	for (i = 51; i > boot_cpu_data.x86_phys_bits; i--)
+		mask |= (1ULL << i);
+
+	if (level > 2)
+		/* bits 7:3 reserved */
+		mask |= 0xf8;
+	else if (level == 2) {
+		if (*sptep & (1ULL << 7))
+			/* 2MB ref, bits 20:12 reserved */
+			mask |= 0x1ff000;
+		else
+			/* bits 6:3 reserved */
+			mask |= 0x78;
+	}
+
+	return mask;
+}
+
+struct ept_inspect_spte {
+	int print;
+	struct mmu_shadow_walk walk;
+};
+
+static int ept_misconfig_inspect_spte(struct kvm_vcpu *vcpu, u64 *sptep,
+				      int level,
+				      struct mmu_shadow_walk *walk)
+{
+	struct ept_inspect_spte *iept = container_of(walk,
+					struct ept_inspect_spte, walk);
+
+	if (iept->print)
+		printk(KERN_ERR "%s: sptep %p spte 0x%llx level %d\n",
+				__func__, sptep, *sptep, level);
+
+	/* 010b (write-only) */
+	WARN_ON((*sptep & 0x7) == 0x2);
+
+	/* 110b (write/execute) */
+	WARN_ON((*sptep & 0x7) == 0x6);
+
+	/* 100b (execute-only) and value not supported by logical processor */
+	if (!cpu_has_vmx_ept_execute_only())
+		WARN_ON((*sptep & 0x7) == 0x4);
+
+	/* not 000b */
+	if ((*sptep & 0x7)) {
+		u64 rsvd_bits = *sptep & ept_rsvd_mask(sptep, level);
+
+		if (rsvd_bits != 0) {
+			printk(KERN_ERR "%s: rsvd_bits = 0x%llx\n",
+					 __func__, rsvd_bits);
+			WARN_ON(1);
+		}
+
+		if (level == 1 || (level == 2 && (*sptep & (1ULL << 7)))) {
+			u64 ept_mem_type = (*sptep & 0x38) >> 3;
+
+			if (ept_mem_type == 2 || ept_mem_type == 3 ||
+			    ept_mem_type == 7) {
+				printk(KERN_ERR "%s: ept_mem_type=0x%llx\n",
+						__func__, ept_mem_type);
+				WARN_ON(1);
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int handle_ept_misconfig(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
+{
+	gpa_t gpa;
+	struct ept_inspect_spte iept = { .print = 1,
+					 .walk.fn =
+					  ept_misconfig_inspect_spte };
+
+	gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
+
+	printk(KERN_ERR "EPT: Misconfiguration.\n");
+	printk(KERN_ERR "EPT: GPA: 0x%llx\n", gpa);
+
+	kvm_mmu_shadow_walk(vcpu, gpa, &iept.walk);
+
+	kvm_run->exit_reason = KVM_EXIT_UNKNOWN;
+	kvm_run->hw.hardware_exit_reason = EXIT_REASON_EPT_MISCONFIG;
+
+	return 0;
+}
+
 static int handle_nmi_window(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 {
 	u32 cpu_based_vm_exec_control;
@@ -3243,8 +3337,9 @@ static int (*kvm_vmx_exit_handlers[])(st
 	[EXIT_REASON_APIC_ACCESS]             = handle_apic_access,
 	[EXIT_REASON_WBINVD]                  = handle_wbinvd,
 	[EXIT_REASON_TASK_SWITCH]             = handle_task_switch,
-	[EXIT_REASON_EPT_VIOLATION]	      = handle_ept_violation,
 	[EXIT_REASON_MCE_DURING_VMENTRY]      = handle_machine_check,
+	[EXIT_REASON_EPT_VIOLATION]	      = handle_ept_violation,
+	[EXIT_REASON_EPT_MISCONFIG]           = handle_ept_misconfig,
 };
 
 static const int kvm_vmx_max_exit_handlers =

[patch 5/5] KVM: VMX: conditionally disable 2M pages

[Marcelo Tosatti]

[-- Attachment #1: disable-2m-pages-vmx --]
[-- Type: text/plain, Size: 2137 bytes --]

Disable usage of 2M pages if VMX_EPT_2MB_PAGE_BIT (bit 16) is clear
in MSR_IA32_VMX_EPT_VPID_CAP and EPT is enabled.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/vmx.c
===================================================================
--- kvm.orig/arch/x86/kvm/vmx.c
+++ kvm/arch/x86/kvm/vmx.c
@@ -1369,6 +1369,9 @@ static __init int hardware_setup(void)
 	if (!cpu_has_vmx_tpr_shadow())
 		kvm_x86_ops->update_cr8_intercept = NULL;
 
+	if (enable_ept && !cpu_has_vmx_ept_2m_page())
+		kvm_disable_largepages();
+
 	return alloc_kvm_area();
 }
 
Index: kvm/include/linux/kvm_host.h
===================================================================
--- kvm.orig/include/linux/kvm_host.h
+++ kvm/include/linux/kvm_host.h
@@ -203,6 +203,7 @@ int kvm_arch_set_memory_region(struct kv
 				struct kvm_userspace_memory_region *mem,
 				struct kvm_memory_slot old,
 				int user_alloc);
+void kvm_disable_largepages(void);
 void kvm_arch_flush_shadow(struct kvm *kvm);
 gfn_t unalias_gfn(struct kvm *kvm, gfn_t gfn);
 struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn);
Index: kvm/virt/kvm/kvm_main.c
===================================================================
--- kvm.orig/virt/kvm/kvm_main.c
+++ kvm/virt/kvm/kvm_main.c
@@ -85,6 +85,8 @@ static long kvm_vcpu_ioctl(struct file *
 
 static bool kvm_rebooting;
 
+static bool largepages_disabled = false;
+
 #ifdef KVM_CAP_DEVICE_ASSIGNMENT
 static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,
 						      int assigned_dev_id)
@@ -1184,6 +1186,10 @@ int __kvm_set_memory_region(struct kvm *
 		if ((base_gfn ^ ugfn) & (KVM_PAGES_PER_HPAGE - 1))
 			for (i = 0; i < largepages; ++i)
 				new.lpage_info[i].write_count = 1;
+
+		if (largepages_disabled)
+			for (i = 0; i < largepages; ++i)
+				new.lpage_info[i].write_count = 1;
 	}
 
 	/* Allocate page dirty bitmap if needed */
@@ -1294,6 +1300,12 @@ out:
 	return r;
 }
 
+void kvm_disable_largepages(void)
+{
+	largepages_disabled = true;
+}
+EXPORT_SYMBOL_GPL(kvm_disable_largepages);
+
 int is_error_page(struct page *page)
 {
 	return page == bad_page;

Re: [patch 0/5] VMX EPT misconfigurtion handler

[Yang, Sheng]

On Wednesday 10 June 2009 05:30:09 Marcelo Tosatti wrote:
> >From the Intel docs:
>
> An EPT misconfiguration occurs when, in the course of translation
> a guest-physical address, the logical processor encounters an EPT
> paging-structure entry that contains an unsupported value.
>
> Handle this event and print useful information for diagnostics.

Looks fine to me, thanks!

-- 
regards
Yang, Sheng

Re: [patch 2/5] KVM: MMU: make for_each_shadow_entry aware of largepages

[Avi Kivity]

Marcelo Tosatti wrote:
> This way there is no need to add explicit checks in every
> for_each_shadow_entry user.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> Index: kvm/arch/x86/kvm/mmu.c
> ===================================================================
> --- kvm.orig/arch/x86/kvm/mmu.c
> +++ kvm/arch/x86/kvm/mmu.c
> @@ -1273,6 +1273,11 @@ static bool shadow_walk_okay(struct kvm_
>  {
>  	if (iterator->level < PT_PAGE_TABLE_LEVEL)
>  		return false;
> +
> +	if (iterator->level == PT_PAGE_TABLE_LEVEL)
> +		if (is_large_pte(*iterator->sptep))
> +			return false;
>
>   
s/==/>/?

-- 
error compiling committee.c: too many arguments to function

Re: [patch 3/5] KVM: MMU: add kvm_mmu_shadow_walk helper

[Avi Kivity]

Marcelo Tosatti wrote:
> Required by EPT misconfiguration handler.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> Index: kvm/arch/x86/kvm/mmu.c
> ===================================================================
> --- kvm.orig/arch/x86/kvm/mmu.c
> +++ kvm/arch/x86/kvm/mmu.c
> @@ -3013,6 +3013,26 @@ out:
>  	return r;
>  }
>  
> +void kvm_mmu_shadow_walk(struct kvm_vcpu *vcpu, u64 addr,
> +			 struct mmu_shadow_walk *walk)
> +{
> +	struct kvm_shadow_walk_iterator iterator;
> +
> +	spin_lock(&vcpu->kvm->mmu_lock);
> +	for_each_shadow_entry(vcpu, addr, iterator) {
> +		int err;
> +
> +		err = walk->fn(vcpu, iterator.sptep, iterator.level, walk);
> +		if (err)
> +			break;
> +
> +		if (!is_shadow_present_pte(*iterator.sptep))
> +			break;
> +	}
> +	spin_unlock(&vcpu->kvm->mmu_lock);
> +}
> +EXPORT_SYMBOL(kvm_mmu_shadow_walk);
> +
>   

Isn't it simpler to invoke for_each_shadow_entry(), instead of defining 
a callback and calling it?

We had those callbacks once, then switched to for_each.


-- 
error compiling committee.c: too many arguments to function

Re: [patch 5/5] KVM: VMX: conditionally disable 2M pages

[Avi Kivity]

Marcelo Tosatti wrote:
> Disable usage of 2M pages if VMX_EPT_2MB_PAGE_BIT (bit 16) is clear
> in MSR_IA32_VMX_EPT_VPID_CAP and EPT is enabled.
>
> Index: kvm/virt/kvm/kvm_main.c
> ===================================================================
> --- kvm.orig/virt/kvm/kvm_main.c
> +++ kvm/virt/kvm/kvm_main.c
> @@ -85,6 +85,8 @@ static long kvm_vcpu_ioctl(struct file *
>  
>  static bool kvm_rebooting;
>  
> +static bool largepages_disabled = false;
> +
>  #ifdef KVM_CAP_DEVICE_ASSIGNMENT
>  static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,
>  						      int assigned_dev_id)
> @@ -1184,6 +1186,10 @@ int __kvm_set_memory_region(struct kvm *
>  		if ((base_gfn ^ ugfn) & (KVM_PAGES_PER_HPAGE - 1))
>  			for (i = 0; i < largepages; ++i)
>  				new.lpage_info[i].write_count = 1;
> +
> +		if (largepages_disabled)
> +			for (i = 0; i < largepages; ++i)
> +				new.lpage_info[i].write_count = 1;
>  	}
>   

Please reuse the loop above, it's exactly the same (think of 1GB pages).


-- 
error compiling committee.c: too many arguments to function

Re: [patch 2/5] KVM: MMU: make for_each_shadow_entry aware of largepages

[Avi Kivity]

Avi Kivity wrote:
> Marcelo Tosatti wrote:
>> This way there is no need to add explicit checks in every
>> for_each_shadow_entry user.
>>
>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>
>> Index: kvm/arch/x86/kvm/mmu.c
>> ===================================================================
>> --- kvm.orig/arch/x86/kvm/mmu.c
>> +++ kvm/arch/x86/kvm/mmu.c
>> @@ -1273,6 +1273,11 @@ static bool shadow_walk_okay(struct kvm_
>>  {
>>      if (iterator->level < PT_PAGE_TABLE_LEVEL)
>>          return false;
>> +
>> +    if (iterator->level == PT_PAGE_TABLE_LEVEL)
>> +        if (is_large_pte(*iterator->sptep))
>> +            return false;
>>
>>   
> s/==/>/?
>

Ah, it's actually fine.  But changing == to >= will make it 1GBpage-ready.

-- 
error compiling committee.c: too many arguments to function

Re: [patch 3/5] KVM: MMU: add kvm_mmu_shadow_walk helper

[Marcelo Tosatti]

On Wed, Jun 10, 2009 at 12:17:09PM +0300, Avi Kivity wrote:
> Marcelo Tosatti wrote:
>> Required by EPT misconfiguration handler.
>>
>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>
>> Index: kvm/arch/x86/kvm/mmu.c
>> ===================================================================
>> --- kvm.orig/arch/x86/kvm/mmu.c
>> +++ kvm/arch/x86/kvm/mmu.c
>> @@ -3013,6 +3013,26 @@ out:
>>  	return r;
>>  }
>>  +void kvm_mmu_shadow_walk(struct kvm_vcpu *vcpu, u64 addr,
>> +			 struct mmu_shadow_walk *walk)
>> +{
>> +	struct kvm_shadow_walk_iterator iterator;
>> +
>> +	spin_lock(&vcpu->kvm->mmu_lock);
>> +	for_each_shadow_entry(vcpu, addr, iterator) {
>> +		int err;
>> +
>> +		err = walk->fn(vcpu, iterator.sptep, iterator.level, walk);
>> +		if (err)
>> +			break;
>> +
>> +		if (!is_shadow_present_pte(*iterator.sptep))
>> +			break;
>> +	}
>> +	spin_unlock(&vcpu->kvm->mmu_lock);
>> +}
>> +EXPORT_SYMBOL(kvm_mmu_shadow_walk);
>> +
>>   
>
> Isn't it simpler to invoke for_each_shadow_entry(), instead of defining  
> a callback and calling it?
>
> We had those callbacks once, then switched to for_each.

The point is its exported to use in a external module (kvm-intel.ko),
so you hide the details (such as locking) in the kvm_mmu_shadow_walk
helper. Let me know how do you prefer this to be.

Re: [patch 3/5] KVM: MMU: add kvm_mmu_shadow_walk helper

[Avi Kivity]

Marcelo Tosatti wrote:
>> Isn't it simpler to invoke for_each_shadow_entry(), instead of defining  
>> a callback and calling it?
>>
>> We had those callbacks once, then switched to for_each.
>>     
>
> The point is its exported to use in a external module (kvm-intel.ko),
> so you hide the details (such as locking) in the kvm_mmu_shadow_walk
> helper. Let me know how do you prefer this to be.
>   

Ah, you're right.

I don't think it's worthwhile to add all this just for debugging.  You 
can add a function that dumps the spte chain as well as the features 
MSR, and we can decode it by hand when we see it.  Disadvantage is more 
work for us when we hit the bug, but at least that function is reusable 
in other contexts.

-- 
error compiling committee.c: too many arguments to function

Re: [patch 3/5] KVM: MMU: add kvm_mmu_shadow_walk helper

[Marcelo Tosatti]

On Wed, Jun 10, 2009 at 03:23:47PM +0300, Avi Kivity wrote:
> Marcelo Tosatti wrote:
>>> Isn't it simpler to invoke for_each_shadow_entry(), instead of 
>>> defining  a callback and calling it?
>>>
>>> We had those callbacks once, then switched to for_each.
>>>     
>>
>> The point is its exported to use in a external module (kvm-intel.ko),
>> so you hide the details (such as locking) in the kvm_mmu_shadow_walk
>> helper. Let me know how do you prefer this to be.
>>   
>
> Ah, you're right.
>
> I don't think it's worthwhile to add all this just for debugging.  You  
> can add a function that dumps the spte chain as well as the features  
> MSR, and we can decode it by hand when we see it.  Disadvantage is more  
> work for us when we hit the bug, but at least that function is reusable  
> in other contexts.

The problem is if someone hits the bug who has no possibility of
reporting the printks. Nice thing about the WARN_ON's there is you can
look up kerneloops.org, match the line number to the kernel version,
and narrow down what bits are wrong (you still need reporter to send
contents of dmesg for full spte value).

Also the bit-by-bit validity checks (the inspect_spte function in vmx.c)
can be used in the mmu audit code (thats the reason for print=0 in the
callback parameters), so it is reusable in other contexes.

What you dislike is hardcoding the bits checked in C code? Don't worry
about the level stuff, will be handled next.

Re: [patch 3/5] KVM: MMU: add kvm_mmu_shadow_walk helper

[Avi Kivity]

Marcelo Tosatti wrote:

  

>> Ah, you're right.
>>
>> I don't think it's worthwhile to add all this just for debugging.  You  
>> can add a function that dumps the spte chain as well as the features  
>> MSR, and we can decode it by hand when we see it.  Disadvantage is more  
>> work for us when we hit the bug, but at least that function is reusable  
>> in other contexts.
>>     
>
> The problem is if someone hits the bug who has no possibility of
> reporting the printks. Nice thing about the WARN_ON's there is you can
> look up kerneloops.org, match the line number to the kernel version,
> and narrow down what bits are wrong (you still need reporter to send
> contents of dmesg for full spte value).
>   

Well, we can KERN_WARNING instead of KERN_DEBUG in that printout.

> Also the bit-by-bit validity checks (the inspect_spte function in vmx.c)
> can be used in the mmu audit code (thats the reason for print=0 in the
> callback parameters), so it is reusable in other contexes.
>
> What you dislike is hardcoding the bits checked in C code? Don't worry
> about the level stuff, will be handled next.
>   

I just don't want to introduce yet another function-callback pair for 
such a small thing.  No objection to the code itself.

Printing out the spte hierarchy seems a good idea in other bug contexts, 
so at least that function is reusable.

-- 
error compiling committee.c: too many arguments to function

Re: [patch 3/5] KVM: MMU: add kvm_mmu_shadow_walk helper

[Avi Kivity]

On 06/10/2009 06:24 PM, Avi Kivity wrote:
>
> I just don't want to introduce yet another function-callback pair for 
> such a small thing.  No objection to the code itself.
>
> Printing out the spte hierarchy seems a good idea in other bug 
> contexts, so at least that function is reusable.
>

Here's one that preserves all information but is simple to use:

    kvm_mmu_get_spte_hierarchy(vcpu, gva, int *nr_sptes, u64 sptes[4])

So you can later do detailed checks on the sptes, but it doesn't expose 
yet another generic walker.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [patch 2/5] KVM: MMU: make for_each_shadow_entry aware of largepages

[Marcelo Tosatti]

On Wed, Jun 10, 2009 at 12:21:05PM +0300, Avi Kivity wrote:
> Avi Kivity wrote:
>> Marcelo Tosatti wrote:
>>> This way there is no need to add explicit checks in every
>>> for_each_shadow_entry user.
>>>
>>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>>
>>> Index: kvm/arch/x86/kvm/mmu.c
>>> ===================================================================
>>> --- kvm.orig/arch/x86/kvm/mmu.c
>>> +++ kvm/arch/x86/kvm/mmu.c
>>> @@ -1273,6 +1273,11 @@ static bool shadow_walk_okay(struct kvm_
>>>  {
>>>      if (iterator->level < PT_PAGE_TABLE_LEVEL)
>>>          return false;
>>> +
>>> +    if (iterator->level == PT_PAGE_TABLE_LEVEL)
>>> +        if (is_large_pte(*iterator->sptep))
>>> +            return false;
>>>
>>>   
>> s/==/>/?
>>
>
> Ah, it's actually fine.  But changing == to >= will make it 1GBpage-ready.

Humpf, better check level explicitly before interpreting bit 7, so lets 
skip this for 1GB pages.

[patch 0/5] VMX EPT misconfiguration handler v2

[Marcelo Tosatti]

Addressing comments.

-- 

[patch 1/5] KVM: VMX: more MSR_IA32_VMX_EPT_VPID_CAP capability bits

[Marcelo Tosatti]

[-- Attachment #1: vmx-ept-check-supported-bits --]
[-- Type: text/plain, Size: 1767 bytes --]

Required for EPT misconfiguration handler.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/include/asm/vmx.h
===================================================================
--- kvm.orig/arch/x86/include/asm/vmx.h
+++ kvm/arch/x86/include/asm/vmx.h
@@ -352,9 +352,16 @@ enum vmcs_field {
 #define VMX_EPT_EXTENT_INDIVIDUAL_ADDR		0
 #define VMX_EPT_EXTENT_CONTEXT			1
 #define VMX_EPT_EXTENT_GLOBAL			2
+
+#define VMX_EPT_EXECUTE_ONLY_BIT		(1ull)
+#define VMX_EPT_PAGE_WALK_4_BIT			(1ull << 6)
+#define VMX_EPTP_UC_BIT				(1ull << 8)
+#define VMX_EPTP_WB_BIT				(1ull << 14)
+#define VMX_EPT_2MB_PAGE_BIT			(1ull << 16)
 #define VMX_EPT_EXTENT_INDIVIDUAL_BIT		(1ull << 24)
 #define VMX_EPT_EXTENT_CONTEXT_BIT		(1ull << 25)
 #define VMX_EPT_EXTENT_GLOBAL_BIT		(1ull << 26)
+
 #define VMX_EPT_DEFAULT_GAW			3
 #define VMX_EPT_MAX_GAW				0x4
 #define VMX_EPT_MT_EPTE_SHIFT			3
Index: kvm/arch/x86/kvm/vmx.c
===================================================================
--- kvm.orig/arch/x86/kvm/vmx.c
+++ kvm/arch/x86/kvm/vmx.c
@@ -270,6 +270,26 @@ static inline bool cpu_has_vmx_flexprior
 		cpu_has_vmx_virtualize_apic_accesses();
 }
 
+static inline bool cpu_has_vmx_ept_execute_only(void)
+{
+	return !!(vmx_capability.ept & VMX_EPT_EXECUTE_ONLY_BIT);
+}
+
+static inline bool cpu_has_vmx_eptp_uncacheable(void)
+{
+	return !!(vmx_capability.ept & VMX_EPTP_UC_BIT);
+}
+
+static inline bool cpu_has_vmx_eptp_writeback(void)
+{
+	return !!(vmx_capability.ept & VMX_EPTP_WB_BIT);
+}
+
+static inline bool cpu_has_vmx_ept_2m_page(void)
+{
+	return !!(vmx_capability.ept & VMX_EPT_2MB_PAGE_BIT);
+}
+
 static inline int cpu_has_vmx_invept_individual_addr(void)
 {
 	return !!(vmx_capability.ept & VMX_EPT_EXTENT_INDIVIDUAL_BIT);

-- 

[patch 2/5] KVM: MMU: make for_each_shadow_entry aware of largepages

[Marcelo Tosatti]

[-- Attachment #1: kvm-shadow-iterator-largepage-descend --]
[-- Type: text/plain, Size: 701 bytes --]

This way there is no need to add explicit checks in every
for_each_shadow_entry user.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/mmu.c
===================================================================
--- kvm.orig/arch/x86/kvm/mmu.c
+++ kvm/arch/x86/kvm/mmu.c
@@ -1273,6 +1273,11 @@ static bool shadow_walk_okay(struct kvm_
 {
 	if (iterator->level < PT_PAGE_TABLE_LEVEL)
 		return false;
+
+	if (iterator->level == PT_PAGE_TABLE_LEVEL)
+		if (is_large_pte(*iterator->sptep))
+			return false;
+
 	iterator->index = SHADOW_PT_INDEX(iterator->addr, iterator->level);
 	iterator->sptep	= ((u64 *)__va(iterator->shadow_addr)) + iterator->index;
 	return true;

-- 

[patch 3/5] KVM: MMU: add kvm_mmu_get_spte_hierarchy helper

[Marcelo Tosatti]

[-- Attachment #1: kvm_mmu_walk_shadow --]
[-- Type: text/plain, Size: 1278 bytes --]

Required by EPT misconfiguration handler.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/mmu.c
===================================================================
--- kvm.orig/arch/x86/kvm/mmu.c
+++ kvm/arch/x86/kvm/mmu.c
@@ -3013,6 +3013,24 @@ out:
 	return r;
 }
 
+int kvm_mmu_get_spte_hierarchy(struct kvm_vcpu *vcpu, u64 addr, u64 *sptes[4])
+{
+	struct kvm_shadow_walk_iterator iterator;
+	int nr_sptes = 0;
+
+	spin_lock(&vcpu->kvm->mmu_lock);
+	for_each_shadow_entry(vcpu, addr, iterator) {
+		sptes[iterator.level-1] = iterator.sptep;
+		nr_sptes++;
+		if (!is_shadow_present_pte(*iterator.sptep))
+			break;
+	}
+	spin_unlock(&vcpu->kvm->mmu_lock);
+
+	return nr_sptes;
+}
+EXPORT_SYMBOL_GPL(kvm_mmu_get_spte_hierarchy);
+
 #ifdef AUDIT
 
 static const char *audit_msg;
Index: kvm/arch/x86/kvm/mmu.h
===================================================================
--- kvm.orig/arch/x86/kvm/mmu.h
+++ kvm/arch/x86/kvm/mmu.h
@@ -37,6 +37,8 @@
 #define PT32_ROOT_LEVEL 2
 #define PT32E_ROOT_LEVEL 3
 
+int kvm_mmu_get_spte_hierarchy(struct kvm_vcpu *vcpu, u64 addr, u64 *sptes[4]);
+
 static inline void kvm_mmu_free_some_pages(struct kvm_vcpu *vcpu)
 {
 	if (unlikely(vcpu->kvm->arch.n_free_mmu_pages < KVM_MIN_FREE_MMU_PAGES))

-- 

[patch 4/5] KVM: VMX: EPT misconfiguration handler

[Marcelo Tosatti]

[-- Attachment #1: vmx-misconfig-handler --]
[-- Type: text/plain, Size: 3179 bytes --]

Handler for EPT misconfiguration which checks for valid state 
in the shadow pagetables, printing the spte on each level.

The separate WARN_ONs are useful for kerneloops.org.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/vmx.c
===================================================================
--- kvm.orig/arch/x86/kvm/vmx.c
+++ kvm/arch/x86/kvm/vmx.c
@@ -3233,6 +3233,90 @@ static int handle_ept_violation(struct k
 	return kvm_mmu_page_fault(vcpu, gpa & PAGE_MASK, 0);
 }
 
+static u64 ept_rsvd_mask(u64 *sptep, int level)
+{
+	int i;
+	u64 mask = 0;
+
+	for (i = 51; i > boot_cpu_data.x86_phys_bits; i--)
+		mask |= (1ULL << i);
+
+	if (level > 2)
+		/* bits 7:3 reserved */
+		mask |= 0xf8;
+	else if (level == 2) {
+		if (*sptep & (1ULL << 7))
+			/* 2MB ref, bits 20:12 reserved */
+			mask |= 0x1ff000;
+		else
+			/* bits 6:3 reserved */
+			mask |= 0x78;
+	}
+
+	return mask;
+}
+
+static void ept_misconfig_inspect_spte(struct kvm_vcpu *vcpu, u64 *sptep,
+				       int level)
+{
+	printk(KERN_ERR "%s: sptep %p spte 0x%llx level %d\n",
+			__func__, sptep, *sptep, level);
+
+	/* 010b (write-only) */
+	WARN_ON((*sptep & 0x7) == 0x2);
+
+	/* 110b (write/execute) */
+	WARN_ON((*sptep & 0x7) == 0x6);
+
+	/* 100b (execute-only) and value not supported by logical processor */
+	if (!cpu_has_vmx_ept_execute_only())
+		WARN_ON((*sptep & 0x7) == 0x4);
+
+	/* not 000b */
+	if ((*sptep & 0x7)) {
+		u64 rsvd_bits = *sptep & ept_rsvd_mask(sptep, level);
+
+		if (rsvd_bits != 0) {
+			printk(KERN_ERR "%s: rsvd_bits = 0x%llx\n",
+					 __func__, rsvd_bits);
+			WARN_ON(1);
+		}
+
+		if (level == 1 || (level == 2 && (*sptep & (1ULL << 7)))) {
+			u64 ept_mem_type = (*sptep & 0x38) >> 3;
+
+			if (ept_mem_type == 2 || ept_mem_type == 3 ||
+			    ept_mem_type == 7) {
+				printk(KERN_ERR "%s: ept_mem_type=0x%llx\n",
+						__func__, ept_mem_type);
+				WARN_ON(1);
+			}
+		}
+	}
+}
+
+static int handle_ept_misconfig(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
+{
+	u64 *sptes[4];
+	int nr_sptes, i;
+	gpa_t gpa;
+
+	gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
+
+	printk(KERN_ERR "EPT: Misconfiguration.\n");
+	printk(KERN_ERR "EPT: GPA: 0x%llx\n", gpa);
+
+	nr_sptes = kvm_mmu_get_spte_hierarchy(vcpu, gpa, sptes);
+
+	for (i = PT64_ROOT_LEVEL; i > PT64_ROOT_LEVEL - nr_sptes; --i)
+		ept_misconfig_inspect_spte(vcpu, sptes[i-1], i);
+
+	kvm_run->exit_reason = KVM_EXIT_UNKNOWN;
+	kvm_run->hw.hardware_exit_reason = EXIT_REASON_EPT_MISCONFIG;
+
+	return 0;
+}
+
 static int handle_nmi_window(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 {
 	u32 cpu_based_vm_exec_control;
@@ -3303,8 +3387,9 @@ static int (*kvm_vmx_exit_handlers[])(st
 	[EXIT_REASON_APIC_ACCESS]             = handle_apic_access,
 	[EXIT_REASON_WBINVD]                  = handle_wbinvd,
 	[EXIT_REASON_TASK_SWITCH]             = handle_task_switch,
-	[EXIT_REASON_EPT_VIOLATION]	      = handle_ept_violation,
 	[EXIT_REASON_MCE_DURING_VMENTRY]      = handle_machine_check,
+	[EXIT_REASON_EPT_VIOLATION]	      = handle_ept_violation,
+	[EXIT_REASON_EPT_MISCONFIG]           = handle_ept_misconfig,
 };
 
 static const int kvm_vmx_max_exit_handlers =

-- 

[patch 5/5] KVM: VMX: conditionally disable 2M pages

[Marcelo Tosatti]

[-- Attachment #1: disable-2m-pages-vmx --]
[-- Type: text/plain, Size: 2333 bytes --]

Disable usage of 2M pages if VMX_EPT_2MB_PAGE_BIT (bit 16) is clear
in MSR_IA32_VMX_EPT_VPID_CAP and EPT is enabled.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/vmx.c
===================================================================
--- kvm.orig/arch/x86/kvm/vmx.c
+++ kvm/arch/x86/kvm/vmx.c
@@ -1393,6 +1393,9 @@ static __init int hardware_setup(void)
 	if (!cpu_has_vmx_tpr_shadow())
 		kvm_x86_ops->update_cr8_intercept = NULL;
 
+	if (enable_ept && !cpu_has_vmx_ept_2m_page())
+		kvm_disable_largepages();
+
 	return alloc_kvm_area();
 }
 
Index: kvm/include/linux/kvm_host.h
===================================================================
--- kvm.orig/include/linux/kvm_host.h
+++ kvm/include/linux/kvm_host.h
@@ -219,6 +219,7 @@ int kvm_arch_set_memory_region(struct kv
 				struct kvm_userspace_memory_region *mem,
 				struct kvm_memory_slot old,
 				int user_alloc);
+void kvm_disable_largepages(void);
 void kvm_arch_flush_shadow(struct kvm *kvm);
 gfn_t unalias_gfn(struct kvm *kvm, gfn_t gfn);
 struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn);
Index: kvm/virt/kvm/kvm_main.c
===================================================================
--- kvm.orig/virt/kvm/kvm_main.c
+++ kvm/virt/kvm/kvm_main.c
@@ -85,6 +85,8 @@ static long kvm_vcpu_ioctl(struct file *
 
 static bool kvm_rebooting;
 
+static bool largepages_disabled = false;
+
 #ifdef KVM_CAP_DEVICE_ASSIGNMENT
 static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,
 						      int assigned_dev_id)
@@ -1171,9 +1173,11 @@ int __kvm_set_memory_region(struct kvm *
 		ugfn = new.userspace_addr >> PAGE_SHIFT;
 		/*
 		 * If the gfn and userspace address are not aligned wrt each
-		 * other, disable large page support for this slot
+		 * other, or if explicitly asked to, disable large page
+		 * support for this slot
 		 */
-		if ((base_gfn ^ ugfn) & (KVM_PAGES_PER_HPAGE - 1))
+		if ((base_gfn ^ ugfn) & (KVM_PAGES_PER_HPAGE - 1) ||
+		    largepages_disabled)
 			for (i = 0; i < largepages; ++i)
 				new.lpage_info[i].write_count = 1;
 	}
@@ -1286,6 +1290,12 @@ out:
 	return r;
 }
 
+void kvm_disable_largepages(void)
+{
+	largepages_disabled = true;
+}
+EXPORT_SYMBOL_GPL(kvm_disable_largepages);
+
 int is_error_page(struct page *page)
 {
 	return page == bad_page;

-- 

Re: [patch 2/5] KVM: MMU: make for_each_shadow_entry aware of largepages

[Avi Kivity]

Marcelo Tosatti wrote:
>>>> @@ -1273,6 +1273,11 @@ static bool shadow_walk_okay(struct kvm_
>>>>  {
>>>>      if (iterator->level < PT_PAGE_TABLE_LEVEL)
>>>>          return false;
>>>> +
>>>> +    if (iterator->level == PT_PAGE_TABLE_LEVEL)
>>>> +        if (is_large_pte(*iterator->sptep))
>>>> +            return false;
>>>>
>>>>   
>>>>         
>>> s/==/>/?
>>>
>>>       
>> Ah, it's actually fine.  But changing == to >= will make it 1GBpage-ready.
>>     
>
> Humpf, better check level explicitly before interpreting bit 7, so lets 
> skip this for 1GB pages.
>
>   

Okay.  But I'm rewriting shadow_walk_* afterwards.

-- 
error compiling committee.c: too many arguments to function

Re: [patch 3/5] KVM: MMU: add kvm_mmu_get_spte_hierarchy helper

[Avi Kivity]

Marcelo Tosatti wrote:
> Required by EPT misconfiguration handler.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> Index: kvm/arch/x86/kvm/mmu.c
> ===================================================================
> --- kvm.orig/arch/x86/kvm/mmu.c
> +++ kvm/arch/x86/kvm/mmu.c
> @@ -3013,6 +3013,24 @@ out:
>  	return r;
>  }
>  
> +int kvm_mmu_get_spte_hierarchy(struct kvm_vcpu *vcpu, u64 addr, u64 *sptes[4])
> +{
> +	struct kvm_shadow_walk_iterator iterator;
> +	int nr_sptes = 0;
> +
> +	spin_lock(&vcpu->kvm->mmu_lock);
> +	for_each_shadow_entry(vcpu, addr, iterator) {
> +		sptes[iterator.level-1] = iterator.sptep;
>   

Returning a pointer...

> +		nr_sptes++;
> +		if (!is_shadow_present_pte(*iterator.sptep))
> +			break;
> +	}
> +	spin_unlock(&vcpu->kvm->mmu_lock);
>   

... and unlocking the lock that protects it.

True, this is called in extreme cases, but I think you can dereference 
the pointer in the function just as easily.


-- 
error compiling committee.c: too many arguments to function

[patch 0/5] VMX EPT misconfiguration handler v3

[Marcelo Tosatti]

Addressing comments.


-- 

[patch 1/5] KVM: VMX: more MSR_IA32_VMX_EPT_VPID_CAP capability bits

[Marcelo Tosatti]

[-- Attachment #1: vmx-ept-check-supported-bits --]
[-- Type: text/plain, Size: 1767 bytes --]

Required for EPT misconfiguration handler.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/include/asm/vmx.h
===================================================================
--- kvm.orig/arch/x86/include/asm/vmx.h
+++ kvm/arch/x86/include/asm/vmx.h
@@ -352,9 +352,16 @@ enum vmcs_field {
 #define VMX_EPT_EXTENT_INDIVIDUAL_ADDR		0
 #define VMX_EPT_EXTENT_CONTEXT			1
 #define VMX_EPT_EXTENT_GLOBAL			2
+
+#define VMX_EPT_EXECUTE_ONLY_BIT		(1ull)
+#define VMX_EPT_PAGE_WALK_4_BIT			(1ull << 6)
+#define VMX_EPTP_UC_BIT				(1ull << 8)
+#define VMX_EPTP_WB_BIT				(1ull << 14)
+#define VMX_EPT_2MB_PAGE_BIT			(1ull << 16)
 #define VMX_EPT_EXTENT_INDIVIDUAL_BIT		(1ull << 24)
 #define VMX_EPT_EXTENT_CONTEXT_BIT		(1ull << 25)
 #define VMX_EPT_EXTENT_GLOBAL_BIT		(1ull << 26)
+
 #define VMX_EPT_DEFAULT_GAW			3
 #define VMX_EPT_MAX_GAW				0x4
 #define VMX_EPT_MT_EPTE_SHIFT			3
Index: kvm/arch/x86/kvm/vmx.c
===================================================================
--- kvm.orig/arch/x86/kvm/vmx.c
+++ kvm/arch/x86/kvm/vmx.c
@@ -270,6 +270,26 @@ static inline bool cpu_has_vmx_flexprior
 		cpu_has_vmx_virtualize_apic_accesses();
 }
 
+static inline bool cpu_has_vmx_ept_execute_only(void)
+{
+	return !!(vmx_capability.ept & VMX_EPT_EXECUTE_ONLY_BIT);
+}
+
+static inline bool cpu_has_vmx_eptp_uncacheable(void)
+{
+	return !!(vmx_capability.ept & VMX_EPTP_UC_BIT);
+}
+
+static inline bool cpu_has_vmx_eptp_writeback(void)
+{
+	return !!(vmx_capability.ept & VMX_EPTP_WB_BIT);
+}
+
+static inline bool cpu_has_vmx_ept_2m_page(void)
+{
+	return !!(vmx_capability.ept & VMX_EPT_2MB_PAGE_BIT);
+}
+
 static inline int cpu_has_vmx_invept_individual_addr(void)
 {
 	return !!(vmx_capability.ept & VMX_EPT_EXTENT_INDIVIDUAL_BIT);

-- 

[patch 2/5] KVM: MMU: make for_each_shadow_entry aware of largepages

[Marcelo Tosatti]

[-- Attachment #1: kvm-shadow-iterator-largepage-descend --]
[-- Type: text/plain, Size: 701 bytes --]

This way there is no need to add explicit checks in every
for_each_shadow_entry user.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/mmu.c
===================================================================
--- kvm.orig/arch/x86/kvm/mmu.c
+++ kvm/arch/x86/kvm/mmu.c
@@ -1273,6 +1273,11 @@ static bool shadow_walk_okay(struct kvm_
 {
 	if (iterator->level < PT_PAGE_TABLE_LEVEL)
 		return false;
+
+	if (iterator->level == PT_PAGE_TABLE_LEVEL)
+		if (is_large_pte(*iterator->sptep))
+			return false;
+
 	iterator->index = SHADOW_PT_INDEX(iterator->addr, iterator->level);
 	iterator->sptep	= ((u64 *)__va(iterator->shadow_addr)) + iterator->index;
 	return true;

-- 

[patch 3/5] KVM: MMU: add kvm_mmu_get_spte_hierarchy helper

[Marcelo Tosatti]

[-- Attachment #1: kvm_mmu_walk_shadow --]
[-- Type: text/plain, Size: 1277 bytes --]

Required by EPT misconfiguration handler.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/mmu.c
===================================================================
--- kvm.orig/arch/x86/kvm/mmu.c
+++ kvm/arch/x86/kvm/mmu.c
@@ -3013,6 +3013,24 @@ out:
 	return r;
 }
 
+int kvm_mmu_get_spte_hierarchy(struct kvm_vcpu *vcpu, u64 addr, u64 sptes[4])
+{
+	struct kvm_shadow_walk_iterator iterator;
+	int nr_sptes = 0;
+
+	spin_lock(&vcpu->kvm->mmu_lock);
+	for_each_shadow_entry(vcpu, addr, iterator) {
+		sptes[iterator.level-1] = *iterator.sptep;
+		nr_sptes++;
+		if (!is_shadow_present_pte(*iterator.sptep))
+			break;
+	}
+	spin_unlock(&vcpu->kvm->mmu_lock);
+
+	return nr_sptes;
+}
+EXPORT_SYMBOL_GPL(kvm_mmu_get_spte_hierarchy);
+
 #ifdef AUDIT
 
 static const char *audit_msg;
Index: kvm/arch/x86/kvm/mmu.h
===================================================================
--- kvm.orig/arch/x86/kvm/mmu.h
+++ kvm/arch/x86/kvm/mmu.h
@@ -37,6 +37,8 @@
 #define PT32_ROOT_LEVEL 2
 #define PT32E_ROOT_LEVEL 3
 
+int kvm_mmu_get_spte_hierarchy(struct kvm_vcpu *vcpu, u64 addr, u64 sptes[4]);
+
 static inline void kvm_mmu_free_some_pages(struct kvm_vcpu *vcpu)
 {
 	if (unlikely(vcpu->kvm->arch.n_free_mmu_pages < KVM_MIN_FREE_MMU_PAGES))

-- 

[patch 4/5] KVM: VMX: EPT misconfiguration handler

[Marcelo Tosatti]

[-- Attachment #1: vmx-misconfig-handler --]
[-- Type: text/plain, Size: 3135 bytes --]

Handler for EPT misconfiguration which checks for valid state 
in the shadow pagetables, printing the spte on each level.

The separate WARN_ONs are useful for kerneloops.org.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/vmx.c
===================================================================
--- kvm.orig/arch/x86/kvm/vmx.c
+++ kvm/arch/x86/kvm/vmx.c
@@ -3233,6 +3233,89 @@ static int handle_ept_violation(struct k
 	return kvm_mmu_page_fault(vcpu, gpa & PAGE_MASK, 0);
 }
 
+static u64 ept_rsvd_mask(u64 spte, int level)
+{
+	int i;
+	u64 mask = 0;
+
+	for (i = 51; i > boot_cpu_data.x86_phys_bits; i--)
+		mask |= (1ULL << i);
+
+	if (level > 2)
+		/* bits 7:3 reserved */
+		mask |= 0xf8;
+	else if (level == 2) {
+		if (spte & (1ULL << 7))
+			/* 2MB ref, bits 20:12 reserved */
+			mask |= 0x1ff000;
+		else
+			/* bits 6:3 reserved */
+			mask |= 0x78;
+	}
+
+	return mask;
+}
+
+static void ept_misconfig_inspect_spte(struct kvm_vcpu *vcpu, u64 spte,
+				       int level)
+{
+	printk(KERN_ERR "%s: spte 0x%llx level %d\n", __func__, spte, level);
+
+	/* 010b (write-only) */
+	WARN_ON((spte & 0x7) == 0x2);
+
+	/* 110b (write/execute) */
+	WARN_ON((spte & 0x7) == 0x6);
+
+	/* 100b (execute-only) and value not supported by logical processor */
+	if (!cpu_has_vmx_ept_execute_only())
+		WARN_ON((spte & 0x7) == 0x4);
+
+	/* not 000b */
+	if ((spte & 0x7)) {
+		u64 rsvd_bits = spte & ept_rsvd_mask(spte, level);
+
+		if (rsvd_bits != 0) {
+			printk(KERN_ERR "%s: rsvd_bits = 0x%llx\n",
+					 __func__, rsvd_bits);
+			WARN_ON(1);
+		}
+
+		if (level == 1 || (level == 2 && (spte & (1ULL << 7)))) {
+			u64 ept_mem_type = (spte & 0x38) >> 3;
+
+			if (ept_mem_type == 2 || ept_mem_type == 3 ||
+			    ept_mem_type == 7) {
+				printk(KERN_ERR "%s: ept_mem_type=0x%llx\n",
+						__func__, ept_mem_type);
+				WARN_ON(1);
+			}
+		}
+	}
+}
+
+static int handle_ept_misconfig(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
+{
+	u64 sptes[4];
+	int nr_sptes, i;
+	gpa_t gpa;
+
+	gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
+
+	printk(KERN_ERR "EPT: Misconfiguration.\n");
+	printk(KERN_ERR "EPT: GPA: 0x%llx\n", gpa);
+
+	nr_sptes = kvm_mmu_get_spte_hierarchy(vcpu, gpa, sptes);
+
+	for (i = PT64_ROOT_LEVEL; i > PT64_ROOT_LEVEL - nr_sptes; --i)
+		ept_misconfig_inspect_spte(vcpu, sptes[i-1], i);
+
+	kvm_run->exit_reason = KVM_EXIT_UNKNOWN;
+	kvm_run->hw.hardware_exit_reason = EXIT_REASON_EPT_MISCONFIG;
+
+	return 0;
+}
+
 static int handle_nmi_window(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 {
 	u32 cpu_based_vm_exec_control;
@@ -3303,8 +3386,9 @@ static int (*kvm_vmx_exit_handlers[])(st
 	[EXIT_REASON_APIC_ACCESS]             = handle_apic_access,
 	[EXIT_REASON_WBINVD]                  = handle_wbinvd,
 	[EXIT_REASON_TASK_SWITCH]             = handle_task_switch,
-	[EXIT_REASON_EPT_VIOLATION]	      = handle_ept_violation,
 	[EXIT_REASON_MCE_DURING_VMENTRY]      = handle_machine_check,
+	[EXIT_REASON_EPT_VIOLATION]	      = handle_ept_violation,
+	[EXIT_REASON_EPT_MISCONFIG]           = handle_ept_misconfig,
 };
 
 static const int kvm_vmx_max_exit_handlers =

-- 

[patch 5/5] KVM: VMX: conditionally disable 2M pages

[Marcelo Tosatti]

[-- Attachment #1: disable-2m-pages-vmx --]
[-- Type: text/plain, Size: 2333 bytes --]

Disable usage of 2M pages if VMX_EPT_2MB_PAGE_BIT (bit 16) is clear
in MSR_IA32_VMX_EPT_VPID_CAP and EPT is enabled.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/vmx.c
===================================================================
--- kvm.orig/arch/x86/kvm/vmx.c
+++ kvm/arch/x86/kvm/vmx.c
@@ -1393,6 +1393,9 @@ static __init int hardware_setup(void)
 	if (!cpu_has_vmx_tpr_shadow())
 		kvm_x86_ops->update_cr8_intercept = NULL;
 
+	if (enable_ept && !cpu_has_vmx_ept_2m_page())
+		kvm_disable_largepages();
+
 	return alloc_kvm_area();
 }
 
Index: kvm/include/linux/kvm_host.h
===================================================================
--- kvm.orig/include/linux/kvm_host.h
+++ kvm/include/linux/kvm_host.h
@@ -219,6 +219,7 @@ int kvm_arch_set_memory_region(struct kv
 				struct kvm_userspace_memory_region *mem,
 				struct kvm_memory_slot old,
 				int user_alloc);
+void kvm_disable_largepages(void);
 void kvm_arch_flush_shadow(struct kvm *kvm);
 gfn_t unalias_gfn(struct kvm *kvm, gfn_t gfn);
 struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn);
Index: kvm/virt/kvm/kvm_main.c
===================================================================
--- kvm.orig/virt/kvm/kvm_main.c
+++ kvm/virt/kvm/kvm_main.c
@@ -85,6 +85,8 @@ static long kvm_vcpu_ioctl(struct file *
 
 static bool kvm_rebooting;
 
+static bool largepages_disabled = false;
+
 #ifdef KVM_CAP_DEVICE_ASSIGNMENT
 static struct kvm_assigned_dev_kernel *kvm_find_assigned_dev(struct list_head *head,
 						      int assigned_dev_id)
@@ -1171,9 +1173,11 @@ int __kvm_set_memory_region(struct kvm *
 		ugfn = new.userspace_addr >> PAGE_SHIFT;
 		/*
 		 * If the gfn and userspace address are not aligned wrt each
-		 * other, disable large page support for this slot
+		 * other, or if explicitly asked to, disable large page
+		 * support for this slot
 		 */
-		if ((base_gfn ^ ugfn) & (KVM_PAGES_PER_HPAGE - 1))
+		if ((base_gfn ^ ugfn) & (KVM_PAGES_PER_HPAGE - 1) ||
+		    largepages_disabled)
 			for (i = 0; i < largepages; ++i)
 				new.lpage_info[i].write_count = 1;
 	}
@@ -1286,6 +1290,12 @@ out:
 	return r;
 }
 
+void kvm_disable_largepages(void)
+{
+	largepages_disabled = true;
+}
+EXPORT_SYMBOL_GPL(kvm_disable_largepages);
+
 int is_error_page(struct page *page)
 {
 	return page == bad_page;

-- 

Re: [patch 0/5] VMX EPT misconfiguration handler v3

[Avi Kivity]

Marcelo Tosatti wrote:
> Addressing comments.
>
>
>   

Applied all, thanks.

-- 
error compiling committee.c: too many arguments to function