From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v5 07/19] xen: arm: allocate dom0 memory separately from preparing the dtb Date: Thu, 14 Nov 2013 00:52:50 +0000 Message-ID: <52841EE2.9070300@linaro.org> References: <1384366234.29080.8.camel@kazak.uk.xensource.com> <1384366285-29277-7-git-send-email-ian.campbell@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1384366285-29277-7-git-send-email-ian.campbell@citrix.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 , xen-devel@lists.xen.org Cc: tim@xen.org, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 11/13/2013 06:11 PM, Ian Campbell wrote: > Mixing these two together is a pain, it forces us to prepare the dtb before > processing the kernel which means we don't know whether the guest is 32- or > 64-bit while we construct its DTB. > > Instead split out the memory allocation (including 1:1 workaround handling) > and p2m setup into a separate phase and then create a memory node in the DTB > based on the result. > > This allows us to move kernel parsing before DTB setup. > > As part of this it was also necessary to rework where the decision regarding > the placement of the DTB and initrd in RAM was made. It is now made when > loading the kernel, which allows it to make use of the zImage/ELF specific > information and therefore to make decisions based on complete knowledge and do > it right rather than guessing in prepare_dtb and relying on a later check to > see if things worked. > > Signed-off-by: Ian Campbell > --- > v3: Also rework module placement, v2 broke boot because dtb_paddr wasn't set > soon enough. This ends up cleaner anyway. > v2: Fixed typo in the commit log > Handle multiple memory nodes as well as individual nodes with several > entries in them. > Strip the original memory node and recreate rather than trying to modify. > --- > xen/arch/arm/domain_build.c | 203 ++++++++++++++++++++++--------------------- > xen/arch/arm/kernel.c | 80 +++++++++++------ > xen/arch/arm/kernel.h | 2 - > 3 files changed, 158 insertions(+), 127 deletions(-) [..] > @@ -236,17 +267,9 @@ static int kernel_try_zimage32_prepare(struct kernel_info *info, > paddr_t load_end; > > load_end = info->mem.bank[0].start + info->mem.bank[0].size; > - load_end = MIN(info->mem.bank[0].start + (128<<20), load_end); > - > - /* > - * FDT is loaded above 128M or as high as possible, so the > - * only way we can clash is if we have <=128MB, in which case > - * FDT will be right at the end and so dtb_paddr will be below > - * the proposed kernel load address. Move the kernel down if > - * necessary. > - */ > - if ( load_end >= info->dtb_paddr ) > - load_end = info->dtb_paddr; > + load_end = MIN(info->mem.bank[0].start + MB(128), load_end); > + > + load_end += MB(2); I didn't notice this line during my previous review, why do you add MB(2)? -- Julien Grall