Re: [Xen-devel] [PATCH 7/8] xen/arm: Set foreign page type to p2m_map_foreign

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-12-05 at 16:41 +0000, Julien Grall wrote:

> >>
> >>       switch ( space )
> >>       {
> >> @@ -1019,8 +1020,8 @@ static int xenmem_add_to_physmap_one(
> >>           break;
> >>       case XENMAPSPACE_gmfn_foreign:
> >>       {
> >> -        paddr_t maddr;
> >>           struct domain *od;
> >> +        struct page_info *page;
> >>           od = rcu_lock_domain_by_any_id(foreign_domid);
> >>           if ( od == NULL )
> >>               return -ESRCH;
> >> @@ -1032,15 +1033,18 @@ static int xenmem_add_to_physmap_one(
> >>               return rc;
> >>           }
> >>
> >> -        maddr = p2m_lookup(od, pfn_to_paddr(idx));
> >> -        if ( maddr == INVALID_PADDR )
> >> +        /* Take refcount to the foreign domain page.
> >
> > "on the foreign" (and maybe "domain's" or just "foreign page"
> >
> >> +         * Refcount will be release in XENMEM_remove_from_physmap */
> >
> > "will be released".
> >
> > The refcount will also be removed by p2m_teardown. That probably needs
> > changing to do an actual walk of the p2m tearing things down, which is a
> > pain.
> >
> > Stefano's now obsolete dma pinning series had a patch which added a
> > fairly generic walker in it which might be reusable.
> 
> Do you have a link to this patch?

I was afraid you'd say that...
/scrobbles

"xen/arm: introduce a generic p2m walker and use it in p2m_lookup". v6
is msgid
<1380298560-29352-2-git-send-email-stefano.stabellini@xxxxxxxxxxxxx>. I
don't recall if that was the last one though.

> 
> > The walk might need to support continuations though.
> >
> > I wonder if this ref ought to be taken in create_p2m_entries for all
> > entries and not just foreign ones, and then released in the appropriate
> > places.
> 
> If I'm not mistaken, Xen already takes a ref when the page is allocated 
> for the domain. Why would we need to take another ref for these pages?

There's a nice symmetry to all p2m entries taking a ref count, and it
makes the teardown a bit simpler since you can just drop the ref every
time without worrying about the type.

In some sense (and I'm not sure it does make sense) you could consider
there to be one reference for owning the page and a second for mapping
it.

I don't think a domain can own a page which isn't mapped in its p2m,
because decrease reservation frees the page, although the 1:1 ballooning
workaround has some aspect of that to it.

Ian.


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