On Wed, 27 Nov 2013 15:31:51 -0800, Roy Franz <roy.franz@xxxxxxxxxx> wrote:
> Both ARM and ARM64 stubs will update the device tree
> that they pass to the kernel. In both cases they
> primarily need to add the same UEFI related information,
> so the function can be shared.
> Create a new FDT related file for this to avoid use
> of architecture #ifdefs in efi-stub-helper.c
> Change EFI allocation type for memory map to
> EFI_RUNTIME_SERVICES_DATA, since we are passing
> this buffer to the kernel, or immediately freeing it.
>
> Signed-off-by: Roy Franz <roy.franz@xxxxxxxxxx>
Hi Roy,
Comments below...
> +static efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt,
> + void *fdt, int new_fdt_size, char *cmdline_ptr,
> + u64 initrd_addr, u64 initrd_size,
> + efi_memory_desc_t *memory_map,
> + unsigned long map_size, unsigned long desc_size,
> + u32 desc_ver)
> +{
> + int node;
> + int status;
> + u32 fdt_val32;
> + u64 fdt_val64;
> + /* Copy definition of linux_banner here. Since this code is
> + * built as part of the decompressor for ARM v7, pulling
> + * in version.c where linux_banner is defined for the
> + * kernel brings other kernel dependencies with it.
> + */
> + const char linux_banner[] =
> + "Linux version " UTS_RELEASE " (" LINUX_COMPILE_BY "@"
> + LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION "\n";
This is inherently fragile because the string is now defined in two
places. This issue shouldn't hold off merging the patch, but a follow up
patch should be crafted to make it possible to pull in version.c without
the other dependencies....
Then again, the version.c strings have a very big "DON'T TOUCH" warning
on them so I'm probably worrying about nothing. :-)
> +
> + status = fdt_open_into(orig_fdt, fdt, new_fdt_size);
> + if (status != 0)
> + goto fdt_set_fail;
> +
> + node = fdt_subnode_offset(fdt, 0, "chosen");
> + if (node < 0) {
> + node = fdt_add_subnode(fdt, 0, "chosen");
> + if (node < 0) {
> + status = node; /* node is error code when negative */
> + goto fdt_set_fail;
> + }
> + }
> +
> + if ((cmdline_ptr != NULL) && (strlen(cmdline_ptr) > 0)) {
> + status = fdt_setprop(fdt, node, "bootargs", cmdline_ptr,
> + strlen(cmdline_ptr) + 1);
> + if (status)
> + goto fdt_set_fail;
> + }
> +
> + /* Set intird address/end in device tree, if present */
sp. initrd
None of the above comments are blockers though.
Acked-by: Grant Likely <grant.likely@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
