Skip to contentSkip to conversation

do not call gva_to_gpa on physical address

PATCH In review Gleb Natapov 2 participants 4 messages Jul 14, 2010

Subsystems (3)
x86 KVM KVM x86
lore
Share & tools
lore

Gleb Natapov

author
#0 lore raw

PATCH 1/1 do not call gva_to_gpa on physical address

When tdp is enabled cr2 passed to emulator is pga not gva, so no need to call gva_to_gpa on it.

Signed-off-by Gleb Natapov <[email protected]>
arch/x86/kvm/x86.c +6 -3
index fb08316..721bf0a 100644
@@ -3944,10 +3944,13 @@ static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva)
 	if (kvm_mmu_unprotect_page_virt(vcpu, gva))
 		return true;
 
-	gpa = kvm_mmu_gva_to_gpa_system(vcpu, gva, NULL);
+	if (!tdp_enabled) {
+		gpa = kvm_mmu_gva_to_gpa_system(vcpu, gva, NULL);
 
-	if (gpa == UNMAPPED_GVA)
-		return true; /* let cpu generate fault */
+		if (gpa == UNMAPPED_GVA)
+			return true; /* let cpu generate fault */
+	} else
+		gpa = gva;
 
 	if (!kvm_is_error_hva(gfn_to_hva(vcpu->kvm, gpa >> PAGE_SHIFT)))
 		return true;

--
Gleb.

#1 lore raw
Quoted 16 lines · Gleb Natapov

When tdp is enabled cr2 passed to emulator is pga not gva, so no need to call gva_to_gpa on it.

Signed-off-by Gleb Natapov<[email protected]>
arch/x86/kvm/x86.c +2 -1
index fb08316..721bf0a 100644
@@ -3944,10 +3944,13 @@ static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva)
  	if (kvm_mmu_unprotect_page_virt(vcpu, gva))
  		return true;
 
-	gpa = kvm_mmu_gva_to_gpa_system(vcpu, gva, NULL);
+	if (!tdp_enabled) {
+		gpa = kvm_mmu_gva_to_gpa_system(vcpu, gva, NULL);

arch/x86/kvm/x86.c static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva) · line 3956

Quoted 10 lines
arch/x86/kvm/x86.c +4 -2
 
-	if (gpa == UNMAPPED_GVA)
-		return true; /* let cpu generate fault */
+		if (gpa == UNMAPPED_GVA)
+			return true; /* let cpu generate fault */
+	} else
+		gpa = gva;
 
  	if (!kvm_is_error_hva(gfn_to_hva(vcpu->kvm, gpa>>  PAGE_SHIFT)))
  		return true;

Why not

    if (tdp_enabled)
              return false;

at the top? Is there any reason ever to reexecute an instruction with tdp enabled?

Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
author
#2 lore raw
Quoted 29 lines · Avi Kivity
Quoted 15 lines · Gleb Natapov

When tdp is enabled cr2 passed to emulator is pga not gva, so no need to call gva_to_gpa on it.

Signed-off-by Gleb Natapov<[email protected]>
arch/x86/kvm/x86.c +2 -1
index fb08316..721bf0a 100644
@@ -3944,10 +3944,13 @@ static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva)
 	if (kvm_mmu_unprotect_page_virt(vcpu, gva))
 		return true;
 
-	gpa = kvm_mmu_gva_to_gpa_system(vcpu, gva, NULL);
+	if (!tdp_enabled) {
+		gpa = kvm_mmu_gva_to_gpa_system(vcpu, gva, NULL);

arch/x86/kvm/x86.c static bool reexecute_instruction(struct kvm_vcpu *vcpu, gva_t gva) · line 3956

Quoted 10 lines
arch/x86/kvm/x86.c +4 -2
 
-	if (gpa == UNMAPPED_GVA)
-		return true; /* let cpu generate fault */
+		if (gpa == UNMAPPED_GVA)
+			return true; /* let cpu generate fault */
+	} else
+		gpa = gva;
 
 	if (!kvm_is_error_hva(gfn_to_hva(vcpu->kvm, gpa>>  PAGE_SHIFT)))
 		return true;

Why not

Quoted 6 lines
    if (tdp_enabled)
              return false;

at the top? Is there any reason ever to reexecute an instruction with tdp enabled?

Can't think of one now. If in the future we will shadow some other data structures in the guest except page tables (nested vmx/svm?) there may be the reason.

--
Gleb.

#3 lore raw
Quoted 6 lines · Gleb Natapov
Quoted 6 lines

Why not

     if (tdp_enabled)
               return false;
Quoted 8 lines
Quoted 4 lines

at the top? Is there any reason ever to reexecute an instruction with tdp enabled?

Can't think of one now. If in the future we will shadow some other data structures in the guest except page tables (nested vmx/svm?) there may be the reason.

Sure, we'll adjust it then.

Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.