Re: [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Wed, 28 May 2014 12:06:35 +0200
Tim Deegan <tim@xen.org> wrote:

> At 16:50 -0700 on 23 May (1400860212), Mukesh Rathor wrote:
> > On Sat, 24 May 2014 01:08:49 +0200
> > Tim Deegan <tim@xen.org> wrote:
> > 
> > > At 15:37 -0700 on 23 May (1400855820), Mukesh Rathor wrote:
> > > > On Fri, 23 May 2014 21:05:34 +0200
> > > > Tim Deegan <tim@xen.org> wrote:
> > > > 
> > > > > At 16:30 -0700 on 22 May (1400772630), Mukesh Rathor wrote:
> > > > > > In this patch, a new function, p2m_add_foreign(), is added
> > > > > > to map pages from a foreign guest into dom0 for various
> > > > > > purposes like domU creation, running xentrace, etc... Such
> > > > > > pages are typed p2m_map_foreign.  Note, it is the nature of
> > > > > > such pages that a refcnt is held during their stay in the
> > > > > > p2m. The refcnt is added and released in the low level ept
> > > > > > function atomic_write_ept_entry. That macro is converted to
> > > > > > a function to allow for such refcounting, which only
> > > > > > applies to leaf entries in the ept. Furthermore, please
> > > > > > note that paging/sharing is disabled if the controlling or
> > > > > > hardware domain is pvh. Any enabling of those features
> > > > > > would need to ensure refcnt are properly maintained for
> > > > > > foreign types, or paging/sharing is skipped for foreign
> > > > > > types.
> > > > > > 
> > > > > > Also, we change get_pg_owner so it allows foreign mappings
> > > > > > for pvh.
> > > > > 
> > > > > But you no longer actually call get_pg_owner() for PVH
> > > > > domains, right? So that hunk should go away.  With that done,
> > > > 
> > > > Hi Tim,
> > > > 
> > > > We actually need get_pg_owner for the mmuext call by the
> > > > toolstack when building a PV domain, doing pinning operations
> > > > on the guest table.
> > > 
> > > Ah, I see.  Let's handle that in a separate patch then, since it's
> > > now unrelated to foreign mappings in PVH any more.
> > > 
> > > Having the change where it is seems fine, but I think the correct
> > > test is (is_pv() && paging_mode_translate()) rather than
> > > (!is_pvh() && paging_mode_translate()) -- it's a weakness of the
> > > PV pagetable ops that's being avoided here, rather than any
> > > special treatment for PVH.
> > 
> > Good point, but Jan had a concern on that when I had dropped the if
> > statement completely, that it would allow HVM guests to go thru. 
> > Hence !is_pvh to let hvm guest continue to fail.
> 
> Well, in that case I don't insist on it.  I'll look at it again as
> part of the PVH->HVM merge.

I think that would be better, as there are several options you mentioned
in the other email, and some of it is cleanup not pvh related. For now
I'll just add the !is_pvh check, so that hvm guests will continue to
be restricted.  Please ack the patch in V16 on its way. If you prefer
( is_pv && paging) instead, I don't mind you just changing it during
commit either, if hvm going thru is not an issue.

thanks
Mukesh