From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 3/3] xen: arm: rework dom0 initrd and dtb placement Date: Wed, 09 Apr 2014 14:09:29 +0100 Message-ID: <53454689.7040705@linaro.org> References: <1397044259.6275.15.camel@kazak.uk.xensource.com> <1397044276-30185-3-git-send-email-ian.campbell@citrix.com> <53454319.9060204@linaro.org> <1397048252.6275.37.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1397048252.6275.37.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: stefano.stabellini@eu.citrix.com, tim@xen.org, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 04/09/2014 01:57 PM, Ian Campbell wrote: > On Wed, 2014-04-09 at 13:54 +0100, Julien Grall wrote: >> Hi Ian, >> >> On 04/09/2014 12:51 PM, Ian Campbell wrote: >>> This now uses the same decision tree as libxc (which is much easier to test). >>> >>> The main change is to explictly handle the placement at 128MB or end of RAM as >> >> s/explictly/explicitly/ >> s/mopules/modules/ > > Gah, fingers not working right today it seems. > >>> since it is redundant with the case where placing them at the end of RAM ends >>> up abutting the kernel. >>> >>> Also round the kernel size up to a 2MB boundary. >> >> A bit surprised that it was not done before :). > > Me too. > >>> Signed-off-by: Ian Campbell >>> --- >>> I'm sure to regret playing with this yet again... >>> --- >>> xen/arch/arm/kernel.c | 22 ++++++++++++---------- >>> 1 file changed, 12 insertions(+), 10 deletions(-) >>> >>> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c >>> index bc625a4..1102392 100644 >>> --- a/xen/arch/arm/kernel.c >>> +++ b/xen/arch/arm/kernel.c >>> @@ -77,7 +77,7 @@ static void place_modules(struct kernel_info *info, >>> const paddr_t rambase = info->mem.bank[0].start; >>> const paddr_t ramsize = info->mem.bank[0].size; >>> const paddr_t ramend = rambase + ramsize; >>> - const paddr_t kernsize = kernend - kernbase; >>> + const paddr_t kernsize = ROUNDUP(kernend, MB(2)) - kernbase; >> >> You are using ROUNDUP(kernend, MB(2)) in few places, why didn't you >> roundup directly kernend? > > It's passed as a paramter, and it's not possible to round it before > using it here (code before declarations), so I'd have to make kernsize > non-const and initialise it after the rounding. I didn't think it was > worth it. In this case I'm lost... You are mixing kernend and ROUNDUP(kernend, MB(2)). As I understand, we don't need to round up on the second if expression (see code below). + if ( ramend >= ram128mb + modsize && kernend < ram128mb ) + modbase = ram128mb; + else if ( ramend - modsize > ROUNDUP(kernend, MB(2)) ) + modbase = ramend - modsize; + else if ( kernbase - rambase > modsize ) Regards, -- Julien Grall