Re: [PATCH v2 3/3] kvm: svm: Use the hardware provided GPA instead of page walk
From: Brijesh Singh
Date: Thu Dec 08 2016 - 14:00:49 EST
Hi Paolo,
On 12/08/2016 09:39 AM, Brijesh Singh wrote:
Hi Paolo,
On 12/08/2016 08:52 AM, Paolo Bonzini wrote:
On 23/11/2016 18:02, Brijesh Singh wrote:
From: Tom Lendacky <thomas.lendacky@xxxxxxx>
When a guest causes a NPF which requires emulation, KVM sometimes walks
the guest page tables to translate the GVA to a GPA. This is unnecessary
most of the time on AMD hardware since the hardware provides the GPA in
EXITINFO2.
The only exception cases involve string operations involving rep or
operations that use two memory locations. With rep, the GPA will only be
the value of the initial NPF and with dual memory locations we won't
know
which memory address was translated into EXITINFO2.
Signed-off-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
Reviewed-by: Borislav Petkov <bp@xxxxxxx>
Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
Tom, Brijesh,
I would like to confirm that you have run kvm-unit-tests with this patch?
I haven't yet tried AMD (will do before sending the pull request to
Linus),
but a similar patch for Intel gives me these 4 failures on emulator.flat:
FAIL: push mem
FAIL: pop mem
FAIL: cross-page mmio read
FAIL: cross-page mmio write
I did not ran kvm-unit-tests. However, I was able to boot Linux
guest and run some stress test. I will run kvm-unit-tests and let
you know soon.
I am able to reproduce it on AMD HW using kvm-unit-tests. Looking at
test, the initial thought is "push mem" has two operands (the memory
being pushed and the stack pointer). The provided GPA could be either
one of those.
If we can detect those cases, we should not set the gpa_available on
them (like what we do with string move).
We probably haven't hit this case in guest booting. Will investigate bit
further and provide a updated patch to handle it.
-Brijesh
The VMX patch to set gpa_available is just this:
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 25d48380c312..5d7b60d4795b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -6393,6 +6393,7 @@ static int handle_ept_violation(struct kvm_vcpu
*vcpu)
/* ept page table is present? */
error_code |= (exit_qualification & 0x38) != 0;
+ vcpu->arch.gpa_available = true;
vcpu->arch.exit_qualification = exit_qualification;
return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
@@ -6410,6 +6411,7 @@ static int handle_ept_misconfig(struct kvm_vcpu
*vcpu)
}
ret = handle_mmio_page_fault(vcpu, gpa, true);
+ vcpu->arch.gpa_available = true;
if (likely(ret == RET_MMIO_PF_EMULATE))
return x86_emulate_instruction(vcpu, gpa, 0, NULL, 0) ==
EMULATE_DONE;
@@ -8524,6 +8526,7 @@ static int vmx_handle_exit(struct kvm_vcpu *vcpu)
u32 vectoring_info = vmx->idt_vectoring_info;
trace_kvm_exit(exit_reason, vcpu, KVM_ISA_VMX);
+ vcpu->arch.gpa_available = false;
/*
* Flush logged GPAs PML buffer, this will make dirty_bitmap more
Thanks,
Paolo
---
arch/x86/include/asm/kvm_emulate.h | 3 +++
arch/x86/include/asm/kvm_host.h | 3 +++
arch/x86/kvm/svm.c | 2 ++
arch/x86/kvm/x86.c | 17 ++++++++++++++++-
4 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/kvm_emulate.h
b/arch/x86/include/asm/kvm_emulate.h
index e9cd7be..2d1ac09 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -344,6 +344,9 @@ struct x86_emulate_ctxt {
struct read_cache mem_read;
};
+/* String operation identifier (matches the definition in emulate.c) */
+#define CTXT_STRING_OP (1 << 13)
+
/* Repeat String Operation Prefix */
#define REPE_PREFIX 0xf3
#define REPNE_PREFIX 0xf2
diff --git a/arch/x86/include/asm/kvm_host.h
b/arch/x86/include/asm/kvm_host.h
index 77cb3f9..fd5b1c8 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -668,6 +668,9 @@ struct kvm_vcpu_arch {
int pending_ioapic_eoi;
int pending_external_vector;
+
+ /* GPA available (AMD only) */
+ bool gpa_available;
};
struct kvm_lpage_info {
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 5e64e656..1bbd04c 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -4246,6 +4246,8 @@ static int handle_exit(struct kvm_vcpu *vcpu)
return 1;
}
+ vcpu->arch.gpa_available = (exit_code == SVM_EXIT_NPF);
+
return svm_exit_handlers[exit_code](svm);
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c30f62dc..5002eea 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4441,7 +4441,19 @@ static int vcpu_mmio_gva_to_gpa(struct
kvm_vcpu *vcpu, unsigned long gva,
return 1;
}
- *gpa = vcpu->arch.walk_mmu->gva_to_gpa(vcpu, gva, access,
exception);
+ /*
+ * If the exit was due to a NPF we may already have a GPA.
+ * If the GPA is present, use it to avoid the GVA to GPA table
+ * walk. Note, this cannot be used on string operations since
+ * string operation using rep will only have the initial GPA
+ * from when the NPF occurred.
+ */
+ if (vcpu->arch.gpa_available &&
+ !(vcpu->arch.emulate_ctxt.d & CTXT_STRING_OP))
+ *gpa = exception->address;
+ else
+ *gpa = vcpu->arch.walk_mmu->gva_to_gpa(vcpu, gva, access,
+ exception);
if (*gpa == UNMAPPED_GVA)
return -1;
@@ -5563,6 +5575,9 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu,
}
restart:
+ /* Save the faulting GPA (cr2) in the address field */
+ ctxt->exception.address = cr2;
+
r = x86_emulate_insn(ctxt);
if (r == EMULATION_INTERCEPTED)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html