On Mon, Mar 17, 2008 at 01:11:11PM +0200, Avi Kivity wrote:
> >+ up_read(&kvm->slots_lock);
> >
> >So as to avoid rmap_nuke, since that will be done through the madvise()
> >path.
> >
> >
>
> Why not do it in userspace?
I don't see any way of knowing whether you have or not mmu notifiers from
userspace except adding an ioctl.
> I'll likely merge zap directly into the backwards compatibility support
> (it won't ever appear in mainline since mmu notifiers will be merged
> sooner).
Even with mmu notifiers QEMU needs to ask the host kernel "do you have
mmu notifiers?" before zapping any page, otherwise the guest will crash.
So some method of finding if the host kernel has mmu notifiers needs to
be in place even in mainline.
And what about allocation of ioctl numbers? Will you reserve the number
used for ZAP_GFN on mainline?
> >>Did you find out what's causing the errors in the first place (if zap is
> >>not used)? It worries me greatly.
> >>
> >
> >Yes, the problem is that the rmap code does not handle the qemu process
> >mappings from vanishing while there is a present rmap. If that happens,
> >and there is a fault for a gfn whose qemu mapping has been removed, a
> >different physical zero page will be allocated:
> >
> > rmap a -> gfn 0 -> physical host page 0
> > mapping for gfn 0 gets removed
> > guest faults in gfn 0 through the same pte "chain"
> > rmap a -> gfn 0 -> physical host page 1
> >
> >When instantiating the shadow mapping for the second time, the
> >"is_rmap_pte" check succeeds, so we release the reference grabbed by
> >gfn_to_page() at mmu_set_spte(). We now have a shadow mapping pointing
> >to a physical page without having an additional reference on that page.
> >
> >The following makes the host not crash under such a condition, but the
> >condition itself is invalid leading to inconsistent state on the guest.
> >So IMHO it shouldnt be allowed to happen in the first place.
> >
> >
>
> The only way to prevent it is with mmu notifiers, which we may not have
> for 2.6.25. So I'd like to send this for 2.6.25-rc.
Well, you can prevent it by never allowing a page from being removed while
there are shadow mappings for it. So while this fixes the host crash, it
won't fix the guest crash.
KVM: MMU: handle page removal with shadow mapping
Do not assume that a shadow mapping will always point to the same host
frame number. Fixes crash with madvise(MADV_DONTNEED).
Signed-off-by: Marcelo Tosatti <[EMAIL PROTECTED]>
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index f0cdfba..4caa1a0 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1016,8 +1016,16 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64
*shadow_pte,
struct page *page)
{
u64 spte;
- int was_rmapped = is_rmap_pte(*shadow_pte);
+ int was_rmapped = 0;
int was_writeble = is_writeble_pte(*shadow_pte);
+ hfn_t host_pfn = (*shadow_pte & PT64_BASE_ADDR_MASK) >> PAGE_SHIFT;
+
+ if (is_rmap_pte(*shadow_pte)) {
+ if (host_pfn != page_to_pfn(page))
+ rmap_remove(vcpu->kvm, shadow_pte);
+ else
+ was_rmapped = 1;
+ }
/*
* If we overwrite a PTE page pointer with a 2MB PMD, unlink
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
