Hi Andreas,
On 7 February 2015 at 14:19, Andreas Bießmann andreas.devel@googlemail.com wrote:
Commit a93648d197df48fa46dd55f925ff70468bd81c71 introduced linker generated lists for imagetool which is part of mkimage. It is a nice feature to remove the annoying register function calls, but is not portable. Unfortunately some host compilers do not support this type of linker scripts. Therefore this commit broke this host-tool for theem, namely FreeBSD and Darwin (OS/X).
This commit tries to fix this. We won't go back to the register functions but we also can not use the linker script. So use another approach copied from linux kernel scripts/mod/file2alias.c.
Signed-off-by: Andreas Bießmann andreas.devel@googlemail.com Cc: Guilherme Maciel Ferreira guilherme.maciel.ferreira@gmail.com
tools/Makefile | 2 -- tools/imagetool.c | 34 +++++++++++++++---------------- tools/imagetool.h | 56 +++++++++++++++++++++++++++++++++++++++++---------- tools/imagetool.lds | 24 ---------------------- 4 files changed, 61 insertions(+), 55 deletions(-) delete mode 100644 tools/imagetool.lds
I have a FreeBSD set up now thanks to Jeroen so have had a play with this.
diff --git a/tools/Makefile b/tools/Makefile index 6e1ce79..ea76a3e 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -124,8 +124,6 @@ HOSTLOADLIBES_dumpimage := $(HOSTLOADLIBES_mkimage) HOSTLOADLIBES_fit_info := $(HOSTLOADLIBES_mkimage) HOSTLOADLIBES_fit_check_sign := $(HOSTLOADLIBES_mkimage)
-HOSTLDFLAGS += -T $(srctree)/tools/imagetool.lds
hostprogs-$(CONFIG_EXYNOS5250) += mkexynosspl hostprogs-$(CONFIG_EXYNOS5420) += mkexynosspl HOSTCFLAGS_mkexynosspl.o := -pedantic diff --git a/tools/imagetool.c b/tools/imagetool.c index 148e466..8563032 100644 --- a/tools/imagetool.c +++ b/tools/imagetool.c @@ -12,16 +12,15 @@
struct image_type_params *imagetool_get_type(int type) {
struct image_type_params *curr;struct image_type_params *start = ll_entry_start(struct image_type_params, image_type);struct image_type_params *end = ll_entry_end(struct image_type_params, image_type);
struct image_type_params **curr;INIT_SECTION(image_type);struct image_type_params **start = __start_image_type;struct image_type_params **end = __stop_image_type; for (curr = start; curr != end; curr++) {
if (curr->check_image_type) {if (!curr->check_image_type(type))return curr;
if ((*curr)->check_image_type) {if (!(*curr)->check_image_type(type))return *curr; } } return NULL;@@ -34,16 +33,15 @@ int imagetool_verify_print_header( struct image_tool_params *params) { int retval = -1;
struct image_type_params *curr;
struct image_type_params **curr;INIT_SECTION(image_type);
struct image_type_params *start = ll_entry_start(struct image_type_params, image_type);struct image_type_params *end = ll_entry_end(struct image_type_params, image_type);
struct image_type_params **start = __start_image_type;struct image_type_params **end = __stop_image_type; for (curr = start; curr != end; curr++) {
if (curr->verify_header) {retval = curr->verify_header((unsigned char *)ptr,
if ((*curr)->verify_header) {retval = (*curr)->verify_header((unsigned char *)ptr, sbuf->st_size, params); if (retval == 0) {@@ -51,12 +49,12 @@ int imagetool_verify_print_header( * Print the image information if verify is * successful */
if (curr->print_header) {curr->print_header(ptr);
if ((*curr)->print_header) {(*curr)->print_header(ptr); } else { fprintf(stderr, "%s: print_header undefined for %s\n",
params->cmdname, curr->name);
params->cmdname, (*curr)->name); } break; }diff --git a/tools/imagetool.h b/tools/imagetool.h index f35dec7..3e15b4e 100644 --- a/tools/imagetool.h +++ b/tools/imagetool.h @@ -20,15 +20,6 @@ #include <unistd.h> #include <u-boot/sha1.h>
-/* define __KERNEL__ in order to get the definitions
- required by the linker list. This is probably not
- the best way to do this */
-#ifndef __KERNEL__ -#define __KERNEL__ -#include <linker_lists.h> -#undef __KERNEL__ -#endif /* __KERNEL__ */
#include "fdt_host.h"
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) @@ -194,6 +185,46 @@ int imagetool_save_subimage(
void pbl_load_uboot(int fd, struct image_tool_params *mparams);
+#define ___cat(a, b) a ## b +#define __cat(a, b) ___cat(a, b)
+/* we need some special handling for this host tool running eventually on
- Darwin. The Mach-O section handling is a bit different than ELF section
- handling. The differnces in detail are:
- a) we have segments which have sections
- b) we need a API call to get the respective section symbols */
+#if defined(__MACH__) +#include <mach-o/getsect.h>
+#define INIT_SECTION(name) do { \
unsigned long name ## _len; \char *__cat(pstart_, name) = getsectdata("__TEXT", \#name, &__cat(name, _len)); \char *__cat(pstop_, name) = __cat(pstart_, name) + \__cat(name, _len); \__cat(__start_, name) = (void *)__cat(pstart_, name); \__cat(__stop_, name) = (void *)__cat(pstop_, name); \} while (0)+#define SECTION(name) __attribute__((section("__TEXT, " #name)))
If I understand this correctly, in both cases we put things in a separate section so that all the pieces get collected together. The only difference seems to be the naming of the section - for MACH it has a __TEXT prefix. Is that right? If so, I wonder if this should go in linker_list.h?
For access to the data, here we are using a list of pointers to the struct rather than a list of struct. Why is that? I can't see that this is required by the MACH system.
If that is correct, then it should be easy to support linker_list on MACH, by:
- Adding a __TEXT prefix to all the section() bits in linker_lists.h - Changing any access to the lists to use INIT_SECTION instead of going there directly
Then we should be able to support running sandbox.
Does that sound feasible?
+struct image_type_params **__start_image_type, **__stop_image_type; +#else +#define INIT_SECTION(name) /* no-op for ELF */ +#define SECTION(name) __attribute__((section(#name)))
+/* We construct a table of pointers in an ELF section (pointers generally
- go unpadded by gcc). ld creates boundary syms for us. */
+extern struct image_type_params *__start_image_type[], *__stop_image_type[]; +#endif /* __MACH__ */
+#if !defined(__used) +# if __GNUC__ == 3 && __GNUC_MINOR__ < 3 +# define __used __attribute__((__unused__)) +# else +# define __used __attribute__((__used__)) +# endif +#endif
#define U_BOOT_IMAGE_TYPE( \ _id, \ _name, \ @@ -208,7 +239,8 @@ void pbl_load_uboot(int fd, struct image_tool_params *mparams); _fflag_handle, \ _vrec_header \ ) \
ll_entry_declare(struct image_type_params, _id, image_type) = { \
static struct image_type_params __cat(image_type_, _id) = \{ \ .name = _name, \ .header_size = _header_size, \ .hdr = _header, \@@ -220,6 +252,8 @@ void pbl_load_uboot(int fd, struct image_tool_params *mparams); .check_image_type = _check_image_type, \ .fflag_handle = _fflag_handle, \ .vrec_header = _vrec_header \
}
}; \static struct image_type_params *SECTION(image_type) __used \__cat(image_type_ptr_, _id) = &__cat(image_type_, _id)#endif /* _IMAGETOOL_H_ */ diff --git a/tools/imagetool.lds b/tools/imagetool.lds deleted file mode 100644 index 7e92b4a..0000000 --- a/tools/imagetool.lds +++ /dev/null @@ -1,24 +0,0 @@ -/*
- Copyright (c) 2011-2012 The Chromium OS Authors.
- Use of this source code is governed by a BSD-style license that can be
- found in the LICENSE file.
- SPDX-License-Identifier: GPL-2.0+
- */
-SECTIONS -{
. = ALIGN(4);.u_boot_list : {KEEP(*(SORT(.u_boot_list*)));}__u_boot_sandbox_option_start = .;_u_boot_sandbox_getopt : { *(.u_boot_sandbox_getopt) }__u_boot_sandbox_option_end = .;__bss_start = .;-}
-INSERT BEFORE .data;
1.7.10.4
Regards, Simon