KVM: nVMX: Enable unrestricted guest mode and fix some nEPT issues

From: Jan Kiszka <[email protected]>

Do not report that we can enter the guest in 64-bit mode if the host is 32-bit only. This is not supported by KVM.

Signed-off-by Jan Kiszka <[email protected]>

---
 arch/x86/kvm/vmx.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

arch/x86/kvm/vmx.c +5 -1

index 773b54d..c810f9b 100644
@@ -2212,9 +2212,13 @@ static __init void nested_vmx_setup_ctls_msrs(void)
 	/* If bit 55 of VMX_BASIC is off, bits 0-8 and 12 must be 1. */
 	nested_vmx_entry_ctls_low = VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR;
 	nested_vmx_entry_ctls_high &=
-		VM_ENTRY_LOAD_IA32_PAT | VM_ENTRY_IA32E_MODE;
+#ifdef CONFIG_X86_64
+		VM_ENTRY_IA32E_MODE |
+#endif
+		VM_ENTRY_LOAD_IA32_PAT;
 	nested_vmx_entry_ctls_high |= (VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR |
 				       VM_ENTRY_LOAD_IA32_EFER);
+
 	/* cpu-based controls */
 	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS,
 		nested_vmx_procbased_ctls_low, nested_vmx_procbased_ctls_high);

Signature · 1 line

1.7.3.4

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

Signed-off-by Jan Kiszka <[email protected]>

---
 arch/x86/kvm/vmx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/x86/kvm/vmx.c +1 -1

