From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH] xen: arm: handle initrd addresses above the 4G boundary Date: Fri, 06 Dec 2013 15:50:16 +0000 Message-ID: <52A1F238.90900@linaro.org> References: <1386254540-14231-1-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: <1386254540-14231-1-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 12/05/2013 02:42 PM, Ian Campbell wrote: > The Xgene platform has no RAM below 4G. > > The /chosen/linux,initrd-* properties do not have "reg" semantics and > therefore #*-size are not used when interpreting. Instead they are are simply > numbers which are interpreted according to the properties length. > > Fix this both when parsing the entry in the host DTB and when creating the > dom0 DTB. For dom0 we simply hardcode a 64-bit size, this is acceptable > even for a 32-bit guest. > > Signed-off-by: Ian Campbell > --- > xen/arch/arm/domain_build.c | 19 +++++++++++++------ > xen/common/device_tree.c | 26 ++++++++++++++++++++++---- > xen/include/xen/device_tree.h | 8 +++++++- > 3 files changed, 42 insertions(+), 11 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index 7b9031b..aac24eb 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -222,11 +222,12 @@ static int write_properties(struct domain *d, struct kernel_info *kinfo, > */ > if ( early_info.modules.module[MOD_INITRD].size ) > { > - res = fdt_property_cell(kinfo->fdt, "linux,initrd-start", 0); > + u64 a = 0; > + res = fdt_property(kinfo->fdt, "linux,initrd-start", &a, sizeof(a)); > if ( res ) > return res; > > - res = fdt_property_cell(kinfo->fdt, "linux,initrd-end", 0); > + res = fdt_property(kinfo->fdt, "linux,initrd-end", &a, sizeof(a)); > if ( res ) > return res; > } > @@ -923,6 +924,8 @@ static void initrd_load(struct kernel_info *kinfo) > unsigned long offs; > int node; > int res; > + u32 val[2]; > + __be32 *cellp; > > if ( !len ) > return; > @@ -935,13 +938,17 @@ static void initrd_load(struct kernel_info *kinfo) > if ( node < 0 ) > panic("Cannot find the /chosen node"); > > - res = fdt_setprop_inplace_cell(kinfo->fdt, node, "linux,initrd-start", > - load_addr); > + cellp = (__be32 *)val; > + dt_set_cell(&cellp, ARRAY_SIZE(val), load_addr); > + res = fdt_setprop_inplace(kinfo->fdt, node, "linux,initrd-start", > + val, sizeof(val)); > if ( res ) > panic("Cannot fix up \"linux,initrd-start\" property"); > > - res = fdt_setprop_inplace_cell(kinfo->fdt, node, "linux,initrd-end", > - load_addr + len); > + cellp = (__be32 *)val; > + dt_set_cell(&cellp, ARRAY_SIZE(val), load_addr + len); > + res = fdt_setprop_inplace(kinfo->fdt, node, "linux,initrd-end", > + val, sizeof(val)); > if ( res ) > panic("Cannot fix up \"linux,initrd-end\" property"); I'm wondering if we can move this code to write_properties ... It seems that we compute the initrd address in kernel_*_load function, but I think we can move place_modules at the end of kernel_prepare. > > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c > index 943b181..2b1b364 100644 > --- a/xen/common/device_tree.c > +++ b/xen/common/device_tree.c > @@ -390,18 +390,36 @@ static void __init process_chosen_node(const void *fdt, int node, > const char *name, > u32 address_cells, u32 size_cells) > { > + const struct fdt_property *prop; > struct dt_mb_module *mod = &early_info.modules.module[MOD_INITRD]; > - u32 start, end; > + paddr_t start, end; > + int len; > > dt_printk("Checking for initrd in /chosen\n"); > > - start = device_tree_get_u32(fdt, node, "linux,initrd-start", 0); > - end = device_tree_get_u32(fdt, node, "linux,initrd-end", 0); > + prop = fdt_get_property(fdt, node, "linux,initrd-start", &len); > + if ( !prop || len < sizeof(u32) || len > sizeof(u64) ) > + { > + dt_printk("err start %p len %d %ld %ld\n", prop, len, sizeof(u32), sizeof(u64)); With a user see this message it's not clear where the error message comes from ... Can you add 'initrd' or something similar in the message? > + return; > + } > + start = dt_read_number((void *)&prop->data, dt_size_to_cells(len)); > + > + prop = fdt_get_property(fdt, node, "linux,initrd-end", &len); > + if ( !prop || len < sizeof(u32) || len > sizeof(u64) ) > + { > + dt_printk("err end %p len %d %ld %ld\n", prop, len, sizeof(u32), sizeof(u64)); Same here. > + return; > + } > + end = dt_read_number((void *)&prop->data, dt_size_to_cells(len)); > > if ( !start || !end || (start >= end) ) > + { > + dt_printk("err limits\n"); Same here. > return; > + } > > - dt_printk("Initrd 0x%x-0x%x\n", start, end); > + dt_printk("Initrd %"PRIpaddr"-%"PRIpaddr"\n", start, end); > > mod->start = start; > mod->size = end - start; > diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h > index da78c9f..9f23b07 100644 > --- a/xen/include/xen/device_tree.h > +++ b/xen/include/xen/device_tree.h > @@ -256,12 +256,18 @@ static inline u64 dt_read_number(const __be32 *cell, int size) > return r; > } > > -/* Helper to convert a number of cells in bytes */ > +/* Helper to convert a number of cells to bytes */ > static inline int dt_cells_to_size(int size) > { > return (size * sizeof (u32)); > } > > +/* Helper to convert a number of bytes to cells */ > +static inline int dt_size_to_cells(int cells) > +{ > + return (cells / sizeof(u32)); > +} > + s/cells/bytes/ Perhaps you update the comment by saying we are assuming bytes is 4-byte aligned. -- Julien Grall