From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v4 08/11] xen/arm: Handle remove foreign mapping Date: Mon, 16 Dec 2013 15:34:06 +0000 Message-ID: <52AF1D6E.7080402@linaro.org> References: <1386963461-6520-1-git-send-email-julien.grall@linaro.org> <1386963461-6520-9-git-send-email-julien.grall@linaro.org> <20131216115131.GB35881@deinos.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VsaBP-0000si-PL for xen-devel@lists.xenproject.org; Mon, 16 Dec 2013 15:34:12 +0000 Received: by mail-we0-f175.google.com with SMTP id t60so4704169wes.20 for ; Mon, 16 Dec 2013 07:34:10 -0800 (PST) In-Reply-To: <20131216115131.GB35881@deinos.phlegethon.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tim Deegan Cc: Keir Fraser , ian.campbell@citrix.com, patches@linaro.org, stefano.stabellini@citrix.com, Jan Beulich , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On 12/16/2013 11:51 AM, 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. > > Also, the comment will have been out of data by the time the x86 > version of this code is finished, as we won't be handling the refcount > at this level. :) I will remove the comment and modify get_page_from_gfn to handle foreign mapping. -- Julien Grall