index b482d47..09666aa 100644
@@ -8106,7 +8106,7 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 	 * Additionally, restore L2's PDPTR to vmcs12.
 	 */
 	if (enable_ept) {
-		vmcs12->guest_cr3 = vmcs_read64(GUEST_CR3);
+		vmcs12->guest_cr3 = kvm_read_cr3(vcpu);
 		vmcs12->guest_pdptr0 = vmcs_read64(GUEST_PDPTR0);
 		vmcs12->guest_pdptr1 = vmcs_read64(GUEST_PDPTR1);
 		vmcs12->guest_pdptr2 = vmcs_read64(GUEST_PDPTR2);

Signature · 1 line

1.7.3.4

Quoted 9 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

I have a headache from trying to think about it already, but shouldn't L1 be the one who setups identity map for L2? I traced what vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) return here and do not see different values in real mode.

Quoted 21 lines

Signed-off-by Jan Kiszka <[email protected]>

---
 arch/x86/kvm/vmx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/x86/kvm/vmx.c +1 -1

index b482d47..09666aa 100644
@@ -8106,7 +8106,7 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 	 * Additionally, restore L2's PDPTR to vmcs12.
 	 */
 	if (enable_ept) {
-		vmcs12->guest_cr3 = vmcs_read64(GUEST_CR3);
+		vmcs12->guest_cr3 = kvm_read_cr3(vcpu);
 		vmcs12->guest_pdptr0 = vmcs_read64(GUEST_PDPTR0);
 		vmcs12->guest_pdptr1 = vmcs_read64(GUEST_PDPTR1);
 		vmcs12->guest_pdptr2 = vmcs_read64(GUEST_PDPTR2);

Signature · 1 line

1.7.3.4

--
Gleb.

Quoted 10 lines · Gleb Natapov

Quoted 9 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

Quoted 4 lines

I have a headache from trying to think about it already, but shouldn't L1 be the one who setups identity map for L2? I traced what vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) return here and do not see different values in real mode.

Did you try with my patches applied and unrestricted guest mode in use?

Jan

Quoted 25 lines

Quoted 21 lines

Signed-off-by Jan Kiszka <[email protected]>

---
 arch/x86/kvm/vmx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/x86/kvm/vmx.c +1 -1

index b482d47..09666aa 100644
@@ -8106,7 +8106,7 @@ static void prepare_vmcs12(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 	 * Additionally, restore L2's PDPTR to vmcs12.
 	 */
 	if (enable_ept) {
-		vmcs12->guest_cr3 = vmcs_read64(GUEST_CR3);
+		vmcs12->guest_cr3 = kvm_read_cr3(vcpu);
 		vmcs12->guest_pdptr0 = vmcs_read64(GUEST_PDPTR0);
 		vmcs12->guest_pdptr1 = vmcs_read64(GUEST_PDPTR1);
 		vmcs12->guest_pdptr2 = vmcs_read64(GUEST_PDPTR2);

Signature · 1 line

1.7.3.4

--
Gleb.

Quoted 16 lines · Jan Kiszka

Quoted 14 lines · Gleb Natapov

Quoted 9 lines · Jan Kiszka

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

I have a headache from trying to think about it already, but shouldn't L1 be the one who setups identity map for L2? I traced what vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) return here and do not see different values in real mode.

Quoted 2 lines

Did you try with my patches applied and unrestricted guest mode in use?

No, for that I need to setup nested environment on the machine that support unrestricted guest first :)

Quoted 10 lines

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and
GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

I have a headache from trying to think about it already, but shouldn't L1 be the one who setups identity map for L2? I traced what vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) return here and do not see

different values in real mode.

arch/x86/kvm/vmx.c static void prepare_vmcs12(struct kvm_vcpu

Quoted 9 lines

Signed-off-by Jan Kiszka <[email protected]>

---
 arch/x86/kvm/vmx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/x86/kvm/vmx.c index renamed from arch/x86/kvm/vmx.c +0 -0

b482d47..09666aa 100644

arch/x86/kvm/vmx.c +0 -0

@@ -8106,7 +8106,7 @@ static void prepare_vmcs12(struct kvm_vcpu

*vcpu, struct vmcs12 *vmcs12)

Quoted 10 lines

arch/x86/kvm/vmx.c +1 -2

 	 * Additionally, restore L2's PDPTR to vmcs12.
 	 */
 	if (enable_ept) {
-		vmcs12->guest_cr3 = vmcs_read64(GUEST_CR3);
+		vmcs12->guest_cr3 = kvm_read_cr3(vcpu);
 		vmcs12->guest_pdptr0 = vmcs_read64(GUEST_PDPTR0);
 		vmcs12->guest_pdptr1 = vmcs_read64(GUEST_PDPTR1);
 		vmcs12->guest_pdptr2 = vmcs_read64(GUEST_PDPTR2);
--

1.7.3.4

Gleb Natapov wrote on 2013-08-06:

Quoted 11 lines · Jan Kiszka

Quoted 10 lines

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and
GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

Quoted 3 lines

I have a headache from trying to think about it already, but shouldn't L1 be the one who setups identity map for L2? I traced what vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) return here and do not see

Here is my understanding:
In vmx_set_cr3(), if enabled ept, it will check whether target vcpu is enabling paging. When L2 running in real mode, then target vcpu is not enabling paging and it will use L0's identity map for L2. If you read GUEST_CR3 from VMCS, then you may get the L2's identity map not L1's.

Quoted 12 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 11 lines · Jan Kiszka

Quoted 10 lines

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and
GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

Quoted 8 lines

Quoted 5 lines

I have a headache from trying to think about it already, but shouldn't
L1 be the one who setups identity map for L2? I traced what vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) return here and do not see

Here is my understanding:
In vmx_set_cr3(), if enabled ept, it will check whether target vcpu is enabling

paging. When L2 running in real mode, then target vcpu is not enabling paging and it will use L0's identity map for L2. If you read GUEST_CR3 from VMCS, then you may get the L2's identity map not L1's.

Quoted 1 line

Yes, but why it makes sense to use L0 identity map for L2? I didn't see different vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) values because L0 and L1 use the same identity map address. When I changed identity address L1 configures vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) are indeed different, but the real CR3 L2 uses points to L0 identity map. If I zero L1 identity map page L2 still works.

Gleb Natapov wrote on 2013-08-06:

Quoted 25 lines · Yang Z

Quoted 12 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 11 lines · Jan Kiszka

Quoted 10 lines

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and
GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

Quoted 8 lines

Quoted 5 lines

I have a headache from trying to think about it already, but shouldn't
L1 be the one who setups identity map for L2? I traced what vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) return here and do not see

Here is my understanding:
In vmx_set_cr3(), if enabled ept, it will check whether target vcpu is enabling

paging. When L2 running in real mode, then target vcpu is not enabling paging and it will use L0's identity map for L2. If you read GUEST_CR3 from VMCS, then you may get the L2's identity map not L1's.

Quoted 1 line

Quoted 7 lines

Yes, but why it makes sense to use L0 identity map for L2? I didn't see different vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) values because L0 and L1 use the same identity map address. When I changed identity address L1 configures vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) are indeed different, but the real CR3 L2 uses points to L0 identity map. If I zero L1 identity map page L2 still works.

If L2 in real mode, then L2PA == L1PA. So L0's identity map also works if L2 is in real mode.

Quoted 25 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 24 lines · Yang Z

Quoted 11 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 10 lines · Jan Kiszka

Quoted 9 lines

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

Quoted 8 lines

Quoted 5 lines

I have a headache from trying to think about it already, but shouldn't
L1 be the one who setups identity map for L2? I traced what vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) return here and do not see

Here is my understanding:
In vmx_set_cr3(), if enabled ept, it will check whether target vcpu is enabling

paging. When L2 running in real mode, then target vcpu is not enabling paging and it will use L0's identity map for L2. If you read GUEST_CR3 from VMCS, then you may get the L2's identity map not

L1's.

Quoted 8 lines

Quoted 8 lines

Quoted 1 line

Yes, but why it makes sense to use L0 identity map for L2? I didn't see different vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) values because L0 and L1 use the same identity map address. When I changed identity address L1 configures vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) are indeed different, but the real CR3 L2 uses points to L0 identity map. If I zero L1 identity map page L2 still works.

Quoted 3 lines

If L2 in real mode, then L2PA == L1PA. So L0's identity map also works if L2 is in real mode.

That not the point. It may work accidentally for kvm on kvm, but what
if other hypervisor plays different tricks and builds different ident map for its guest?

Gleb Natapov wrote on 2013-08-06:

Quoted 35 lines · Yang Z

Quoted 25 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 24 lines · Yang Z

Quoted 11 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 10 lines · Jan Kiszka

Quoted 9 lines

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

Quoted 8 lines

Quoted 5 lines

I have a headache from trying to think about it already, but shouldn't
L1 be the one who setups identity map for L2? I traced what vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) return here and do not see

Here is my understanding:
In vmx_set_cr3(), if enabled ept, it will check whether target vcpu is enabling

paging. When L2 running in real mode, then target vcpu is not enabling paging and it will use L0's identity map for L2. If you read GUEST_CR3 from VMCS, then you may get the L2's identity map not

L1's.

Quoted 8 lines

Quoted 8 lines

Quoted 1 line

Yes, but why it makes sense to use L0 identity map for L2? I didn't see different vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) values because L0 and L1 use the same identity map address. When I changed identity address L1 configures vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) are indeed different, but the real CR3 L2 uses points to L0 identity map. If I zero L1 identity map page L2 still works.

Quoted 5 lines

Quoted 3 lines

If L2 in real mode, then L2PA == L1PA. So L0's identity map also works if L2 is in real mode.

That not the point. It may work accidentally for kvm on kvm, but what
if other hypervisor plays different tricks and builds different ident map for its guest?

Yes, if other hypervisor doesn't build the 1:1 mapping for its guest, it will fail to work. But I cannot imagine what kind of hypervisor will do this and what the purpose is. Anyway, current logic is definitely wrong. It should use L1's identity map instead L0's.

Quoted 36 lines · Yang Z

Gleb Natapov wrote on 2013-08-06:

Quoted 35 lines · Yang Z

Quoted 25 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 24 lines · Yang Z

Quoted 11 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 10 lines · Jan Kiszka

Quoted 9 lines

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

Quoted 8 lines

Quoted 5 lines

I have a headache from trying to think about it already, but shouldn't
L1 be the one who setups identity map for L2? I traced what vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) return here and do not see

