Re: [Xen-devel] [PATCH v4 08/11] xen/arm: Handle remove foreign mapping

[Tim Deegan <tim@xxxxxxx>]

At 11:55 +0000 on 16 Dec (1387191318), Ian Campbell wrote:
> On Mon, 2013-12-16 at 12:51 +0100, Tim Deegan wrote:
> > At 19:37 +0000 on 13 Dec (1386959858), Julien Grall wrote:
> > > @@ -693,12 +694,21 @@ 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 autotranslate guest, (eg pvh), the gfn could be mapped to 
> > > a mfn
> > > +         * from foreign domain by the 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 the page.
> > > +         */
> > > +        page = get_page_from_gfn(d, xrfp.gpfn, &p2mt, P2M_ALLOC);
> > >          if ( page )
> > >          {
> > >              guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 
> > > 0);
> > >              put_page(page);
> > >          }
> > > +        else if ( p2m_is_foreign(p2mt) )
> > > +            rc = p2m_remove_foreign(d, xrfp.gpfn);
> > 
> > This doesn't seem like the right interface -- having special cases
> > like this in the callers is something we slipped into in x86 for a lot
> > of the paging/sharing code and it's not nice.  I think maybe we can
> > just have get_page_from_gfn() DTRT for foreign (and grant) entries.
> 
> DYM guest_physmap_remove_page?

I think I mean both get_page_from_gfn and guest_physmap_remove_page.

> I asked Mukesh a few times to make get_page_from_gfn handle the foreign
> page refcounting and return a valid struct page_info *.

Yes, that sounds like a better place for it, if it can be done
safely.  It would need a bit of care at the existing callers to make
sure they're not going to break when handed a foreign-owned page (but
they should be audited anyway for this new case).

Tim.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel