| Message ID | 147916176259.16347.7828367075943432152.stgit@brijesh-build-machine (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 14/11/2016 23:16, Brijesh Singh wrote: > From: Tom Lendacky <thomas.lendacky@amd.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@amd.com> > Reviewed-by: Borislav Petkov <bp@suse.de> > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > arch/x86/include/asm/kvm_emulate.h | 3 +++ > arch/x86/include/asm/kvm_host.h | 3 +++ > arch/x86/kvm/svm.c | 9 ++++++++- > arch/x86/kvm/x86.c | 17 ++++++++++++++++- > 4 files changed, 30 insertions(+), 2 deletions(-) > > 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..b442c5a 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -275,6 +275,9 @@ static int avic; > module_param(avic, int, S_IRUGO); > #endif > > +/* EXITINFO2 contains valid GPA */ > +static bool gpa_avail = true; > + > /* AVIC VM ID bit masks and lock */ > static DECLARE_BITMAP(avic_vm_id_bitmap, AVIC_VM_ID_NR); > static DEFINE_SPINLOCK(avic_vm_id_lock); > @@ -1055,8 +1058,10 @@ static __init int svm_hardware_setup(void) > goto err; > } > > - if (!boot_cpu_has(X86_FEATURE_NPT)) > + if (!boot_cpu_has(X86_FEATURE_NPT)) { > npt_enabled = false; > + gpa_avail = false; This is not necessary, since you will never have exit_code == SVM_EXIT_NPF && !gpa_avail. > + } > > if (npt_enabled && !npt) { > printk(KERN_INFO "kvm: Nested Paging disabled\n"); > @@ -4192,6 +4197,8 @@ static int handle_exit(struct kvm_vcpu *vcpu) > vcpu->arch.cr0 = svm->vmcb->save.cr0; > if (npt_enabled) > vcpu->arch.cr3 = svm->vmcb->save.cr3; > + if (gpa_avail) > + vcpu->arch.gpa_available = (exit_code == SVM_EXIT_NPF); The "if", and the body moved just before return svm_exit_handlers[exit_code](svm); I'll try doing a similar patch for Intel too, for better testing. > > if (unlikely(svm->nested.exit_required)) { > nested_svm_vmexit(svm); > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index d02aeff..c290794 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -4420,7 +4420,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; > @@ -5542,6 +5554,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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11/21/2016 9:07 AM, Paolo Bonzini wrote: > > > On 14/11/2016 23:16, Brijesh Singh wrote: >> From: Tom Lendacky <thomas.lendacky@amd.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@amd.com> >> Reviewed-by: Borislav Petkov <bp@suse.de> >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> >> --- >> arch/x86/include/asm/kvm_emulate.h | 3 +++ >> arch/x86/include/asm/kvm_host.h | 3 +++ >> arch/x86/kvm/svm.c | 9 ++++++++- >> arch/x86/kvm/x86.c | 17 ++++++++++++++++- >> 4 files changed, 30 insertions(+), 2 deletions(-) >> >> 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..b442c5a 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -275,6 +275,9 @@ static int avic; >> module_param(avic, int, S_IRUGO); >> #endif >> >> +/* EXITINFO2 contains valid GPA */ >> +static bool gpa_avail = true; >> + >> /* AVIC VM ID bit masks and lock */ >> static DECLARE_BITMAP(avic_vm_id_bitmap, AVIC_VM_ID_NR); >> static DEFINE_SPINLOCK(avic_vm_id_lock); >> @@ -1055,8 +1058,10 @@ static __init int svm_hardware_setup(void) >> goto err; >> } >> >> - if (!boot_cpu_has(X86_FEATURE_NPT)) >> + if (!boot_cpu_has(X86_FEATURE_NPT)) { >> npt_enabled = false; >> + gpa_avail = false; > > This is not necessary, since you will never have exit_code == > SVM_EXIT_NPF && !gpa_avail. Ah, yes, that is a bit redundant. We'll get rid of the gpa_avail flag. Thanks, Tom > >> + } >> >> if (npt_enabled && !npt) { >> printk(KERN_INFO "kvm: Nested Paging disabled\n"); >> @@ -4192,6 +4197,8 @@ static int handle_exit(struct kvm_vcpu *vcpu) >> vcpu->arch.cr0 = svm->vmcb->save.cr0; >> if (npt_enabled) >> vcpu->arch.cr3 = svm->vmcb->save.cr3; >> + if (gpa_avail) >> + vcpu->arch.gpa_available = (exit_code == SVM_EXIT_NPF); > > The "if", and the body moved just before > > return svm_exit_handlers[exit_code](svm); > > I'll try doing a similar patch for Intel too, for better testing. > >> >> if (unlikely(svm->nested.exit_required)) { >> nested_svm_vmexit(svm); >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index d02aeff..c290794 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -4420,7 +4420,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; >> @@ -5542,6 +5554,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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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..b442c5a 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -275,6 +275,9 @@ static int avic; module_param(avic, int, S_IRUGO); #endif +/* EXITINFO2 contains valid GPA */ +static bool gpa_avail = true; + /* AVIC VM ID bit masks and lock */ static DECLARE_BITMAP(avic_vm_id_bitmap, AVIC_VM_ID_NR); static DEFINE_SPINLOCK(avic_vm_id_lock); @@ -1055,8 +1058,10 @@ static __init int svm_hardware_setup(void) goto err; } - if (!boot_cpu_has(X86_FEATURE_NPT)) + if (!boot_cpu_has(X86_FEATURE_NPT)) { npt_enabled = false; + gpa_avail = false; + } if (npt_enabled && !npt) { printk(KERN_INFO "kvm: Nested Paging disabled\n"); @@ -4192,6 +4197,8 @@ static int handle_exit(struct kvm_vcpu *vcpu) vcpu->arch.cr0 = svm->vmcb->save.cr0; if (npt_enabled) vcpu->arch.cr3 = svm->vmcb->save.cr3; + if (gpa_avail) + vcpu->arch.gpa_available = (exit_code == SVM_EXIT_NPF); if (unlikely(svm->nested.exit_required)) { nested_svm_vmexit(svm); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index d02aeff..c290794 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4420,7 +4420,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; @@ -5542,6 +5554,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)