Here is my understanding:
In vmx_set_cr3(), if enabled ept, it will check whether target vcpu is enabling

paging. When L2 running in real mode, then target vcpu is not enabling paging and it will use L0's identity map for L2. If you read GUEST_CR3 from VMCS, then you may get the L2's identity map not

L1's.

Quoted 8 lines

Quoted 8 lines

Quoted 1 line

Yes, but why it makes sense to use L0 identity map for L2? I didn't see different vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) values because L0 and L1 use the same identity map address. When I changed identity address L1 configures vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) are indeed different, but the real CR3 L2 uses points to L0 identity map. If I zero L1 identity map page L2 still works.

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

If L2 in real mode, then L2PA == L1PA. So L0's identity map also works if L2 is in real mode.

That not the point. It may work accidentally for kvm on kvm, but what
if other hypervisor plays different tricks and builds different ident map for its guest?

Yes, if other hypervisor doesn't build the 1:1 mapping for its guest, it will fail to work. But I cannot imagine what kind of hypervisor will do this and what the purpose is. Anyway, current logic is definitely wrong. It should use L1's identity map instead L0's.

So something like this is rather needed?

arch/x86/kvm/vmx.c +0 -0

index 44494ed..60a3644 100644
@@ -3375,8 +3375,10 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
 	if (enable_ept) {
 		eptp = construct_eptp(cr3);

Quoted 57 lines · Jan Kiszka

Quoted 36 lines · Yang Z

Gleb Natapov wrote on 2013-08-06:

Quoted 35 lines · Yang Z

Quoted 25 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 24 lines · Yang Z

Quoted 11 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 10 lines · Jan Kiszka

Quoted 9 lines

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

Quoted 8 lines

Quoted 5 lines

I have a headache from trying to think about it already, but shouldn't
L1 be the one who setups identity map for L2? I traced what vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) return here and do not see

