Re: [RFC PATCH v6 75/92] kvm: x86: disable gpa_available optimization in emulator_read_write_onepage()

On 13/08/19 16:33, Adalbert Lazăr wrote:

On Tue, 13 Aug 2019 10:47:34 +0200, Paolo Bonzini [off-list ref] wrote:

quoted

On 09/08/19 18:00, Adalbert Lazăr wrote:

quoted

If the EPT violation was caused by an execute restriction imposed by the
introspection tool, gpa_available will point to the instruction pointer,
not the to the read/write location that has to be used to emulate the
current instruction.

This optimization should be disabled only when the VM is introspected,
not just because the introspection subsystem is present.

Signed-off-by: Adalbert Lazăr <redacted>

The right thing to do is to not set gpa_available for fetch failures in 
kvm_mmu_page_fault instead:

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 24843cf49579..1bdca40fa831 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c

@@ -5364,8 +5364,12 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u64 error_code,
 	enum emulation_result er;
 	bool direct = vcpu->arch.mmu->direct_map;
 
-	/* With shadow page tables, fault_address contains a GVA or nGPA.  */
-	if (vcpu->arch.mmu->direct_map) {
+	/*
+	 * With shadow page tables, fault_address contains a GVA or nGPA.
+	 * On a fetch fault, fault_address contains the instruction pointer.
+	 */
+	if (vcpu->arch.mmu->direct_map &&
+	    likely(!(error_code & PFERR_FETCH_MASK)) {
 		vcpu->arch.gpa_available = true;
 		vcpu->arch.gpa_val = cr2;
 	}

Sure, but I think we'll have to extend the check.

Searching the logs I've found:

    kvm/x86: re-translate broken translation that caused EPT violation
    
    Signed-off-by: Mircea Cirjaliu [off-list ref]

 arch/x86/kvm/x86.c | 1 +
 1 file changed, 1 insertion(+)

/home/b/kvmi@9cad844~1/arch/x86/kvm/x86.c:4757,4762 - /home/b/kvmi@9cad844/arch/x86/kvm/x86.c:4757,4763
  	 */
  	if (vcpu->arch.gpa_available &&
  	    emulator_can_use_gpa(ctxt) &&
+ 	    (vcpu->arch.error_code & PFERR_GUEST_FINAL_MASK) &&
  	    (addr & ~PAGE_MASK) == (vcpu->arch.gpa_val & ~PAGE_MASK)) {
  		gpa = vcpu->arch.gpa_val;
  		ret = vcpu_is_mmio_gpa(vcpu, addr, gpa, write);

Yes, adding that check makes sense as well (still in kvm_mmu_page_fault).

Paolo
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization