From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCH v3 5/8] xen/arm: preserve DTB mappings Date: Tue, 8 Jan 2013 19:04:57 +0000 Message-ID: <50EC6DD9.5060703@citrix.com> References: <1355856402-26614-5-git-send-email-stefano.stabellini@eu.citrix.com> <1355920600.14620.371.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: "xen-devel@lists.xensource.com" , "Tim (Xen.org)" , Ian Campbell List-Id: xen-devel@lists.xenproject.org On 08/01/13 18:33, Stefano Stabellini wrote: > On Wed, 19 Dec 2012, Ian Campbell wrote: >>> @@ -295,12 +296,23 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr) >>> write_pte(xen_second + second_linear_offset(XEN_VIRT_START), pte); >>> /* TLBFLUSH and ISB would be needed here, but wait until we set WXN */ >>> >>> + /* preserve the DTB mapping a little while longer */ >> >> Not so much "preserve" as "put back" after this function clobbered it. >> >> I'm not convinced by this effectively open coding of a "stack" of >> mappings in the BOOT_MISC_VIRT_START slot. IMHO this slot should only be >> used for ephemeral mappings within individual functions or over short >> spans of code -- otherwise there is plenty of potential for clashes >> which lead to hard to decipher bugs. >> >> Can we not map the boot fdt as and when we need it instead of trying to >> preserve a mapping? > > I don't like this idea because it means that the device initialization > functions in start_xen that are called after setup_pagetables and before > setup_mm, like gic_init, suddenly become position dependent: they cannot > be moved after setup_mm. The mapping of the DTB into BOOT_MISC_VIRT_START was only for use by device_tree_early_init(). Everything else should be using the final mapping setup in setup_mm(). I think this means setup_mm() needs to immediately after setup_pagestables() and before gic_init(), pl011_init() etc. David