Here is my understanding:
In vmx_set_cr3(), if enabled ept, it will check whether target vcpu is enabling

paging. When L2 running in real mode, then target vcpu is not enabling paging and it will use L0's identity map for L2. If you read GUEST_CR3 from VMCS, then you may get the L2's identity map not

L1's.

Quoted 8 lines

Quoted 8 lines

Quoted 1 line

Yes, but why it makes sense to use L0 identity map for L2? I didn't see different vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) values because L0 and L1 use the same identity map address. When I changed identity address L1 configures vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) are indeed different, but the real CR3 L2 uses points to L0 identity map. If I zero L1 identity map page L2 still works.

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

If L2 in real mode, then L2PA == L1PA. So L0's identity map also works if L2 is in real mode.

That not the point. It may work accidentally for kvm on kvm, but what
if other hypervisor plays different tricks and builds different ident map for its guest?

Yes, if other hypervisor doesn't build the 1:1 mapping for its guest, it will fail to work. But I cannot imagine what kind of hypervisor will do this and what the purpose is. Anyway, current logic is definitely wrong. It should use L1's identity map instead L0's.

So something like this is rather needed?

arch/x86/kvm/vmx.c +0 -2

index 44494ed..60a3644 100644
@@ -3375,8 +3375,10 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
 	if (enable_ept) {
 		eptp = construct_eptp(cr3);
 		vmcs_write64(EPT_POINTER, eptp);
-		guest_cr3 = is_paging(vcpu) ? kvm_read_cr3(vcpu) :
-			vcpu->kvm->arch.ept_identity_map_addr;

arch/x86/kvm/vmx.c static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3) · line 3384

Quoted 7 lines

arch/x86/kvm/vmx.c +4 -0

+		if (is_paging(vcpu) || is_guest_mode(vcpu))
+			guest_cr3 = kvm_read_cr3(vcpu) :
+		else
+			guest_cr3 = vcpu->kvm->arch.ept_identity_map_addr;
 		ept_load_pdptrs(vcpu);
 	}
 

That what I am thinking, will think about it some more tomorrow.

Quoted 58 lines · Gleb Natapov

Quoted 57 lines · Jan Kiszka

Quoted 36 lines · Yang Z

Gleb Natapov wrote on 2013-08-06:

Quoted 35 lines · Yang Z

Quoted 25 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 24 lines · Yang Z

Quoted 11 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 10 lines · Jan Kiszka

Quoted 9 lines

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

