On 11.08.2017 18:52, Paolo Bonzini wrote:
> From: Brijesh Singh <brijesh.singh@xxxxxxx>
>
> When a guest causes a page fault which requires emulation, the
> vcpu->arch.gpa_available flag is set to indicate that cr2 contains a
> valid GPA.
>
> Currently, emulator_read_write_onepage() makes use of gpa_available flag
> to avoid a guest page walk for a known MMIO regions. Lets not limit
> the gpa_available optimization to just MMIO region. The patch extends
> the check to avoid page walk whenever gpa_available flag is set.
Can we move that to a separate patch?
>
> Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
> [Fix EPT=0 according to Wanpeng Li's fix, plus ensure VMX also uses the
> new code. - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
[...]
> +++ b/arch/x86/kvm/svm.c
> @@ -4235,8 +4235,7 @@ static int handle_exit(struct kvm_vcpu *vcpu)
> u32 exit_code = svm->vmcb->control.exit_code;
>
> trace_kvm_exit(exit_code, vcpu, KVM_ISA_SVM);
> -
> - vcpu->arch.gpa_available = (exit_code == SVM_EXIT_NPF);
> + vcpu->arch.gpa_available = false;
Can we move resetting to false to vcpu_enter_guest()? It should be reset
before handle_exit() is called for both cases.
(maybe an additional patch)
>
> if (!is_cr_intercept(svm, INTERCEPT_CR0_WRITE))
> vcpu->arch.cr0 = svm->vmcb->save.cr0;
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 45fb0ea78ee8..79efb00dd70d 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6393,9 +6393,7 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
> error_code |= (exit_qualification & 0x100) != 0 ?
> PFERR_GUEST_FINAL_MASK : PFERR_GUEST_PAGE_MASK;
>
> - vcpu->arch.gpa_available = true;
> vcpu->arch.exit_qualification = exit_qualification;
> -
> return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0);
> }
>
> @@ -6410,7 +6408,6 @@ static int handle_ept_misconfig(struct kvm_vcpu *vcpu)
> return kvm_skip_emulated_instruction(vcpu);
> }
>
> - vcpu->arch.gpa_available = true;
> ret = kvm_mmu_page_fault(vcpu, gpa, PFERR_RSVD_MASK, NULL, 0);
> if (ret >= 0)
> return ret;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index e40a779711a9..bb05b705c295 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -4657,25 +4657,18 @@ 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;
> + (addr & ~PAGE_MASK) == (vcpu->arch.gpa_val & ~PAGE_MASK)) {
> + gpa = vcpu->arch.gpa_val;
> + ret = vcpu_is_mmio_gpa(vcpu, addr, gpa, write);
> + } else {
> + ret = vcpu_mmio_gva_to_gpa(vcpu, addr, &gpa, exception, write);
> }
>
> - 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;
> -
> - if (ops->read_write_emulate(vcpu, gpa, val, bytes))
> + if (!ret && ops->read_write_emulate(vcpu, gpa, val, bytes))
> return X86EMUL_CONTINUE;
>
> -mmio:
> /*
> * Is this MMIO handled locally?
> */
>
Looks good to me.
--
Thanks,
David
