| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <65a10dd8-5fae-350f-b597-f8f0261da766@redhat.com>
Date: Thu, 8 Dec 2016 15:52:32 +0100
From: Paolo Bonzini <pbonzini@...hat.com>
To: Brijesh Singh <brijesh.singh@....com>, kvm@...r.kernel.org
Cc: thomas.lendacky@....com, rkrcmar@...hat.com, joro@...tes.org,
x86@...nel.org, linux-kernel@...r.kernel.org, mingo@...hat.com,
hpa@...or.com, tglx@...utronix.de, bp@...e.de
Subject: Re: [PATCH v2 3/3] kvm: svm: Use the hardware provided GPA instead of
page walk
On 23/11/2016 18:02, Brijesh Singh wrote:
> From: Tom Lendacky <thomas.lendacky@....com>
>
> 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@....com>
> Reviewed-by: Borislav Petkov <bp@...e.de>
> Signed-off-by: Brijesh Singh <brijesh.singh@....com>
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
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@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Powered by blists - more mailing lists