From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH RFC 5/7] xen: arm: rewrite start of day page table and cpu bring up Date: Tue, 17 Sep 2013 17:55:20 +0100 Message-ID: <52388978.2080309@linaro.org> References: <1379381846.11304.73.camel@hastur.hellion.org.uk> <1379382050-11821-5-git-send-email-ian.campbell@citrix.com> <523880CC.7020703@linaro.org> <1379435765.11304.185.camel@hastur.hellion.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1379435765.11304.185.camel@hastur.hellion.org.uk> 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, Andre Przywara , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 09/17/2013 05:36 PM, Ian Campbell wrote: > On Tue, 2013-09-17 at 17:18 +0100, Julien Grall wrote: >>> + /* Now we can install the fixmap and dtb mappings, since we >>> + * don't need the 1:1 map any more */ >>> + dsb sy >>> + ldr r1, =boot_second >>> +#if defined(EARLY_PRINTK) >>> + /* xen_fixmap pagetable */ >> >> Can you add a comment to explain why we don't need to map the fixmap >> when early printk is not enabled? > > It's covered by the overall description of the boot tables layout which > is in mm.c and referenced elsewhere in this file. Is the suficient? I just saw the comment, I'm fine with it. It took me several minutes to find where :). > [..] >>> diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h >>> index f460e9c..f616807 100644 >>> --- a/xen/include/asm-arm/platform.h >>> +++ b/xen/include/asm-arm/platform.h >>> @@ -14,6 +14,11 @@ struct platform_desc { >>> /* Platform initialization */ >>> int (*init)(void); >>> int (*init_time)(void); >>> +#ifdef CONFIG_ARM_32 >>> + /* SMP */ >>> + int (*cpu_init)(int cpu); >> >> I don't think a cpu_init callback is usefull. An smp_init callback would >> be better. >> >> This will allow you to move the sys_flags check for the versatile >> express in smp_init. > > I wondered if there might be platforms with differeing mbox addresses > for different CPU. e.g. the armv8 stuff (which doesn't use this path) > makes provisions for this. I believe, it's the case on midway. Andre, can you confirm? But, I think this code can be merge in cpu_up. > > But I'll make the suggested change -- we can also refactor or add a > second callback if such a platform shows up. > > You didn't trim your quotes, I hope I didn't miss any comments (I > trimmed a bunch of the s/xen_/boot_/ ones... Will do a thorough > sweep...) You didn't miss any comments, next time I will trim my quotes. -- Julien Grall