Quoted 8 lines

Quoted 5 lines

I have a headache from trying to think about it already, but shouldn't
L1 be the one who setups identity map for L2? I traced what vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) return here and do not see

Here is my understanding:
In vmx_set_cr3(), if enabled ept, it will check whether target vcpu is enabling

paging. When L2 running in real mode, then target vcpu is not enabling paging and it will use L0's identity map for L2. If you read GUEST_CR3 from VMCS, then you may get the L2's identity map not

L1's.

Quoted 8 lines

Quoted 8 lines

Quoted 1 line

Yes, but why it makes sense to use L0 identity map for L2? I didn't see different vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) values because L0 and L1 use the same identity map address. When I changed identity address L1 configures vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) are indeed different, but the real CR3 L2 uses points to L0 identity map. If I zero L1 identity map page L2 still works.

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

If L2 in real mode, then L2PA == L1PA. So L0's identity map also works if L2 is in real mode.

That not the point. It may work accidentally for kvm on kvm, but what
if other hypervisor plays different tricks and builds different ident map for its guest?

Yes, if other hypervisor doesn't build the 1:1 mapping for its guest, it will fail to work. But I cannot imagine what kind of hypervisor will do this and what the purpose is. Anyway, current logic is definitely wrong. It should use L1's identity map instead L0's.

So something like this is rather needed?

arch/x86/kvm/vmx.c +0 -2

index 44494ed..60a3644 100644
@@ -3375,8 +3375,10 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
 	if (enable_ept) {
 		eptp = construct_eptp(cr3);
 		vmcs_write64(EPT_POINTER, eptp);
-		guest_cr3 = is_paging(vcpu) ? kvm_read_cr3(vcpu) :
-			vcpu->kvm->arch.ept_identity_map_addr;

Quoted 8 lines

arch/x86/kvm/vmx.c static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3) · line 3384

Quoted 7 lines

arch/x86/kvm/vmx.c +4 -0

+		if (is_paging(vcpu) || is_guest_mode(vcpu))
+			guest_cr3 = kvm_read_cr3(vcpu) :
+		else
+			guest_cr3 = vcpu->kvm->arch.ept_identity_map_addr;
 		ept_load_pdptrs(vcpu);
 	}
 

That what I am thinking, will think about it some more tomorrow.

OK, and I'll feed it into a local test.

Quoted 68 lines · Jan Kiszka

Quoted 58 lines · Gleb Natapov

Quoted 57 lines · Jan Kiszka

Quoted 36 lines · Yang Z

Gleb Natapov wrote on 2013-08-06:

Quoted 35 lines · Yang Z

Quoted 25 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 24 lines · Yang Z

Quoted 11 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 10 lines · Jan Kiszka

Quoted 9 lines

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

Quoted 8 lines

Quoted 5 lines

I have a headache from trying to think about it already, but shouldn't
L1 be the one who setups identity map for L2? I traced what vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) return here and do not see

Here is my understanding:
In vmx_set_cr3(), if enabled ept, it will check whether target vcpu is enabling

paging. When L2 running in real mode, then target vcpu is not enabling paging and it will use L0's identity map for L2. If you read GUEST_CR3 from VMCS, then you may get the L2's identity map not

L1's.

Quoted 8 lines

Quoted 8 lines

Quoted 1 line

Yes, but why it makes sense to use L0 identity map for L2? I didn't see different vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) values because L0 and L1 use the same identity map address. When I changed identity address L1 configures vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) are indeed different, but the real CR3 L2 uses points to L0 identity map. If I zero L1 identity map page L2 still works.

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

If L2 in real mode, then L2PA == L1PA. So L0's identity map also works if L2 is in real mode.

That not the point. It may work accidentally for kvm on kvm, but what
if other hypervisor plays different tricks and builds different ident map for its guest?

Yes, if other hypervisor doesn't build the 1:1 mapping for its guest, it will fail to work. But I cannot imagine what kind of hypervisor will do this and what the purpose is. Anyway, current logic is definitely wrong. It should use L1's identity map instead L0's.

So something like this is rather needed?

arch/x86/kvm/vmx.c +0 -2

