From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3 03/14] xen: arm: allocate dom0 memory separately from preparing the dtb Date: Thu, 07 Nov 2013 23:18:27 -0800 Message-ID: <527C9043.1050702@linaro.org> References: <1383842678.3189.15.camel@kazak.uk.xensource.com> <1383842696-8598-3-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: <1383842696-8598-3-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: ian.jackson@eu.citrix.com, tim@xen.org, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 11/07/2013 08:44 AM, 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. Your solution to create the memory node won't work in some case. From the EPAR, memory nodes can be everywhere. So we can have a device tree like that: / { motherboard { #address-cells = 2 #size-cells = 2 memory { device_type = "memory"; reg = < ... > } } } Here, the root (/) has #address-cells = 2 and #size-cells = 1, that is the default value. As you will create the memory node in slash, you will loose 1 cell of the size. > This allows us to move kernel parsing before DTB setup. Why do you want to move the kernel parsing earlier? Xen don't use d->arch.type during dom0 building. > 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 | 198 ++++++++++++++++++++++--------------------- > xen/arch/arm/kernel.c | 80 +++++++++++------ > xen/arch/arm/kernel.h | 2 - > 3 files changed, 155 insertions(+), 125 deletions(-) > [..] > static void kernel_elf_load(struct kernel_info *info) > { > + place_modules(info, > + info->elf.parms.virt_kstart, > + info->elf.parms.virt_kend); > + > printk("Loading ELF image into guest memory\n"); > info->elf.elf.dest_base = (void*)(unsigned long)info->elf.parms.virt_kstart; > info->elf.elf.dest_size = > info->elf.parms.virt_kend - info->elf.parms.virt_kstart; > + spurious line? > elf_load_binary(&info->elf.elf); > > printk("Free temporary kernel buffer\n"); > @@ -321,7 +348,6 @@ static int kernel_try_elf_prepare(struct kernel_info *info, > */ > info->entry = info->elf.parms.virt_entry; > info->load = kernel_elf_load; > - info->check_overlap = NULL; > > if ( elf_check_broken(&info->elf.elf) ) > printk("Xen: warning: ELF kernel broken: %s\n", > diff --git a/xen/arch/arm/kernel.h b/xen/arch/arm/kernel.h > index debf590..b48c2c9 100644 > --- a/xen/arch/arm/kernel.h > +++ b/xen/arch/arm/kernel.h > @@ -40,8 +40,6 @@ struct kernel_info { > }; > > void (*load)(struct kernel_info *info); > - /* Callback to check overlap between the kernel and the device tree */ > - void (*check_overlap)(struct kernel_info *kinfo); > int load_attr; > }; > > -- Julien Grall