| Message ID | 20180822215225.28703-3-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Don't re-{try,execute} L2 instrs | expand |
On 22/08/2018 23:52, Sean Christopherson wrote: > reexecute_instruction(), added by commit a6f177efaa58 ("KVM: Reenter > guest after emulation failure if due to access to non-mmio address"), > is intended to return true if and only if emulation failed due to a > write to a shadowed page, i.e. MMIO emulation that happens to fail > should never be retried. Set EMULTYPE_NO_REEXECUTE when we know we > are emulating a (alleged) MMIO access to document that we should not > re-execute the instruction. > > This is purely a documentation update as reexecute_instruction() does > not re-execute instructions whose faulting GPA corresponds to an error > or no-slot PFN, e.g. EMULTYPE_NO_REEXECUTE was added by commit > 991eebf9f8e5 ("KVM: VMX: do not try to reexecute failed instruction > while emulating invalid guest state") for reasons entirely unrelated > to MMIO. Case in point, mmio_info_in_cache() always returns false for > nested VMs, e.g. EMULTYPE_NO_REEXECUTE won't be set if MMIO emulation > for a nested VM is triggered outside of the handle_mmio_page_fault() > path. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > arch/x86/kvm/mmu.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > index a282321329b5..1c152fd9fc7c 100644 > --- a/arch/x86/kvm/mmu.c > +++ b/arch/x86/kvm/mmu.c > @@ -5231,7 +5231,7 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u64 error_code, > if (unlikely(error_code & PFERR_RSVD_MASK)) { > r = handle_mmio_page_fault(vcpu, cr2, direct); > if (r == RET_PF_EMULATE) { > - emulation_type = 0; > + emulation_type = EMULTYPE_NO_REEXECUTE; > goto emulate; > } > } > @@ -5261,7 +5261,7 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u64 error_code, > } > > if (mmio_info_in_cache(vcpu, cr2, direct)) > - emulation_type = 0; > + emulation_type = EMULTYPE_NO_REEXECUTE; > emulate: > /* > * On AMD platforms, under certain conditions insn_len may be zero on #NPF. > Even better, EMULTYPE_NO_REEXECUTE is never set if EMULTYPE_RETRY is set, and vice versa. The sole exception is rsm_interception in svm.c that should actually specify EMULTYPE_NO_REEXECUTE but doesn't. So it seems to me that they can be unified to a single EMULTYPE_RETRY, which makes sense since they do basically the same thing---only EMULTYPE_NO_REEXECUTE is a bit more desperado because decoding has failed. Paolo
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c index a282321329b5..1c152fd9fc7c 100644 --- a/arch/x86/kvm/mmu.c +++ b/arch/x86/kvm/mmu.c @@ -5231,7 +5231,7 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u64 error_code, if (unlikely(error_code & PFERR_RSVD_MASK)) { r = handle_mmio_page_fault(vcpu, cr2, direct); if (r == RET_PF_EMULATE) { - emulation_type = 0; + emulation_type = EMULTYPE_NO_REEXECUTE; goto emulate; } } @@ -5261,7 +5261,7 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u64 error_code, } if (mmio_info_in_cache(vcpu, cr2, direct)) - emulation_type = 0; + emulation_type = EMULTYPE_NO_REEXECUTE; emulate: /* * On AMD platforms, under certain conditions insn_len may be zero on #NPF.
reexecute_instruction(), added by commit a6f177efaa58 ("KVM: Reenter guest after emulation failure if due to access to non-mmio address"), is intended to return true if and only if emulation failed due to a write to a shadowed page, i.e. MMIO emulation that happens to fail should never be retried. Set EMULTYPE_NO_REEXECUTE when we know we are emulating a (alleged) MMIO access to document that we should not re-execute the instruction. This is purely a documentation update as reexecute_instruction() does not re-execute instructions whose faulting GPA corresponds to an error or no-slot PFN, e.g. EMULTYPE_NO_REEXECUTE was added by commit 991eebf9f8e5 ("KVM: VMX: do not try to reexecute failed instruction while emulating invalid guest state") for reasons entirely unrelated to MMIO. Case in point, mmio_info_in_cache() always returns false for nested VMs, e.g. EMULTYPE_NO_REEXECUTE won't be set if MMIO emulation for a nested VM is triggered outside of the handle_mmio_page_fault() path. Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- arch/x86/kvm/mmu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)