index 44494ed..60a3644 100644
@@ -3375,8 +3375,10 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
 	if (enable_ept) {
 		eptp = construct_eptp(cr3);
 		vmcs_write64(EPT_POINTER, eptp);
-		guest_cr3 = is_paging(vcpu) ? kvm_read_cr3(vcpu) :
-			vcpu->kvm->arch.ept_identity_map_addr;

Quoted 8 lines

arch/x86/kvm/vmx.c static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3) · line 3384

Quoted 7 lines

arch/x86/kvm/vmx.c +4 -0

+		if (is_paging(vcpu) || is_guest_mode(vcpu))
+			guest_cr3 = kvm_read_cr3(vcpu) :
+		else
+			guest_cr3 = vcpu->kvm->arch.ept_identity_map_addr;
 		ept_load_pdptrs(vcpu);
 	}
 

That what I am thinking, will think about it some more tomorrow.

Quoted 2 lines

OK, and I'll feed it into a local test.

Thought about is some more. So without nested unrestricted guest (nUG) is_paging() will always be true (since without nUG guest entry is not possible otherwise) and guest's cr3 will be used, but with nUG identity map is not used (that is why L2 still works even though wrong identity map pointer is assigned to cr3), so the code here just corrupts nested guest's cr3 for no reason and that is why you had to use kvm_read_cr3() in prepare_vmcs12() to get correct cr3 value. The patch above should be used instead of original one IMO. How is testing going?

Quoted 71 lines · Gleb Natapov

Quoted 68 lines · Jan Kiszka

Quoted 58 lines · Gleb Natapov

Quoted 57 lines · Jan Kiszka

Quoted 36 lines · Yang Z

Gleb Natapov wrote on 2013-08-06:

Quoted 35 lines · Yang Z

Quoted 25 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 24 lines · Yang Z

Quoted 11 lines

Gleb Natapov wrote on 2013-08-06:

Quoted 10 lines · Jan Kiszka

Quoted 9 lines

From: Jan Kiszka <[email protected]>

If nested EPT is enabled, the L2 guest may change CR3 without any exits. We therefore have to read the current value from the VMCS when switching to L1. However, if paging wasn't enabled, L0 tracks L2's CR3, and GUEST_CR3 rather contains the real-mode identity map. So we need to retrieve CR3 from the architectural state after conditionally updating it - and this is what kvm_read_cr3 does.

Quoted 8 lines

Quoted 5 lines

I have a headache from trying to think about it already, but shouldn't
L1 be the one who setups identity map for L2? I traced what vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) return here and do not see

Here is my understanding:
In vmx_set_cr3(), if enabled ept, it will check whether target vcpu is enabling

paging. When L2 running in real mode, then target vcpu is not enabling paging and it will use L0's identity map for L2. If you read GUEST_CR3 from VMCS, then you may get the L2's identity map not

L1's.

Quoted 8 lines

Quoted 8 lines

Quoted 1 line

Yes, but why it makes sense to use L0 identity map for L2? I didn't see different vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) values because L0 and L1 use the same identity map address. When I changed identity address L1 configures vmcs_read64(GUEST_CR3)/kvm_read_cr3(vcpu) are indeed different, but the real CR3 L2 uses points to L0 identity map. If I zero L1 identity map page L2 still works.

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

If L2 in real mode, then L2PA == L1PA. So L0's identity map also works if L2 is in real mode.

That not the point. It may work accidentally for kvm on kvm, but what
if other hypervisor plays different tricks and builds different ident map for its guest?

Yes, if other hypervisor doesn't build the 1:1 mapping for its guest, it will fail to work. But I cannot imagine what kind of hypervisor will do this and what the purpose is. Anyway, current logic is definitely wrong. It should use L1's identity map instead L0's.

So something like this is rather needed?

arch/x86/kvm/vmx.c +0 -2

index 44494ed..60a3644 100644
@@ -3375,8 +3375,10 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
  	if (enable_ept) {
  		eptp = construct_eptp(cr3);
  		vmcs_write64(EPT_POINTER, eptp);
-		guest_cr3 = is_paging(vcpu) ? kvm_read_cr3(vcpu) :
-			vcpu->kvm->arch.ept_identity_map_addr;

Quoted 8 lines

arch/x86/kvm/vmx.c static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3) · line 3384

