Re: [PATCH] x86: kvm: Avoid guest page table walk when gpa_available is set

[Brijesh Singh]

Hi Radim,

> This will probably return false negatives when then vcpu->arch.gpa_val
> couldn't be used anyway (possibly after a VM exits), so it is hard to
> draw a conclusion.
>
> I would really like if we had a solution that passed the gpa inside
> function parameters.
>
> (Btw. cr2 can also be a virtual address, so we can call it gpa directly)

I've tried the below patch and it seems to work fine. This does not consider
PIO case and as you rightly pointed PIO should trigger #NPF relatively rarely.
At least so far in my runs I have not seen PIO causing #NPF. If this sounds
acceptable approach then I can submit v2 with these changes and remove gpa_val
additional.

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 13c132b..c040e38 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4662,17 +4662,17 @@ static int emulator_read_write_onepage(unsigned long addr, void *val,
         */
        if (vcpu->arch.gpa_available &&
            emulator_can_use_gpa(ctxt) &&
-           vcpu_is_mmio_gpa(vcpu, addr, exception->address, write) &&
            (addr & ~PAGE_MASK) == (exception->address & ~PAGE_MASK)) {
                gpa = exception->address;
-               goto mmio;
+               ret = vcpu_is_mmio_gpa(vcpu, addr, gpa, write);
+       } else {
+               dump_stack();
+               ret = vcpu_mmio_gva_to_gpa(vcpu, addr, &gpa, exception, write);
+
+               if (ret < 0)
+                       return X86EMUL_PROPAGATE_FAULT;
        }
 
-       ret = vcpu_mmio_gva_to_gpa(vcpu, addr, &gpa, exception, write);
-
-       if (ret < 0)
-               return X86EMUL_PROPAGATE_FAULT;
-
        /* For APIC access vmexit */
        if (ret)
                goto mmio;
@@ -7056,11 +7056,11 @@ static int vcpu_run(struct kvm_vcpu *vcpu)
        return r;
 }
 
-static inline int complete_emulated_io(struct kvm_vcpu *vcpu)
+static inline int complete_emulated_io(struct kvm_vcpu *vcpu, unsigned long cr2)
 {
        int r;
        vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu);
-       r = emulate_instruction(vcpu, EMULTYPE_NO_DECODE);
+       r = x86_emulate_instruction(vcpu, cr2, EMULTYPE_NO_DECODE, NULL, 0);
        srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);
        if (r != EMULATE_DONE)
                return 0;
@@ -7071,7 +7071,7 @@ static int complete_emulated_pio(struct kvm_vcpu *vcpu)
 {
        BUG_ON(!vcpu->arch.pio.count);
 
-       return complete_emulated_io(vcpu);
+       return complete_emulated_io(vcpu, 0);
 }
 /*
@@ -7097,6 +7097,7 @@ static int complete_emulated_mmio(struct kvm_vcpu *vcpu)
        struct kvm_run *run = vcpu->run;
        struct kvm_mmio_fragment *frag;
        unsigned len;
+       gpa_t gpa;
 
        BUG_ON(!vcpu->mmio_needed);
 
@@ -7106,6 +7107,12 @@ static int complete_emulated_mmio(struct kvm_vcpu *vcpu)
        if (!vcpu->mmio_is_write)
                memcpy(frag->data, run->mmio.data, len);
 
+       /*
+       * lets use the GPA from previous guest page table walk to avoid yet
+       * another guest page table walk when completing the MMIO page-fault.
+        */
+       gpa = frag->gpa;
+
        if (frag->len <= 8) {
                /* Switch to the next fragment. */
                frag++;
@@ -7124,7 +7131,7 @@ static int complete_emulated_mmio(struct kvm_vcpu *vcpu)
                if (vcpu->mmio_is_write)
                        return 1;
                vcpu->mmio_read_completed = 1;
-               return complete_emulated_io(vcpu);
+               return complete_emulated_io(vcpu, gpa);
        }
 
        run->exit_reason = KVM_EXIT_MMIO;