Re: [Xen-devel] [PATCH] Make get_page_from_l1e refcount correctly onforeign pagetables.

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

At 08:42 +0100 on 14 May (1242290576), Jan Beulich wrote:
> >>> Tim Deegan <Tim.Deegan@xxxxxxxxxx> 13.05.09 18:07 >>>
> >
> >Hypercalls from dom0 can end up doing resyncs on HVM guests' out-of-sync
> >shadow pagetables.  At that point the check against current->domain in 
> >get_page_from_l1e() triggers the typecount exemption for foreign mappings
> >and a writeable typecount gets lost. 
> >
> >Make the foreign-domain check explicit by having get_page_from_l1e_for(),
> >which understands both the dom whose right are being used and the dom
> >whose pagetables are being updated.  Most callers of get_page_from_l1e() 
> >have both the same (instead of one hard-coded to current->domain as before).
> >
> >Analysis and fix from David Lively.
> 
> I have to admit that the change to mod_l1_entry() look suspicious to me -
> as I understand it, the third parameter of get_page_from_l1e_for() represents
> the target domain, and that's what FOREIGNDOM is to be used for.

Possibly.  IIUC get_page_from_l1e_for()'s first domain argument is the
domain whose rights we are testing; so e.g. dom0 mapping domU memory
uses FOREIGNDOM there to say "this should be domU's page".  The second
argument (whose pagetables are these) has always implicitly been "mine",
i.e. current->domain.  Again correct when dom0 maps domU's page. 

In the case we're trying to fix, although current->domain is dom0 (who
is making a shadow control hypercall) the pagetables belong to domU.

In the mod_l1_entry() call, get_page_from_l1e_for(nl1e, FOREIGNDOM, d)
just gets us the old behaviour (since d == vcpu->domain).

> Perhaps the whole thing gets more convoluted because of c/s 19383, which
> added a vcpu parameter for no apparent reason (current is used for that
> everywhere afaict).

That parameter is used so that dom0 can modify a PV domU's pagetables
using the MMU ops (so dom0 tools can offline a suspect page without
domU's help).  In that case, I think the current patch actually fixes a
latent bug in refcounting foreign mappings from the target domU to a
third (HVM) domain, that was missed in cs 19383.

This is all pretty confusing, though, and Keir may correct me on any or
all of it. :)

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

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