From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH] xen/arm: p2m: Don't create new table when the mapping is removed Date: Thu, 19 Dec 2013 15:28:18 +0000 Message-ID: <52B31092.9080600@linaro.org> References: <1387387873-3269-1-git-send-email-julien.grall@linaro.org> <1387445896.9925.7.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VtfWR-0001y1-7F for xen-devel@lists.xenproject.org; Thu, 19 Dec 2013 15:28:23 +0000 Received: by mail-ea0-f175.google.com with SMTP id z10so550477ead.20 for ; Thu, 19 Dec 2013 07:28:21 -0800 (PST) In-Reply-To: <1387445896.9925.7.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, george.dunlap@citrix.com, stefano.stabellini@citrix.com, patches@linaro.org List-Id: xen-devel@lists.xenproject.org On 12/19/2013 09:38 AM, Ian Campbell wrote: > On Wed, 2013-12-18 at 17:31 +0000, Julien Grall wrote: >> When Xen is removing/relinquishing mapping, it will create second/third tables >> if they don't exist. >> >> Non-existent table means the address range was never mapped, so Xen can safely >> skip them. >> >> Signed-off-by: Julien Grall >> >> --- >> Release: This is an improvement for Xen 4.4. It will save time during >> relinquish phase and avoid dummy allocation. >> The downside is the patch is modifying p2m loop which is used everywhere. >> --- >> xen/arch/arm/p2m.c | 22 ++++++++++++++++++++-- >> 1 file changed, 20 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c >> index d24a6fc..9ef8819 100644 >> --- a/xen/arch/arm/p2m.c >> +++ b/xen/arch/arm/p2m.c >> @@ -246,10 +246,12 @@ static int create_p2m_entries(struct domain *d, >> cur_first_offset = ~0, >> cur_second_offset = ~0; >> unsigned long count = 0; >> + bool_t populate = (op == INSERT || op == ALLOCATE); >> >> spin_lock(&p2m->lock); >> >> - for(addr = start_gpaddr; addr < end_gpaddr; addr += PAGE_SIZE) >> + addr = start_gpaddr; >> + while ( addr < end_gpaddr ) >> { >> if ( cur_first_page != p2m_first_level_index(addr) ) >> { >> @@ -265,8 +267,15 @@ static int create_p2m_entries(struct domain *d, >> >> if ( !first[first_table_offset(addr)].p2m.valid ) >> { >> + if ( !populate ) >> + { >> + addr += FIRST_SIZE; > > I think this subtly does the wrong thing if addr is not FIRST_SIZE > aligned, which it might be on the first iteration. That will skip the > start of the next 1st level block. > > I think addr needs to be rounded up to the next first boundary. Right, I will send a new version of this patch. -- Julien Grall