[Xen-devel] Races with p2m translations and p2m updates

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

The p2m translation functions were written at a time when the p2m was,
for the most part, write-once (or, write a bit a the beginning and
then stop).  As a result, most of the code which calls gfn_to_mfn*()
assumes that the p2m isn't going to change: once it has the mfn, it
will map it, write to it, and unmap it, even though there is no
guarantee that the mfn is still in the p2m, or in fact even owned by
the same domain.

The write-once assumption is already broken by the populate-on-demand
feature, and will be broken even more by future features such as page
sharing and page swapping.  The problem with all of these features is
that they remove or change mfns that are backing gfns and then
immediately use them for another purpose: mapping them at a different
gfn, or giving them to a different domain.

I'm planning some work to fix the issue, but I wanted to run it by the
developer list first.

First: The key issue is with features that will, "behind the scenes"
so-to-speak, remove mfns from the p2m and then use them somewhere
else.  This includes the "zero-page sweep" for the populate-on-demand
code, paging out, copy-on-write and page-sharing, probably page
de-fragmentation as well.

I've chatted through a bunch of the issues with Tim Deegan, and the
following is our proposal:

* Change the p2m lock to a multiple-reader-single-writer lock.
gfn_to_mfn*() will return with read lock still held if the translation
succeeds.  The caller is then responsible to release the lock when
it's done using the resulting mfn from the translation.
 + This will take care of all races internal to xen, such as emulation
code and so on.
* If the caller of gfn_to_mfn*()  is going to use the mfn after
holding the lock, it must increase the reference count for the mfn.
 + This will take care of external references, such as those held by qemu
* No p2m entry may be changed if there are outstanding references to
it (refcount > 1).

This means that functions which modify the p2m (e.g.,
guest_physmap_remove_page or guest_physmap_add_entry) may fail, and
callers will need to deal with that.  Hopefully this won't actually
affect any of the current callers, except logdirty, which will have to
be special-cased to leave pages "dirty" which have outstanding
references.  The PoD code already checks the reference count before
reclaiming a page.

Thoughts?  Anything major we missed?

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel