From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH RFC 03/15] xen: arm: allocate dom0 memory separately from preparing the dtb Date: Thu, 10 Oct 2013 15:38:56 +0100 Message-ID: <5256BC00.1020309@linaro.org> References: <1381163944.21562.129.camel@kazak.uk.xensource.com> <1381164001-1446-3-git-send-email-ian.campbell@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1381164001-1446-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 Cc: stefano.stabellini@eu.citrix.com, tim@xen.org, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 10/07/2013 05:39 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 seaprate phase and then fill in the memory nodes in the separate > DTB based on the result while generating the DTB. > > This allows us to move kernel parsing before DTB setup. > > Signed-off-by: Ian Campbell > --- > xen/arch/arm/domain_build.c | 94 ++++++++++++++++++++++++++++++------------- > 1 file changed, 65 insertions(+), 29 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index fb1fa56..1287934 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -63,11 +63,8 @@ struct vcpu *__init alloc_dom0_vcpu0(void) > return alloc_vcpu(dom0, 0, 0); > } > > -static int set_memory_reg_11(struct domain *d, struct kernel_info *kinfo, > - const struct dt_property *pp, > - const struct dt_device_node *np, __be32 *new_cell) > +static int allocate_memory_11(struct domain *d, struct kernel_info *kinfo) This function always return 0 or panic if an error occurred. Perhaps you can move the return type to void? > { > - int reg_size = dt_cells_to_size(dt_n_addr_cells(np) + dt_n_size_cells(np)); > paddr_t start; > paddr_t size; > struct page_info *pg; > @@ -91,40 +88,53 @@ static int set_memory_reg_11(struct domain *d, struct kernel_info *kinfo, > if ( res ) > panic("Unable to add pages in DOM0: %d\n", res); > > - dt_set_range(&new_cell, np, start, size); > - > kinfo->mem.bank[0].start = start; > kinfo->mem.bank[0].size = size; > kinfo->mem.nr_banks = 1; > > - return reg_size; > + kinfo->unassigned_mem -= size; > + > + return 0; > } > > -static int set_memory_reg(struct domain *d, struct kernel_info *kinfo, > - const struct dt_property *pp, > - const struct dt_device_node *np, __be32 *new_cell) > +static int allocate_memory(struct domain *d, struct kernel_info *kinfo) Same here. > { > - int reg_size = dt_cells_to_size(dt_n_addr_cells(np) + dt_n_size_cells(np)); > - int l = 0; > + > + const struct dt_device_node *memory; > + const void *reg; > + u32 reg_len, reg_size; > + int l = 0, ret; > unsigned int bank = 0; > - u64 start; > - u64 size; > - int ret; > > if ( platform_has_quirk(PLATFORM_QUIRK_DOM0_MAPPING_11) ) > - return set_memory_reg_11(d, kinfo, pp, np, new_cell); > + return allocate_memory_11(d, kinfo); > + > + memory = dt_find_node_by_type(NULL, "memory"); Can we try to have the same way to retrieve the memory node in each place? - common/device_tree.c: looking by memory@unit - arch/arm/domain_build.c:write_properties: looking only the exact node name "memory" - here: looking by type "memory" Furthermore, your are assuming that there is only one memory node in DTS tree. Perhaps a loop is better here? -- Julien Grall