[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [V9 PATCH 6/8] pvh dom0: Add and remove foreign pages
At 19:09 -0700 on 24 Apr (1398362965), Mukesh Rathor wrote: > On Thu, 24 Apr 2014 11:46:41 +0200 > Tim Deegan <tim@xxxxxxx> wrote: > > > At 19:21 -0700 on 23 Apr (1398277311), Mukesh Rathor wrote: > > > On Thu, 17 Apr 2014 14:58:42 +0100 > > > "Jan Beulich" <JBeulich@xxxxxxxx> wrote: > > > > > > > >>> On 17.04.14 at 14:36, <tim@xxxxxxx> wrote: > > > > > At 07:50 +0100 on 17 Apr (1397717440), Jan Beulich wrote: > > > > >> >>> On 17.04.14 at 03:37, <mukesh.rathor@xxxxxxxxxx> wrote: > > > > >> > On Wed, 16 Apr 2014 17:00:35 +0100 > > > > >> > "Jan Beulich" <JBeulich@xxxxxxxx> wrote: > ...... > > > > > That said, it should be easy enough only to refcount on leaf > > > > > entries, right? I can't see how that would be incompatible > > > > > with the intermediate-node changes that Jan is working on. > > > > > > > > Right - keeping the macro as is and introducing a derived > > > > function to handle the extra requirements on leaf entries would > > > > seem quite okay, so long as error propagation can be done > > > > properly. > > > > > > Ok, how about something like the following? In case of get_page > > > failure, not sure EINVAL is the best one to return, EBUSY? > > > > This goes back to having refcounts open-coded. Having the refcounts > > open-coded around the atomic_write_ept_entry() in ept_set_entry() > > means there are now places where the epte can change without > > maintaining the refcount invariants: ept_change_entry_type_page(), for > > example. > > Correct, altho, at present I've checks in p2m paths to not allow foreign > types to come down to such calls. > > > I would _much_ prefer to have atomic_write_ept_entry() DTRT -- it > > would have to know the difference between leaf and non-leaf entries, > > and return an error code. I'd also be OK with having two > > atomic_write ops, one for leaf and one for non-leaf, with appropriate > > ASSERT()s on the contents. > > Ok, how about something like shown further below? (I think > it would be more simpler to have one atomic_write ops, instead of two) I like this approach. I think the test for foreignness needs to check that it's dealing with a leaf node, e.g. by checking that bit .sp == 1 and setting that bit in all leaf entries. At the moment we clear .sp in 4k entries but the docs say it's ignored by hardware so we could set it, I think -- it would need an adjustment of ept_next_level() to detect superpages by checking the level. > One thing, on returning error code, since at present there are no > paths allowing superpages for foreign types, it appears I'd not need > to worry about undoing the ept_split_super_page(), so I added > assert in there. Sound right? Yep, an ASSERT to catch foreign superpage mappings is useful. > Finally, I can leave other callers of atomic_write_ept_entry as is, or > add ASSERTs for rc == 0. LMK. I would be inclined to ASSERT(). Cheers, Tim. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |