Re: [Xen-devel] [PATCH v2 08/10] xen/arm: Implement xen_rem_foreign_from_p2m

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-12-09 at 17:08 +0000, Julien Grall wrote:
> On 12/09/2013 04:31 PM, Ian Campbell wrote:
> > On Mon, 2013-12-09 at 03:34 +0000, Julien Grall wrote:
> >> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> >>
> >> ---
> >>     Changes in v2:
> >>         - Introduce the patch
> >> ---
> >>  xen/arch/arm/mm.c         |   16 ++++++++++++++++
> >>  xen/include/asm-arm/p2m.h |    6 +-----
> >>  2 files changed, 17 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> >> index 960c872..ba51f6e 100644
> >> --- a/xen/arch/arm/mm.c
> >> +++ b/xen/arch/arm/mm.c
> >> @@ -968,6 +968,22 @@ void share_xen_page_with_privileged_guests(
> >>      share_xen_page_with_guest(page, dom_xen, readonly);
> >>  }
> >>  
> >> +int xenmem_rem_foreign_from_p2m(struct domain *d, unsigned long gpfn)
> >> +{
> >> +    unsigned long mfn = gmfn_to_mfn(d, gpfn);
> >> +    if ( !mfn_valid(mfn) )
> >> +    {
> >> +        gdprintk(XENLOG_WARNING, "Invalid mfn for gpfn:%lx domid:%d\n",
> >> +                 gpfn, d->domain_id);
> > 
> > Is this guest triggerable? Or does it indicate a mistake when we made
> > the mapping?
> 
> It indicates a mistake when we made the mapping. Perhaps an ASSERT is
> better here.

In that case either works for me.

> > 
> >> +        return -EINVAL;
> >> +    }
> >> +
> >> +    guest_physmap_remove_page(d, gpfn, mfn, 0);
> >> +    put_page(mfn_to_page(mfn));
> > 
> > Some of this could become a common helper with create_p2m_entries and
> > the relinquish stuff in the previous patch.
> 
> The best solution is to move put_page in guest_physmap_remove_page. But
> as we didn't plan to handle refcount for each mapping in Xen 4.4,
> perhaps we can delay it?

You can move it and still have it be conditional I think, since g_p_r_p
can see the type in the pte, can't it?

Ian.


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