From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 7/8] xen/arm: Set foreign page type to p2m_map_foreign Date: Thu, 05 Dec 2013 16:41:48 +0000 Message-ID: <52A0ACCC.3030903@linaro.org> References: <1386258131-755-1-git-send-email-julien.grall@linaro.org> <1386258131-755-8-git-send-email-julien.grall@linaro.org> <1386261264.20047.117.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Vobzu-0002qv-BD for xen-devel@lists.xenproject.org; Thu, 05 Dec 2013 16:41:54 +0000 Received: by mail-bk0-f45.google.com with SMTP id mx13so7218660bkb.18 for ; Thu, 05 Dec 2013 08:41:52 -0800 (PST) In-Reply-To: <1386261264.20047.117.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xenproject.org, tim@xen.org, stefano.stabellini@citrix.com, patches@linaro.org List-Id: xen-devel@lists.xenproject.org On 12/05/2013 04:34 PM, Ian Campbell wrote: > On Thu, 2013-12-05 at 15:42 +0000, Julien Grall wrote: >> Xen needs to know that the current page belongs to another domain. Also take >> the refcount to this page. >> >> Signed-off-by: Julien Grall >> --- >> xen/arch/arm/mm.c | 14 +++++++++----- >> 1 file changed, 9 insertions(+), 5 deletions(-) >> >> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c >> index 960c872..bf383a7 100644 >> --- a/xen/arch/arm/mm.c >> +++ b/xen/arch/arm/mm.c >> @@ -977,6 +977,7 @@ static int xenmem_add_to_physmap_one( >> { >> unsigned long mfn = 0; >> int rc; >> + p2m_type_t t = p2m_ram_rw; > > Can we set this explicitly on a per-mapspace basis please, so the > compiler will complain about an uninitialised variable if we forget to > add the type for a new map space, instead of silently creating writable > ram mappings. Will do. >> >> 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? > 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? -- Julien Grall