| Message ID | 1431893048-5214-24-git-send-email-parth.dixit@linaro.org |
|---|---|
| State | New |
| Headers | show |
+shannon On 2 June 2015 at 23:10, Julien Grall <julien.grall@citrix.com> wrote: > Hi Parth, > > On 17/05/15 21:03, Parth Dixit wrote: >> Create a chosen node for DOM0 with >> - bootargs >> - initrd > > I would have merge this patch with #22. It doesn't contain > controversial/difficult code. > >> Signed-off-by: Naresh Bhat <naresh.bhat@linaro.org> >> Signed-off-by: Parth Dixit <parth.dixit@linaro.org> >> --- >> xen/arch/arm/domain_build.c | 46 +++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 46 insertions(+) >> >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index c830702..e688a78 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -1219,6 +1219,47 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, >> return res; >> } >> #ifdef CONFIG_ACPI >> +static int make_chosen_node(struct domain *d, const struct kernel_info *kinfo) >> +{ >> + int res = 0; > > Not necessary to initialize. > >> + const char *bootargs = NULL; >> + const struct bootmodule *mod = kinfo->kernel_bootmodule; >> + void *fdt = kinfo->fdt; >> + >> + DPRINT("Create bootargs chosen node\n"); > > The name of the node is "chosen" not "bootargs chosen". > >> + >> + if ( mod && mod->cmdline[0] ) >> + bootargs = &mod->cmdline[0]; >> + res = fdt_begin_node(fdt, "chosen"); >> + if ( res ) >> + return res; >> + >> + res = fdt_property(fdt, "bootargs", bootargs, (strlen(bootargs)+1)); > > strlen(bootargs) + 1. And the ( ) around it is not necessary. > > Furthermore, you are assuming that bootargs is always present here. > Although if the module is not present, the variable will be NULL (see > your check above) and Xen will crash. > >> + if ( res ) >> + return res; >> + >> + /* >> + * If the bootloader provides an initrd, we must create a placeholder >> + * for the initrd properties. The values will be replaced later. >> + */ >> + if ( mod && mod->size ) >> + { >> + u64 a = 0; >> + res = fdt_property(kinfo->fdt, "linux,initrd-start", &a, sizeof(a)); >> + if ( res ) >> + return res; >> + >> + res = fdt_property(kinfo->fdt, "linux,initrd-end", &a, sizeof(a)); >> + if ( res ) >> + return res; >> + } >> + >> + res = fdt_end_node(fdt); >> + >> + return res; >> +} >> + >> /* >> * Prepare a minimal DTB for DOM0 which contains >> * bootargs, initrd, memory information, >> @@ -1259,6 +1300,11 @@ static int create_acpi_dtb(struct domain *d, struct kernel_info *kinfo, struct m >> if ( ret ) >> return ret; >> >> + /* Create a chosen node for DOM0 */ >> + ret = make_chosen_node(d, kinfo); >> + if ( ret ) >> + goto err; >> + >> ret = fdt_end_node(kinfo->fdt); >> if ( ret < 0 ) >> goto err; >> > > Regards, > > -- > Julien Grall
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index c830702..e688a78 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1219,6 +1219,47 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo, return res; } #ifdef CONFIG_ACPI +static int make_chosen_node(struct domain *d, const struct kernel_info *kinfo) +{ + int res = 0; + const char *bootargs = NULL; + const struct bootmodule *mod = kinfo->kernel_bootmodule; + void *fdt = kinfo->fdt; + + DPRINT("Create bootargs chosen node\n"); + + if ( mod && mod->cmdline[0] ) + bootargs = &mod->cmdline[0]; + + res = fdt_begin_node(fdt, "chosen"); + if ( res ) + return res; + + res = fdt_property(fdt, "bootargs", bootargs, (strlen(bootargs)+1)); + if ( res ) + return res; + + /* + * If the bootloader provides an initrd, we must create a placeholder + * for the initrd properties. The values will be replaced later. + */ + if ( mod && mod->size ) + { + u64 a = 0; + res = fdt_property(kinfo->fdt, "linux,initrd-start", &a, sizeof(a)); + if ( res ) + return res; + + res = fdt_property(kinfo->fdt, "linux,initrd-end", &a, sizeof(a)); + if ( res ) + return res; + } + + res = fdt_end_node(fdt); + + return res; +} + /* * Prepare a minimal DTB for DOM0 which contains * bootargs, initrd, memory information, @@ -1259,6 +1300,11 @@ static int create_acpi_dtb(struct domain *d, struct kernel_info *kinfo, struct m if ( ret ) return ret; + /* Create a chosen node for DOM0 */ + ret = make_chosen_node(d, kinfo); + if ( ret ) + goto err; + ret = fdt_end_node(kinfo->fdt); if ( ret < 0 ) goto err;