Quoted 7 lines

arch/x86/kvm/vmx.c +4 -0

+		if (is_paging(vcpu) || is_guest_mode(vcpu))
+			guest_cr3 = kvm_read_cr3(vcpu) :
+		else
+			guest_cr3 = vcpu->kvm->arch.ept_identity_map_addr;
  		ept_load_pdptrs(vcpu);
  	}
 

That what I am thinking, will think about it some more tomorrow.

Quoted 2 lines

OK, and I'll feed it into a local test.

Quoted 8 lines

Thought about is some more. So without nested unrestricted guest (nUG) is_paging() will always be true (since without nUG guest entry is not possible otherwise) and guest's cr3 will be used, but with nUG identity map is not used (that is why L2 still works even though wrong identity map pointer is assigned to cr3), so the code here just corrupts nested guest's cr3 for no reason and that is why you had to use kvm_read_cr3() in prepare_vmcs12() to get correct cr3 value. The patch above should be used instead of original one IMO. How is testing going?

Yes, testing worked fine. I've queued above patch and will send it out within the next round.

Quoted 30 lines

Quoted 29 lines

Quoted 19 lines

Quoted 10 lines

Quoted 10 lines

arch/x86/kvm/vmx.c +0 -2

index 44494ed..60a3644 100644
@@ -3375,8 +3375,10 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
 	if (enable_ept) {
 		eptp = construct_eptp(cr3);
 		vmcs_write64(EPT_POINTER, eptp);
-		guest_cr3 = is_paging(vcpu) ? kvm_read_cr3(vcpu) :
-			vcpu->kvm->arch.ept_identity_map_addr;

Quoted 8 lines

arch/x86/kvm/vmx.c static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3) · line 3384

Quoted 7 lines

arch/x86/kvm/vmx.c +4 -0

+		if (is_paging(vcpu) || is_guest_mode(vcpu))
+			guest_cr3 = kvm_read_cr3(vcpu) :
+		else
+			guest_cr3 = vcpu->kvm->arch.ept_identity_map_addr;
 		ept_load_pdptrs(vcpu);
 	}
 

That what I am thinking, will think about it some more tomorrow.

Quoted 2 lines

OK, and I'll feed it into a local test.

Thought about is some more. So without nested unrestricted guest (nUG) is_paging() will always be true (since without nUG guest entry is not possible otherwise) and guest's cr3 will be used, but with nUG identity map is not used (that is why L2 still works even though wrong identity map pointer is assigned to cr3), so the code here just corrupts nested guest's cr3 for no reason and that is why you had to use kvm_read_cr3() in prepare_vmcs12() to get correct cr3 value. The patch above should be used instead of original one IMO. How is testing going?

Quoted 2 lines

Yes, testing worked fine. I've queued above patch and will send it out within the next round.

Just reply here with the commit message you desire and Signed-off-by, so I can queue it for people who wish to play with nEPT.

Quoted 30 lines · Paolo Bonzini

Quoted 30 lines

Quoted 29 lines

Quoted 19 lines

Quoted 10 lines

Quoted 10 lines

arch/x86/kvm/vmx.c +0 -2

index 44494ed..60a3644 100644
@@ -3375,8 +3375,10 @@ static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
  	if (enable_ept) {
  		eptp = construct_eptp(cr3);
  		vmcs_write64(EPT_POINTER, eptp);
-		guest_cr3 = is_paging(vcpu) ? kvm_read_cr3(vcpu) :
-			vcpu->kvm->arch.ept_identity_map_addr;

Quoted 8 lines

arch/x86/kvm/vmx.c static void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3) · line 3384

Quoted 7 lines

arch/x86/kvm/vmx.c +4 -0

+		if (is_paging(vcpu) || is_guest_mode(vcpu))
+			guest_cr3 = kvm_read_cr3(vcpu) :
+		else
+			guest_cr3 = vcpu->kvm->arch.ept_identity_map_addr;
  		ept_load_pdptrs(vcpu);
  	}
 

