Re: [V5 PATCH 6/7] pvh dom0: Add and remove foreign pages

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

On Thu, 5 Dec 2013 17:15:04 -0800
Mukesh Rathor <mukesh.rathor@oracle.com> wrote:

> On Thu, 5 Dec 2013 12:00:24 +0000
> Ian Campbell <Ian.Campbell@citrix.com> wrote:
> 
> > On Wed, 2013-12-04 at 18:05 -0800, Mukesh Rathor wrote:
> > > diff --git a/xen/common/memory.c b/xen/common/memory.c
> > > index eb7b72b..ae11828 100644
> > > --- a/xen/common/memory.c
> > > +++ b/xen/common/memory.c
> > > @@ -675,9 +675,11 @@ long do_memory_op(unsigned long cmd,
> > > XEN_GUEST_HANDLE_PARAM(void) arg) 
> > >      case XENMEM_remove_from_physmap:
> > >      {
> > > +        unsigned long mfn;
> > >          struct xen_remove_from_physmap xrfp;
> > >          struct page_info *page;
> > >          struct domain *d;
> > > +        p2m_type_t p2mt;
> > >  
> > >          if ( copy_from_guest(&xrfp, arg, 1) )
> > >              return -EFAULT;
> > > @@ -693,11 +695,41 @@ long do_memory_op(unsigned long cmd,
> > > XEN_GUEST_HANDLE_PARAM(void) arg) return rc;
> > >          }
> > >  
> > > -        page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
> > > -        if ( page )
> > > +        /*
> > > +         * if PVH, the gfn could be mapped to a mfn from foreign
> > > domain by the
> > 
> > s/PVH/autotranslated/ I think?
> > 
> > > +         * user space tool during domain creation. We need to
> > > check for that,
> > > +         * free it up from the p2m, and release refcnt on it. In
> > > such a case,
> > > +         * page would be NULL and the following call would not
> > > have refcnt'd
> > 
> > Why is page NULL in this case? I'd have thought that
> > get_page_from_gfn could handle the p2m_foreign case internally and
> > still return the page, with the ref count taken too.
> > 
> > Doing that would cause a lot of the magic, and in particular the
> > ifdef, in the following code to disappear.
> 
> I had brought this up earlier this year (that's how old this patch
> is). get_page_from_gfn can't be used because the mfn owner is foreign
> domain and not domain "d", and get_page() will barf.

Rephrase: get_page_from_gfn can't be changed to handle p2m_foreign because...

thanks
mukesh