| Message ID | 54087343.5050409@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 04, 2014 at 04:12:19PM +0200, Paolo Bonzini wrote: > Il 04/09/2014 09:02, Gleb Natapov ha scritto: > > On Tue, Sep 02, 2014 at 05:13:49PM +0200, Paolo Bonzini wrote: > >> > This is required for the following patch to work correctly. If a nested page > >> > fault happens during emulation, we must inject a vmexit, not a page fault. > >> > Luckily we already have the required machinery: it is enough to return > >> > X86EMUL_INTERCEPTED instead of X86EMUL_PROPAGATE_FAULT. > >> > > > I wonder why this patch is needed. X86EMUL_PROPAGATE_FAULT causes > > ctxt->have_exception to be set to true in x86_emulate_insn(). > > x86_emulate_instruction() checks ctxt->have_exception and calls > > inject_emulated_exception() if it is true. inject_emulated_exception() > > calls kvm_propagate_fault() where we check if the fault was nested and > > generate vmexit or a page fault accordingly. > > Good question. :) > > If you do that, KVM gets down to the "if (writeback)" and writes the > ctxt->eip from L2 into the L1 EIP. Heh, that's a bummer. We should not write back if an instruction caused a vmexit. > > Possibly this patch can be replaced by just this? > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 022513b..475e979 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5312,7 +5312,7 @@ restart: > > if (ctxt->have_exception) { > inject_emulated_exception(vcpu); > - r = EMULATE_DONE; > + return EMULATE_DONE; If there was no vmexit we still want to writeback. Perhaps: writeback = inject_emulated_exception(vcpu); and return false if there was vmexit due to nested page fault (or any fault, can't L1 ask for #GP/#UD intercept that need to be handled here too?) > } else if (vcpu->arch.pio.count) { > if (!vcpu->arch.pio.in) { > /* FIXME: return into emulator if single-stepping. */ > > But I'm not sure how to test it, and I like the idea of treating nested page > faults like other nested vmexits during emulation (which is what this patch > does). IMO exits due to instruction intercept and exits due to other interceptable events that may happen during instruction emulation are sufficiently different to be handled slightly different. If my assumption about #GP above are correct with current approach it can be easily handled inside inject_emulated_exception(). -- Gleb. -- 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
Il 04/09/2014 17:05, Gleb Natapov ha scritto: >> > if (ctxt->have_exception) { >> > inject_emulated_exception(vcpu); >> > - r = EMULATE_DONE; >> > + return EMULATE_DONE; > If there was no vmexit we still want to writeback. Perhaps: > writeback = inject_emulated_exception(vcpu); > and return false if there was vmexit due to nested page fault (or any fault, > can't L1 ask for #GP/#UD intercept that need to be handled here too?) > Sounds good. Paolo -- 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/kvm/x86.c b/arch/x86/kvm/x86.c index 022513b..475e979 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5312,7 +5312,7 @@ restart: if (ctxt->have_exception) { inject_emulated_exception(vcpu); - r = EMULATE_DONE; + return EMULATE_DONE; } else if (vcpu->arch.pio.count) { if (!vcpu->arch.pio.in) { /* FIXME: return into emulator if single-stepping. */ But I'm not sure how to test it, and I like the idea of treating nested page faults like other nested vmexits during emulation (which is what this patch does). If I included this patch, I could then remove kvm_propagate_fault like (I think) this: diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 92493e10937c..e096db566ac2 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -4910,9 +4902,10 @@ static void toggle_interruptibility(struct kvm_vcpu *vcpu, u32 mask) static void inject_emulated_exception(struct kvm_vcpu *vcpu) { struct x86_emulate_ctxt *ctxt = &vcpu->arch.emulate_ctxt; - if (ctxt->exception.vector == PF_VECTOR) - kvm_propagate_fault(vcpu, &ctxt->exception); - else if (ctxt->exception.error_code_valid) + if (ctxt->exception.vector == PF_VECTOR) { + WARN_ON(fault->nested_page_fault); + vcpu->arch.walk_mmu->inject_page_fault(vcpu, fault); + } else if (ctxt->exception.error_code_valid) kvm_queue_exception_e(vcpu, ctxt->exception.vector, ctxt->exception.error_code); else