From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCH 2/5] device tree, arm: supply a flat device tree to dom0 Date: Mon, 2 Apr 2012 10:21:00 +0100 Message-ID: <4F796F7C.4070400@citrix.com> References: <1332443876-28506-1-git-send-email-david.vrabel@citrix.com> <1332443876-28506-3-git-send-email-david.vrabel@citrix.com> <1333123166.15932.113.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: <1333123166.15932.113.camel@zakaz.uk.xensource.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: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On 30/03/12 16:59, Ian Campbell wrote: > On Thu, 2012-03-22 at 19:17 +0000, David Vrabel wrote: >> From: David Vrabel >> >> Build a flat device tree for dom0 based on the one supplied to Xen. >> The following changes are made: >> >> * In the /chosen node, the xen,dom0-bootargs parameter is renamed to >> bootargs. >> >> * In all memory nodes, the reg parameters are adjusted to reflect >> the amount of memory dom0 can use. The p2m is updated using this >> info. >> >> Support for passing ATAGS to dom0 is removed. >> >> Signed-off-by: David Vrabel >> --- >> xen/arch/arm/domain_build.c | 257 +++++++++++++++++++++++++++++------ >> xen/arch/arm/kernel.c | 2 +- >> xen/arch/arm/kernel.h | 8 +- >> xen/common/device_tree.c | 47 ++++-- >> xen/include/xen/device_tree.h | 8 + >> xen/include/xen/libfdt/libfdt_env.h | 3 + >> 6 files changed, 265 insertions(+), 60 deletions(-) >> >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index 15632f7..b4c0452 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -6,6 +6,10 @@ >> #include >> #include >> #include >> +#include >> +#include >> +#include >> +#include >> >> #include "gic.h" >> #include "kernel.h" >> @@ -13,6 +17,13 @@ >> static unsigned int __initdata opt_dom0_max_vcpus; >> integer_param("dom0_max_vcpus", opt_dom0_max_vcpus); >> >> +/* >> + * Amount of extra space required to dom0's device tree. No new nodes >> + * are added (yet) but one terminating reserve map entry (16 bytes) is >> + * added. >> + */ >> +#define DOM0_FDT_EXTRA_SIZE (sizeof(struct fdt_reserve_entry)) >> + >> struct vcpu *__init alloc_dom0_vcpu0(void) >> { >> if ( opt_dom0_max_vcpus == 0 ) >> @@ -28,43 +39,210 @@ struct vcpu *__init alloc_dom0_vcpu0(void) >> return alloc_vcpu(dom0, 0, 0); >> } >> >> -extern void guest_mode_entry(void); >> +static void set_memory_reg(struct domain *d, struct kernel_info *kinfo, >> + const void *fdt, >> + const u32 *cell, int address_cells, int size_cells, >> + u32 *new_cell, int *len) >> +{ >> + int reg_size = (address_cells + size_cells) * sizeof(*cell); >> + int l; >> + u64 start; >> + u64 size; >> + >> + l = *len; > >> + >> + while ( kinfo->unassigned_mem > 0 && l >= reg_size >> + && kinfo->mem.nr_banks < NR_MEM_BANKS ) >> + { >> + device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); >> + if ( size > kinfo->unassigned_mem ) >> + size = kinfo->unassigned_mem; >> + >> + device_tree_set_reg(&new_cell, address_cells, size_cells, start, size); > > This assumes/requires that address_cells and size_cells a 1, right? If > they end up being 2 or more then device_tree_get_reg will trample on the > stack via &start and &size. No. address_cells and size_cells can be 1 or 2. This is enough for 64 addresses. >> + >> + printk("Populate P2M %#llx->%#llx\n", start, start + size); >> + p2m_populate_ram(d, start, start + size); >> + kinfo->mem.bank[kinfo->mem.nr_banks].start = start; >> + kinfo->mem.bank[kinfo->mem.nr_banks].size = size; >> + kinfo->mem.nr_banks++; >> + kinfo->unassigned_mem -= size; >> + >> + l -= reg_size; >> + } >> + >> + *len -= l; > > I had a bit of trouble following the logic wrt l and the updating of > *len in this function. I've updated this as per your suggestion. >> --- a/xen/include/xen/libfdt/libfdt_env.h >> +++ b/xen/include/xen/libfdt/libfdt_env.h >> @@ -13,4 +13,7 @@ >> #define fdt64_to_cpu(x) be64_to_cpu(x) >> #define cpu_to_fdt64(x) cpu_to_be64(x) >> >> +/* Xen-specific libfdt error code. */ >> +#define FDT_ERR_XEN(err) (FDT_ERR_MAX + (err)) > > Looks like the only user is FDT_ERR_XEN(ENOMEM) which could as well be > FDT_ERR_NOSPACE? No. FDT_ERR_NOSPACE means the buffer isn't large enough to add new nodes etc. This needs to be a different error code from a memory allocation failure. > FWIW I think adding a new error code would be a fair reason to diverge > in a controlled way from the pristine upstream source. I don't know what you're asking for here. David