Re: [PATCH 3/3] Reenter guest after instruction emulation failure if emulation was due to access to non-mmio address.

[Gleb Natapov <gleb@redhat.com>]

On Thu, Jul 08, 2010 at 12:15:18PM +0300, Avi Kivity wrote:
> On 07/08/2010 12:06 PM, Avi Kivity wrote:
> >On 07/07/2010 08:16 PM, Gleb Natapov wrote:
> >>When shadow pages are in use sometimes KVM try to emulate an instruction
> >>when it accesses a shadowed page. If emulation fails KVM un-shadows the
> >>page and reenter guest to allow vcpu to execute the instruction. If page
> >>is not in shadow page hash KVM assumes that this was attempt to do MMIO
> >>and reports emulation failure to userspace since there is no way to fix
> >>the situation. This logic has a race though. If two vcpus tries to write
> >>to the same shadowed page simultaneously both will enter emulator, but
> >>only one of them will find the page in shadow page hash since
> >>the one who
> >>founds it also removes it from there, so another cpu will report failure
> >>to userspace and will abort the guest.
> >>
> >>Fix this by checking (in addition to checking shadowed page hash) that
> >>page that caused the emulation belongs to valid memory slot. If it is
> >>then reenter the guest to allow vcpu to reexecute the instruction.
> >>
> >>
> >>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >>index 7070b41..dd7b241 100644
> >>--- a/arch/x86/kvm/x86.c
> >>+++ b/arch/x86/kvm/x86.c
> >>@@ -4000,6 +4000,8 @@ int emulate_instruction(struct kvm_vcpu *vcpu,
> >>          if (r)  {
> >>              if (kvm_mmu_unprotect_page_virt(vcpu, cr2))
> >>                  return EMULATE_DONE;
> >>+            if (!kvm_is_error_hva(gfn_to_hva(vcpu->kvm, cr2)))
> >>+                return EMULATE_DONE;
> >
> >cr2 is a gva, not a gfn.
> 
> btw, that will mean another page walk, so better fold into
> kvm_mmu_unprotect_page_virt() (which needs a new name, since it does
> more than unprotect a page now).
> 
But this code will be taken very rarely and usually on the way to
failure anyway, do you think additional page walk is a problem?

> Say, kvm_make_guest_writeable().
> 
> -- 
> error compiling committee.c: too many arguments to function

--
			Gleb.