[PATCH v2 0/2] KVM: VMX: require EPTP WB (write-back) support

[PATCH v2 0/2] KVM: VMX: require EPTP WB (write-back) support

[David Hildenbrand]

This version should no longer result in a panic :(

(note to self: always retest after tiny changes)

v1 -> v2:
- fix typo (invalid guest page-walk length)

David Hildenbrand (2):
  KVM: VMX: cleanup EPTP definitions
  KVM: VMX: always require WB memory type for EPT

 arch/x86/include/asm/vmx.h | 11 ++++++-----
 arch/x86/kvm/vmx.c         | 34 ++++++++++++++++++----------------
 2 files changed, 24 insertions(+), 21 deletions(-)

-- 
2.9.4

[PATCH v2 1/2] KVM: VMX: cleanup EPTP definitions

[David Hildenbrand]

Don't use shifts, tag them correctly as EPTP and use better matching
names (PWL vs. GAW).

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/include/asm/vmx.h | 11 ++++++-----
 arch/x86/kvm/vmx.c         | 25 +++++++++++--------------
 2 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 5f63a2e..340007a 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -468,12 +468,13 @@ enum vmcs_field {
 #define VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT      (1ull << 10) /* (42 - 32) */
 #define VMX_VPID_EXTENT_SINGLE_NON_GLOBAL_BIT   (1ull << 11) /* (43 - 32) */
 
-#define VMX_EPT_DEFAULT_GAW			3
-#define VMX_EPT_MAX_GAW				0x4
 #define VMX_EPT_MT_EPTE_SHIFT			3
-#define VMX_EPT_GAW_EPTP_SHIFT			3
-#define VMX_EPT_AD_ENABLE_BIT			(1ull << 6)
-#define VMX_EPT_DEFAULT_MT			0x6ull
+#define VMX_EPTP_PWL_MASK			0x38ull
+#define VMX_EPTP_PWL_4				0x18ull
+#define VMX_EPTP_AD_ENABLE_BIT			(1ull << 6)
+#define VMX_EPTP_MT_MASK			0x7ull
+#define VMX_EPTP_MT_WB				0x6ull
+#define VMX_EPTP_MT_UC				0x0ull
 #define VMX_EPT_READABLE_MASK			0x1ull
 #define VMX_EPT_WRITABLE_MASK			0x2ull
 #define VMX_EPT_EXECUTABLE_MASK			0x4ull
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 4658c27..0dd9c65 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4298,14 +4298,12 @@ static void vmx_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 
 static u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa)
 {
-	u64 eptp;
+	u64 eptp = VMX_EPTP_MT_WB | VMX_EPTP_PWL_4;
 
 	/* TODO write the value reading from MSR */
-	eptp = VMX_EPT_DEFAULT_MT |
-		VMX_EPT_DEFAULT_GAW << VMX_EPT_GAW_EPTP_SHIFT;
 	if (enable_ept_ad_bits &&
 	    (!is_guest_mode(vcpu) || nested_ept_ad_enabled(vcpu)))
-		eptp |= VMX_EPT_AD_ENABLE_BIT;
+		eptp |= VMX_EPTP_AD_ENABLE_BIT;
 	eptp |= (root_hpa & PAGE_MASK);
 
 	return eptp;
@@ -7889,16 +7887,15 @@ static int handle_preemption_timer(struct kvm_vcpu *vcpu)
 static bool valid_ept_address(struct kvm_vcpu *vcpu, u64 address)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
-	u64 mask = address & 0x7;
 	int maxphyaddr = cpuid_maxphyaddr(vcpu);
 
 	/* Check for memory type validity */
-	switch (mask) {
-	case 0:
+	switch (address & VMX_EPTP_MT_MASK) {
+	case VMX_EPTP_MT_UC:
 		if (!(vmx->nested.nested_vmx_ept_caps & VMX_EPTP_UC_BIT))
 			return false;
 		break;
-	case 6:
+	case VMX_EPTP_MT_WB:
 		if (!(vmx->nested.nested_vmx_ept_caps & VMX_EPTP_WB_BIT))
 			return false;
 		break;
@@ -7906,8 +7903,8 @@ static bool valid_ept_address(struct kvm_vcpu *vcpu, u64 address)
 		return false;
 	}
 
-	/* Bits 5:3 must be 3 */
-	if (((address >> VMX_EPT_GAW_EPTP_SHIFT) & 0x7) != VMX_EPT_DEFAULT_GAW)
+	/* only 4 levels page-walk length are valid */
+	if ((address & VMX_EPTP_PWL_MASK) != VMX_EPTP_PWL_4)
 		return false;
 
 	/* Reserved bits should not be set */
@@ -7915,7 +7912,7 @@ static bool valid_ept_address(struct kvm_vcpu *vcpu, u64 address)
 		return false;
 
 	/* AD, if set, should be supported */
-	if ((address & VMX_EPT_AD_ENABLE_BIT)) {
+	if (address & VMX_EPTP_AD_ENABLE_BIT) {
 		if (!(vmx->nested.nested_vmx_ept_caps & VMX_EPT_AD_BIT))
 			return false;
 	}
@@ -7943,7 +7940,7 @@ static int nested_vmx_eptp_switching(struct kvm_vcpu *vcpu,
 				     &address, index * 8, 8))
 		return 1;
 
-	accessed_dirty = !!(address & VMX_EPT_AD_ENABLE_BIT);
+	accessed_dirty = !!(address & VMX_EPTP_AD_ENABLE_BIT);
 
 	/*
 	 * If the (L2) guest does a vmfunc to the currently
@@ -9509,7 +9506,7 @@ static void __init vmx_check_processor_compat(void *rtn)
 
 static int get_ept_level(void)
 {
-	return VMX_EPT_DEFAULT_GAW + 1;
+	return 4;
 }
 
 static u64 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
@@ -9710,7 +9707,7 @@ static void nested_ept_inject_page_fault(struct kvm_vcpu *vcpu,
 
 static bool nested_ept_ad_enabled(struct kvm_vcpu *vcpu)
 {
-	return nested_ept_get_cr3(vcpu) & VMX_EPT_AD_ENABLE_BIT;
+	return nested_ept_get_cr3(vcpu) & VMX_EPTP_AD_ENABLE_BIT;
 }
 
 /* Callbacks for nested_ept_init_mmu_context: */
-- 
2.9.4

[PATCH v2 2/2] KVM: VMX: always require WB memory type for EPT

[David Hildenbrand]

We already always set that type but don't check if it is supported. Also
for nVMX, we only support WB for now. Let's just require it.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/x86/kvm/vmx.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 0dd9c65..d97f9e4 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1200,6 +1200,11 @@ static inline bool cpu_has_vmx_ept_4levels(void)
 	return vmx_capability.ept & VMX_EPT_PAGE_WALK_4_BIT;
 }
 
+static inline bool cpu_has_vmx_ept_mt_wb(void)
+{
+	return vmx_capability.ept & VMX_EPTP_WB_BIT;
+}
+
 static inline bool cpu_has_vmx_ept_ad_bits(void)
 {
 	return vmx_capability.ept & VMX_EPT_AD_BIT;
@@ -4300,7 +4305,6 @@ static u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa)
 {
 	u64 eptp = VMX_EPTP_MT_WB | VMX_EPTP_PWL_4;
 
-	/* TODO write the value reading from MSR */
 	if (enable_ept_ad_bits &&
 	    (!is_guest_mode(vcpu) || nested_ept_ad_enabled(vcpu)))
 		eptp |= VMX_EPTP_AD_ENABLE_BIT;
@@ -6642,7 +6646,8 @@ static __init int hardware_setup(void)
 		init_vmcs_shadow_fields();
 
 	if (!cpu_has_vmx_ept() ||
-	    !cpu_has_vmx_ept_4levels()) {
+	    !cpu_has_vmx_ept_4levels() ||
+	    !cpu_has_vmx_ept_mt_wb()) {
 		enable_ept = 0;
 		enable_unrestricted_guest = 0;
 		enable_ept_ad_bits = 0;
-- 
2.9.4

Re: [PATCH v2 0/2] KVM: VMX: require EPTP WB (write-back) support

[Wanpeng Li]

2017-08-11 5:15 GMT+08:00 David Hildenbrand <david@redhat.com>:
> This version should no longer result in a panic :(
>
> (note to self: always retest after tiny changes)
>
> v1 -> v2:
> - fix typo (invalid guest page-walk length)
>
> David Hildenbrand (2):
>   KVM: VMX: cleanup EPTP definitions
>   KVM: VMX: always require WB memory type for EPT

Hmm, I saw these patches in kvm/queue, and linux guest stuck during
boot due to patch 1/2.

Regards,
Wanpeng Li

Re: [PATCH v2 0/2] KVM: VMX: require EPTP WB (write-back) support

[Wanpeng Li]

2017-08-11 7:09 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
> 2017-08-11 5:15 GMT+08:00 David Hildenbrand <david@redhat.com>:
>> This version should no longer result in a panic :(
>>
>> (note to self: always retest after tiny changes)
>>
>> v1 -> v2:
>> - fix typo (invalid guest page-walk length)
>>
>> David Hildenbrand (2):
>>   KVM: VMX: cleanup EPTP definitions
>>   KVM: VMX: always require WB memory type for EPT
>
> Hmm, I saw these patches in kvm/queue, and linux guest stuck during
> boot due to patch 1/2.

Sorry, I make a mistake. The commit which should be blamed is this
one: https://git.kernel.org/pub/scm/virt/kvm/kvm.git/commit/?h=queue&id=c016004494b0914205e3c4aa23ea92cfb57ba6ee

Regards,
Wanpeng Li

Re: [PATCH v2 0/2] KVM: VMX: require EPTP WB (write-back) support

[Wanpeng Li]

2017-08-11 9:20 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
> 2017-08-11 7:09 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
>> 2017-08-11 5:15 GMT+08:00 David Hildenbrand <david@redhat.com>:
>>> This version should no longer result in a panic :(
>>>
>>> (note to self: always retest after tiny changes)
>>>
>>> v1 -> v2:
>>> - fix typo (invalid guest page-walk length)
>>>
>>> David Hildenbrand (2):
>>>   KVM: VMX: cleanup EPTP definitions
>>>   KVM: VMX: always require WB memory type for EPT
>>
>> Hmm, I saw these patches in kvm/queue, and linux guest stuck during
>> boot due to patch 1/2.
>
> Sorry, I make a mistake. The commit which should be blamed is this
> one: https://git.kernel.org/pub/scm/virt/kvm/kvm.git/commit/?h=queue&id=c016004494b0914205e3c4aa23ea92cfb57ba6ee

After several rounds testing, it is still patch 1/2. Please have a try, David.

Regards,
Wanpeng Li

Re: [PATCH v2 0/2] KVM: VMX: require EPTP WB (write-back) support

[Wanpeng Li]

2017-08-11 10:43 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
> 2017-08-11 9:20 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
>> 2017-08-11 7:09 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
>>> 2017-08-11 5:15 GMT+08:00 David Hildenbrand <david@redhat.com>:
>>>> This version should no longer result in a panic :(
>>>>
>>>> (note to self: always retest after tiny changes)
>>>>
>>>> v1 -> v2:
>>>> - fix typo (invalid guest page-walk length)
>>>>
>>>> David Hildenbrand (2):
>>>>   KVM: VMX: cleanup EPTP definitions
>>>>   KVM: VMX: always require WB memory type for EPT
>>>
>>> Hmm, I saw these patches in kvm/queue, and linux guest stuck during
>>> boot due to patch 1/2.
>>
>> Sorry, I make a mistake. The commit which should be blamed is this
>> one: https://git.kernel.org/pub/scm/virt/kvm/kvm.git/commit/?h=queue&id=c016004494b0914205e3c4aa23ea92cfb57ba6ee
>
> After several rounds testing, it is still patch 1/2. Please have a try, David.

I found that both this patch 1/2 and the Brijesh's patch have issues.
I just posted a patch to fix Brijesh's commit since his commit has
already been merged in kvm/next branch.

Regards,
Wanpeng Li

Re: [PATCH v2 0/2] KVM: VMX: require EPTP WB (write-back) support

[David Hildenbrand]

On 11.08.2017 08:09, Wanpeng Li wrote:
> 2017-08-11 10:43 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
>> 2017-08-11 9:20 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
>>> 2017-08-11 7:09 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
>>>> 2017-08-11 5:15 GMT+08:00 David Hildenbrand <david@redhat.com>:
>>>>> This version should no longer result in a panic :(
>>>>>
>>>>> (note to self: always retest after tiny changes)
>>>>>
>>>>> v1 -> v2:
>>>>> - fix typo (invalid guest page-walk length)
>>>>>
>>>>> David Hildenbrand (2):
>>>>>   KVM: VMX: cleanup EPTP definitions
>>>>>   KVM: VMX: always require WB memory type for EPT
>>>>
>>>> Hmm, I saw these patches in kvm/queue, and linux guest stuck during
>>>> boot due to patch 1/2.
>>>
>>> Sorry, I make a mistake. The commit which should be blamed is this
>>> one: https://git.kernel.org/pub/scm/virt/kvm/kvm.git/commit/?h=queue&id=c016004494b0914205e3c4aa23ea92cfb57ba6ee
>>
>> After several rounds testing, it is still patch 1/2. Please have a try, David.
> 
> I found that both this patch 1/2 and the Brijesh's patch have issues.
> I just posted a patch to fix Brijesh's commit since his commit has
> already been merged in kvm/next branch.
> 
> Regards,
> Wanpeng Li
> 

Hi Wanpeng,

thanks for testing. As far as I can see kvm/queue contains v1 of this
patch, which has a BUG fixed in this series.

Did you try with this patch here or with kvm/queue?

-- 

Thanks,

David

Re: [PATCH v2 0/2] KVM: VMX: require EPTP WB (write-back) support

[Wanpeng Li]

2017-08-11 14:23 GMT+08:00 David Hildenbrand <david@redhat.com>:
> On 11.08.2017 08:09, Wanpeng Li wrote:
>> 2017-08-11 10:43 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
>>> 2017-08-11 9:20 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
>>>> 2017-08-11 7:09 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
>>>>> 2017-08-11 5:15 GMT+08:00 David Hildenbrand <david@redhat.com>:
>>>>>> This version should no longer result in a panic :(
>>>>>>
>>>>>> (note to self: always retest after tiny changes)
>>>>>>
>>>>>> v1 -> v2:
>>>>>> - fix typo (invalid guest page-walk length)
>>>>>>
>>>>>> David Hildenbrand (2):
>>>>>>   KVM: VMX: cleanup EPTP definitions
>>>>>>   KVM: VMX: always require WB memory type for EPT
>>>>>
>>>>> Hmm, I saw these patches in kvm/queue, and linux guest stuck during
>>>>> boot due to patch 1/2.
>>>>
>>>> Sorry, I make a mistake. The commit which should be blamed is this
>>>> one: https://git.kernel.org/pub/scm/virt/kvm/kvm.git/commit/?h=queue&id=c016004494b0914205e3c4aa23ea92cfb57ba6ee
>>>
>>> After several rounds testing, it is still patch 1/2. Please have a try, David.
>>
>> I found that both this patch 1/2 and the Brijesh's patch have issues.
>> I just posted a patch to fix Brijesh's commit since his commit has
>> already been merged in kvm/next branch.
>>
>> Regards,
>> Wanpeng Li
>>
>
> Hi Wanpeng,
>
> thanks for testing. As far as I can see kvm/queue contains v1 of this
> patch, which has a BUG fixed in this series.
>
> Did you try with this patch here or with kvm/queue?

Yeah, the v2 works. :)

Regards,
Wanpeng Li

Re: [PATCH v2 0/2] KVM: VMX: require EPTP WB (write-back) support

[David Hildenbrand]

On 10.08.2017 23:15, David Hildenbrand wrote:
> This version should no longer result in a panic :(
> 
> (note to self: always retest after tiny changes)
> 
> v1 -> v2:
> - fix typo (invalid guest page-walk length)
> 
> David Hildenbrand (2):
>   KVM: VMX: cleanup EPTP definitions
>   KVM: VMX: always require WB memory type for EPT
> 
>  arch/x86/include/asm/vmx.h | 11 ++++++-----
>  arch/x86/kvm/vmx.c         | 34 ++++++++++++++++++----------------
>  2 files changed, 24 insertions(+), 21 deletions(-)
> 
Paolo, Radim

this version should work as intended.

-- 

Thanks,

David