That what I am thinking, will think about it some more tomorrow.

Quoted 2 lines

OK, and I'll feed it into a local test.

Thought about is some more. So without nested unrestricted guest (nUG) is_paging() will always be true (since without nUG guest entry is not possible otherwise) and guest's cr3 will be used, but with nUG identity map is not used (that is why L2 still works even though wrong identity map pointer is assigned to cr3), so the code here just corrupts nested guest's cr3 for no reason and that is why you had to use kvm_read_cr3() in prepare_vmcs12() to get correct cr3 value. The patch above should be used instead of original one IMO. How is testing going?

Quoted 6 lines

Quoted 2 lines

Yes, testing worked fine. I've queued above patch and will send it out within the next round.

Just reply here with the commit message you desire and Signed-off-by, so I can queue it for people who wish to play with nEPT.

I would love to have a comment there too :)

Quoted 38 lines · Gleb Natapov

Quoted 37 lines · Paolo Bonzini

Quoted 36 lines

Quoted 22 lines

Quoted 20 lines

Quoted 11 lines

Quoted 11 lines

arch/x86/kvm/vmx.c +0 -2

index 44494ed..60a3644 100644
@@ -3375,8 +3375,10 @@ static void vmx_set_cr3(struct kvm_vcpu
 *vcpu, unsigned long cr3)
      if (enable_ept) {
          eptp = construct_eptp(cr3);
          vmcs_write64(EPT_POINTER, eptp);
-        guest_cr3 = is_paging(vcpu) ? kvm_read_cr3(vcpu) :
-            vcpu->kvm->arch.ept_identity_map_addr;

Quoted 8 lines

arch/x86/kvm/vmx.c static void vmx_set_cr3(struct kvm_vcpu · line 3384

Quoted 7 lines

arch/x86/kvm/vmx.c +4 -0

+        if (is_paging(vcpu) || is_guest_mode(vcpu))
+            guest_cr3 = kvm_read_cr3(vcpu) :
+        else
+            guest_cr3 = vcpu->kvm->arch.ept_identity_map_addr;
          ept_load_pdptrs(vcpu);
      }

That what I am thinking, will think about it some more tomorrow.

Quoted 2 lines

OK, and I'll feed it into a local test.

Quoted 11 lines

Thought about is some more. So without nested unrestricted guest (nUG) is_paging() will always be true (since without nUG guest entry is not possible otherwise) and guest's cr3 will be used, but with nUG identity
map is not used (that is why L2 still works even though wrong identity map pointer is assigned to cr3), so the code here just corrupts nested guest's cr3 for no reason and that is why you had to use kvm_read_cr3()
in prepare_vmcs12() to get correct cr3 value. The patch above should be
used instead of original one IMO. How is testing going?

Yes, testing worked fine. I've queued above patch and will send it out within the next round.

Quoted 5 lines

Quoted 3 lines

Just reply here with the commit message you desire and Signed-off-by, so I can queue it for people who wish to play with nEPT.

I would love to have a comment there too :)

Ok, then it can wait since it is only needed with nested unrestricted guest.

On the other hand, it should come before patch 4 on the next submission.

These patches apply on top of nEPT v7.

Changes in v2:
- rebased over v7
- added patch to fix VM_ENTRY_IA32E_MODE reporting

Jan Kiszka (8):
KVM: nEPT: Advertise WB type EPTP
KVM: nVMX: Fix up VM_ENTRY_IA32E_MODE control feature reporting
KVM: nVMX: Replace kvm_set_cr0 with vmx_set_cr0 in
load_vmcs12_host_state
KVM: nVMX: Enable unrestricted guest mode support
KVM: nVMX: Fix guest CR3 read-back on VM-exit
KVM: nVMX: Load nEPT state after EFER
KVM: nVMX: Implement support for EFER saving on VM-exit
KVM: nVMX: Update mmu.base_role.nxe after EFER loading on
VM-entry/exit

  arch/x86/kvm/vmx.c |   50 +++++++++++++++++++++++++++++++++++---------------
  1 files changed, 35 insertions(+), 15 deletions(-)