[PATCH v3 0/7] elf: add better bootm support

[PATCH v3 0/7] elf: add better bootm support

[Clement Leger]

Currently, when booting an elf file using "bootm /dev/mtdx", bootm will
simply pass the file to the bootm and the read done on it will read the
entire flash partition. This series starts by some cleanup and then add an
elf_open function to load the elf file size only based on the elf header.
A special handling for the elf file is also added in bootm data to allow
using directly the elf file structure. Finally the mips bootm is modified
to use bootm_load_os directly instead of manual elf loading.

Changes v2 -> v3
 - Integrate elf loading in bootm_load_os
 - Add patch to remove now unused elf_load_image/elf_release_image
 - Use malloc instead of xmalloc and check return value

Changes v1 -> v2
 - Add BOOTM_ELF config to select elf support and add checks in code
 - Add an elf_get_mem_size function to avoid computing elf size in bootm.c
 - Use xmalloc and read_full in elf_open instead of xzalloc/read
 - Fix data->elf NULL reset
 - Remove elf struct entirely from mips bootm code

Clement Leger (7):
  common: elf: add computation of elf boundaries
  common: elf: fix warning on 32 bits architectures
  common: elf: split init to be reused from other function
  common: elf: add elf_open, elf_close and elf_load
  common: bootm: add support for elf file loading
  mips: lib: bootm: use bootm elf loading capabilities
  common: elf: remove elf_load_image/elf_release_image

 arch/mips/lib/bootm.c |  31 +++++-------
 common/Kconfig        |   8 ++++
 common/bootm.c        |  33 +++++++++++++
 common/elf.c          | 107 ++++++++++++++++++++++++++++++++++++------
 include/bootm.h       |   3 ++
 include/elf.h         |  16 ++++++-
 6 files changed, 161 insertions(+), 37 deletions(-)

-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v3 1/7] common: elf: add computation of elf boundaries

[Clement Leger]

In order to correctly load an initrd or a device tree after an elf file,
we need to know its boundaries. This commit adds support for that and
allow the bootm implementations to use it for memory loading.

Signed-off-by: Clement Leger <cleger@kalray.eu>
---
 common/elf.c  | 7 +++++++
 include/elf.h | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/common/elf.c b/common/elf.c
index 4733accb0..d64de401c 100644
--- a/common/elf.c
+++ b/common/elf.c
@@ -59,6 +59,11 @@ static int load_elf_phdr_segment(struct elf_image *elf, void *src,
 	if (!p_filesz)
 		return 0;
 
+	if (dst < elf->low_addr)
+		elf->low_addr = dst;
+	if (dst + p_memsz > elf->high_addr)
+		elf->high_addr = dst + p_memsz;
+
 	pr_debug("Loading phdr to 0x%p (%llu bytes)\n", dst, p_filesz);
 
 	ret = elf_request_region(elf, (resource_size_t)dst, p_filesz);
@@ -124,6 +129,8 @@ struct elf_image *elf_load_image(void *buf)
 	INIT_LIST_HEAD(&elf->list);
 
 	elf->buf = buf;
+	elf->low_addr = (void *) (unsigned long) -1;
+	elf->high_addr = 0;
 
 	ret = elf_check_image(elf);
 	if (ret)
diff --git a/include/elf.h b/include/elf.h
index 113728f08..403412f3f 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -403,9 +403,16 @@ struct elf_image {
 	struct list_head list;
 	u8 class;
 	u64 entry;
+	void *low_addr;
+	void *high_addr;
 	void *buf;
 };
 
+static inline size_t elf_get_mem_size(struct elf_image *elf)
+{
+	return elf->high_addr - elf->low_addr;
+}
+
 struct elf_image *elf_load_image(void *buf);
 void elf_release_image(struct elf_image *elf);
 
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v3 2/7] common: elf: fix warning on 32 bits architectures

[Clement Leger]

When pointers are 32 bits wide and we cast a potentially 64 bits
value in it, the compiler will yield an error. Cast that value first
into a phys_addr_t to match the architecture pointer size and then
in a void *.

Signed-off-by: Clement Leger <cleger@kalray.eu>
---
 common/elf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/elf.c b/common/elf.c
index d64de401c..55f5bc645 100644
--- a/common/elf.c
+++ b/common/elf.c
@@ -47,7 +47,7 @@ static void elf_release_regions(struct elf_image *elf)
 static int load_elf_phdr_segment(struct elf_image *elf, void *src,
 				 void *phdr)
 {
-	void *dst = (void *) elf_phdr_p_paddr(elf, phdr);
+	void *dst = (void *) (phys_addr_t) elf_phdr_p_paddr(elf, phdr);
 	int ret;
 	u64 p_filesz = elf_phdr_p_filesz(elf, phdr);
 	u64 p_memsz = elf_phdr_p_memsz(elf, phdr);
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v3 3/7] common: elf: split init to be reused from other function

[Clement Leger]

New elf_open function will also need to initialize an elf file. Split
this to avoid missing members initialization.

Signed-off-by: Clement Leger <cleger@kalray.eu>
---
 common/elf.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/common/elf.c b/common/elf.c
index 55f5bc645..5534632b2 100644
--- a/common/elf.c
+++ b/common/elf.c
@@ -119,6 +119,15 @@ static int elf_check_image(struct elf_image *elf)
 	return 0;
 }
 
+static int elf_check_init(struct elf_image *elf, void *buf)
+{
+	elf->buf = buf;
+	elf->low_addr = (void *) (unsigned long) -1;
+	elf->high_addr = 0;
+
+	return elf_check_image(elf);
+}
+
 struct elf_image *elf_load_image(void *buf)
 {
 	struct elf_image *elf;
@@ -128,13 +137,11 @@ struct elf_image *elf_load_image(void *buf)
 
 	INIT_LIST_HEAD(&elf->list);
 
-	elf->buf = buf;
-	elf->low_addr = (void *) (unsigned long) -1;
-	elf->high_addr = 0;
-
-	ret = elf_check_image(elf);
-	if (ret)
+	ret = elf_check_init(elf, buf);
+	if (ret) {
+		free(elf);
 		return ERR_PTR(ret);
+	}
 
 	ret = load_elf_image_phdr(elf);
 	if (ret) {
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v3 4/7] common: elf: add elf_open, elf_close and elf_load

[Clement Leger]

When loading an elf file from a mtd device, this allows to parse the
header and load only the needed data according to the elf size. Without
that support, loading a elf file from a /dev/mtd would try to read the
entire partition. Finally, split elf open and segment loading to allow
loading segment separately when using bootm_load_os.

Signed-off-by: Clement Leger <cleger@kalray.eu>
---
 common/elf.c  | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/elf.h |  8 +++++
 2 files changed, 100 insertions(+)

diff --git a/common/elf.c b/common/elf.c
index 5534632b2..9edd2742f 100644
--- a/common/elf.c
+++ b/common/elf.c
@@ -5,7 +5,12 @@
 
 #include <common.h>
 #include <elf.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <libfile.h>
 #include <memory.h>
+#include <unistd.h>
+#include <linux/fs.h>
 
 struct elf_section {
 	struct list_head list;
@@ -158,3 +163,90 @@ void elf_release_image(struct elf_image *elf)
 
 	free(elf);
 }
+
+int elf_load(struct elf_image *elf)
+{
+	int ret;
+
+	ret = load_elf_image_phdr(elf);
+	if (ret)
+		elf_release_regions(elf);
+
+	return ret;
+}
+
+static u64 elf_get_size(struct elf_image *elf)
+{
+	u64 sh_size = elf_hdr_e_shentsize(elf, elf->buf) *
+		      elf_hdr_e_shnum(elf, elf->buf);
+
+	/*
+	 * The section header table is located at the end of the elf file thus
+	 * we can take the offset and add the size of this table to obtain the
+	 * file size.
+	 */
+	return elf_hdr_e_shoff(elf, elf->buf) + sh_size;
+}
+
+struct elf_image *elf_open(const char *filename)
+{
+	int fd, ret;
+	u64 size;
+	struct elf64_hdr hdr;
+	struct elf_image *elf;
+	ssize_t read_ret;
+
+	fd = open(filename, O_RDONLY);
+	if (fd < 0) {
+		printf("could not open: %s\n", errno_str());
+		return ERR_PTR(-errno);
+	}
+
+	if (read(fd, &hdr, sizeof(hdr)) < 0) {
+		printf("could not read elf header: %s\n", errno_str());
+		ret = -errno;
+		goto err_close_fd;
+	}
+
+	elf = xzalloc(sizeof(*elf));
+
+	ret = elf_check_init(elf, &hdr);
+	if (ret) {
+		ret = -errno;
+		goto err_free_elf;
+	}
+
+	size = elf_get_size(elf);
+
+	elf->buf = malloc(size);
+	if (!elf->buf) {
+		ret = -ENOMEM;
+		goto err_free_elf;
+	}
+
+	lseek(fd, 0, SEEK_SET);
+
+	read_ret = read_full(fd, elf->buf, size);
+	if (read_ret < 0) {
+		printf("could not read elf file: %s\n", errno_str());
+		ret = -errno;
+		goto err_free_buf;
+	}
+
+	return elf;
+
+err_free_buf:
+	free(elf->buf);
+err_free_elf:
+	free(elf);
+err_close_fd:
+	close(fd);
+
+	return ERR_PTR(ret);
+}
+
+void elf_close(struct elf_image *elf)
+{
+	free(elf->buf);
+	elf_release_image(elf);
+}
diff --git a/include/elf.h b/include/elf.h
index 403412f3f..b36b917e5 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -416,6 +416,10 @@ static inline size_t elf_get_mem_size(struct elf_image *elf)
 struct elf_image *elf_load_image(void *buf);
 void elf_release_image(struct elf_image *elf);
 
+struct elf_image *elf_open(const char *filename);
+void elf_close(struct elf_image *elf);
+int elf_load(struct elf_image *elf);
+
 #define ELF_GET_FIELD(__s, __field, __type) \
 static inline __type elf_##__s##_##__field(struct elf_image *elf, void *arg) { \
 	if (elf->class == ELFCLASS32) \
@@ -427,6 +431,10 @@ static inline __type elf_##__s##_##__field(struct elf_image *elf, void *arg) { \
 ELF_GET_FIELD(hdr, e_entry, u64)
 ELF_GET_FIELD(hdr, e_phnum, u16)
 ELF_GET_FIELD(hdr, e_phoff, u64)
+ELF_GET_FIELD(hdr, e_shoff, u64)
+ELF_GET_FIELD(hdr, e_shentsize, u16)
+ELF_GET_FIELD(hdr, e_machine, u16)
+ELF_GET_FIELD(hdr, e_shnum, u16)
 ELF_GET_FIELD(hdr, e_type, u16)
 ELF_GET_FIELD(phdr, p_paddr, u64)
 ELF_GET_FIELD(phdr, p_filesz, u64)
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v3 5/7] common: bootm: add support for elf file loading

[Clement Leger]

This will allows elf loader to directly have an elf file available. Thus
filetype_elf bootm handlers will be able to use this elf file directly.

Signed-off-by: Clement Leger <cleger@kalray.eu>
---
 common/Kconfig  |  8 ++++++++
 common/bootm.c  | 33 +++++++++++++++++++++++++++++++++
 include/bootm.h |  3 +++
 3 files changed, 44 insertions(+)

diff --git a/common/Kconfig b/common/Kconfig
index 400c0553c..b28791060 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -597,6 +597,14 @@ config BOOTM_AIMAGE
 	help
 	  Support using Android Images.
 
+config BOOTM_ELF
+	bool
+	depends on BOOTM
+	select ELF
+	prompt "elf loading support"
+	help
+	  Add support to load elf file with bootm.
+
 config BOOTM_FITIMAGE
 	bool
 	prompt "FIT image support"
diff --git a/common/bootm.c b/common/bootm.c
index 299985678..488adcb22 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -143,6 +143,9 @@ int bootm_load_os(struct image_data *data, unsigned long load_address)
 		return 0;
 	}
 
+	if (IS_ENABLED(CONFIG_ELF) && data->elf)
+		return elf_load(data->elf);
+
 	if (data->os_file) {
 		data->os_res = file_to_sdram(data->os_file, load_address);
 		if (!data->os_res)
@@ -470,6 +473,8 @@ int bootm_get_os_size(struct image_data *data)
 {
 	int ret;
 
+	if (data->elf)
+		return elf_get_mem_size(data->elf);
 	if (data->os)
 		return uimage_get_size(data->os, uimage_part_num(data->os_part));
 	if (data->os_fit)
@@ -517,6 +522,22 @@ static int bootm_open_os_uimage(struct image_data *data)
 	return 0;
 }
 
+static int bootm_open_elf(struct image_data *data)
+{
+	if (!IS_ENABLED(CONFIG_ELF))
+		return -ENOSYS;
+
+	data->elf = elf_open(data->os_file);
+	if (IS_ERR(data->elf))
+		return PTR_ERR(data->elf);
+
+	printf("Entry Point:  %08llx\n", data->elf->entry);
+
+	data->os_address = data->elf->entry;
+
+	return 0;
+}
+
 static void bootm_print_info(struct image_data *data)
 {
 	if (data->os_res)
@@ -651,6 +672,16 @@ int bootm_boot(struct bootm_data *bootm_data)
 		}
 	}
 
+	if (os_type == filetype_elf) {
+		ret = bootm_open_elf(data);
+		if (ret) {
+			printf("Loading ELF image failed with: %s\n",
+					strerror(-ret));
+			data->elf = NULL;
+			goto err_out;
+		}
+	}
+
 	if (bootm_data->appendroot) {
 		char *rootarg;
 
@@ -720,6 +751,8 @@ err_out:
 		uimage_close(data->initrd);
 	if (data->os)
 		uimage_close(data->os);
+	if (IS_ENABLED(CONFIG_ELF) && data->elf)
+		elf_close(data->elf);
 	if (IS_ENABLED(CONFIG_FITIMAGE) && data->os_fit)
 		fit_close(data->os_fit);
 	if (data->of_root_node && data->of_root_node != of_get_root_node())
diff --git a/include/bootm.h b/include/bootm.h
index 7782de7a4..ef5148f31 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -46,6 +46,9 @@ struct image_data {
 	/* if os is an uImage this will be provided */
 	struct uimage_handle *os;
 
+	/* if os is an elf file this will be provided */
+	struct elf_image *elf;
+
 	/* if os is a FIT image this will be provided */
 	struct fit_handle *os_fit;
 
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v3 6/7] mips: lib: bootm: use bootm elf loading capabilities

[Clement Leger]

Now that the elf file is loaded by the bootm core, there is no need for
elf pointer anymore. Thus all elf related fields can be removed and
bootm_load_os can be used.

Signed-off-by: Clement Leger <cleger@kalray.eu>
---
 arch/mips/lib/bootm.c | 31 +++++++++++--------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c
index 5bb09cc2d..48f0d83cf 100644
--- a/arch/mips/lib/bootm.c
+++ b/arch/mips/lib/bootm.c
@@ -46,43 +46,34 @@ static struct binfmt_hook binfmt_barebox_hook = {
 static int do_bootm_elf(struct image_data *data)
 {
 	void (*entry)(int, void *);
-	struct elf_image *elf;
-	void *fdt, *buf;
-	int ret = 0;
+	void *fdt;
+	int ret;
 
-	buf = read_file(data->os_file, NULL);
-	if (!buf)
-		return -EINVAL;
-
-	elf = elf_load_image(buf);
-	if (IS_ERR(elf))
-		return PTR_ERR(elf);
+	ret = bootm_load_os(data, data->os_address);
+	if (ret)
+		return ret;
 
 	fdt = bootm_get_devicetree(data);
-	if (IS_ERR(fdt)) {
-		ret = PTR_ERR(fdt);
-		goto bootm_elf_done;
-	}
+	if (IS_ERR(fdt))
+		return PTR_ERR(fdt);
 
 	pr_info("Starting application at 0x%08lx, dts 0x%08lx...\n",
-		phys_to_virt(elf->entry), data->of_root_node);
+		phys_to_virt(data->os_address), data->of_root_node);
 
 	if (data->dryrun)
-		goto bootm_elf_done;
+		goto bootm_free_fdt;
 
 	shutdown_barebox();
 
-	entry = (void *) (unsigned long) elf->entry;
+	entry = (void *) (unsigned long) data->os_address;
 
 	entry(-2, phys_to_virt((unsigned long)fdt));
 
 	pr_err("ELF application terminated\n");
 	ret = -EINVAL;
 
-bootm_elf_done:
-	elf_release_image(elf);
+bootm_free_fdt:
 	free(fdt);
-	free(buf);
 
 	return ret;
 }
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v3 7/7] common: elf: remove elf_load_image/elf_release_image

[Clement Leger]

Since elf loading has been integrated in bootm using elf_open/elf_close,
remove these two functions which are now unused.

Signed-off-by: Clement Leger <cleger@kalray.eu>
---
 common/elf.c  | 39 +++++----------------------------------
 include/elf.h |  3 ---
 2 files changed, 5 insertions(+), 37 deletions(-)

diff --git a/common/elf.c b/common/elf.c
index 9edd2742f..e69f6928b 100644
--- a/common/elf.c
+++ b/common/elf.c
@@ -96,8 +96,8 @@ static int load_elf_image_phdr(struct elf_image *elf)
 		void *src = buf + elf_phdr_p_offset(elf, phdr);
 
 		ret = load_elf_phdr_segment(elf, src, phdr);
-		/* in case of error elf_load_image() caller should clean up and
-		 * call elf_release_image() */
+		/* in case of error elf_load() caller should clean up and
+		 * call elf_release_regions() */
 		if (ret)
 			return ret;
 
@@ -133,37 +133,6 @@ static int elf_check_init(struct elf_image *elf, void *buf)
 	return elf_check_image(elf);
 }
 
-struct elf_image *elf_load_image(void *buf)
-{
-	struct elf_image *elf;
-	int ret;
-
-	elf = xzalloc(sizeof(*elf));
-
-	INIT_LIST_HEAD(&elf->list);
-
-	ret = elf_check_init(elf, buf);
-	if (ret) {
-		free(elf);
-		return ERR_PTR(ret);
-	}
-
-	ret = load_elf_image_phdr(elf);
-	if (ret) {
-		elf_release_image(elf);
-		return ERR_PTR(ret);
-	}
-
-	return elf;
-}
-
-void elf_release_image(struct elf_image *elf)
-{
-	elf_release_regions(elf);
-
-	free(elf);
-}
-
 int elf_load(struct elf_image *elf)
 {
 	int ret;
@@ -248,5 +217,7 @@ err_close_fd:
 void elf_close(struct elf_image *elf)
 {
 	free(elf->buf);
-	elf_release_image(elf);
+	elf_release_regions(elf);
+
+	free(elf);
 }
diff --git a/include/elf.h b/include/elf.h
index b36b917e5..75ee0c975 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -413,9 +413,6 @@ static inline size_t elf_get_mem_size(struct elf_image *elf)
 	return elf->high_addr - elf->low_addr;
 }
 
-struct elf_image *elf_load_image(void *buf);
-void elf_release_image(struct elf_image *elf);
-
 struct elf_image *elf_open(const char *filename);
 void elf_close(struct elf_image *elf);
 int elf_load(struct elf_image *elf);
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3 0/7] elf: add better bootm support

[Oleksij Rempel]

[-- Attachment #1.1: Type: text/plain, Size: 5367 bytes --]

Hi Clement,

i tested you patches on my MIPS board. I get following ooops:
===============================================================================
barebox@DPTechnics DPT-Module:/ iomem
0x00000000 - 0xffffffff (size 0x00000000) iomem
  0x18020000 - 0x18020013 (size 0x00000014) 18020000.uart@18020000.of
  0x18040000 - 0x18040033 (size 0x00000034) 18040000.gpio@18040000.of
  0x18050000 - 0x180500ff (size 0x00000100) 18050000.pll-controller@18050000.of
  0x18060008 - 0x1806000f (size 0x00000008) 18060008.wdt@18060008.of
  0x18070000 - 0x180700ff (size 0x00000100) 19000000.ethernet@19000000.of
  0x19000000 - 0x190001ff (size 0x00000200) 19000000.ethernet@19000000.of
  0x1f000000 - 0x1f00000f (size 0x00000010) 1f000000.spi@1f000000.of
  0x80000000 - 0x83ffffff (size 0x04000000) kseg0_ram0
    0x80000000 - 0x80007fff (size 0x00008000) vector
    0x83788000 - 0x8378ffff (size 0x00008000) stack
    0x83790000 - 0x83f8ffff (size 0x00800000) malloc space
    0x83f90000 - 0x83fe66df (size 0x000566e0) barebox
    0x83fe66e0 - 0x83feb0bf (size 0x000049e0) barebox data
    0x83ff30c0 - 0x83ff5fe7 (size 0x00002f28) bss
barebox@DPTechnics DPT-Module:/ boot net
Booting entry 'net'
eth0: DHCP client bound to address 192.168.26.229
Entry Point:  00000000

Loading ELF '/mnt/tftp/ore-linux-dpt-module'
Loading devicetree from '/mnt/tftp/ore-oftree-dpt-module'
Starting application at 0x80000000, dts 0x837c325c...

Ooops, TLB miss on load or ifetch!

$ 0   : 00000000 00000020 00000000 80010000
$ 4   : fffffffe 83a80000 00010000 00008000
$ 8   : 83790008 8379b440 8379b440 8379b5ec
$12   : 83790000 00010000 00000060 00000011
$16   : 837c3728 ffffffea 83a80000 80000000
$20   : 00000000 00000000 00000000 00000000
$24   : 00000000 00000000
$28   : 83e6c000 8378fc30 83fdfa20 83fd55cc
Hi    : 00063c6c
Lo    : 60000000
epc   : 00000000
ra    : 83fd55cc
Status: 10000002
Cause : 40008008
Config: 80208483

### ERROR ### Please RESET the board ###
===============================================================================

The same test without patches looks like this: 
rebox@DPTechnics DPT-Module:/ boot net
Booting entry 'net'
eth0: DHCP client bound to address 192.168.26.229

Loading ELF '/mnt/tftp/ore-linux-dpt-module'
Loading devicetree from '/mnt/tftp/ore-oftree-dpt-module'
Starting application at 0x80980000, dts 0x837c2e54...
[    0.000000] Linux version 5.7.0-rc1-rt7-00316-g9b716caac68c9 (ptxdist@ptxdist) (gcc version 9.2.1 20191130 (OSELAS.Toolchain-2019.09.1 9-20191130), GNU ld (GNU Binutils) 2.32) #1 PREEMPT 2020-04-23T06:16:46+00:00
[    0.000000] printk: bootconsole [early0] enabled
[

The main difference is:
Starting application at 0x80980000, dts 0x837c2e54... <<- works
Starting application at 0x80000000, dts 0x837c325c... <<- oops

Even with applied patch to protect exception vectore code, we still
overwriting it:
    0x80000000 - 0x80007fff (size 0x00008000) vector

Any ideas?

On Tue, Apr 28, 2020 at 09:50:53AM +0200, Clement Leger wrote:
> Currently, when booting an elf file using "bootm /dev/mtdx", bootm will
> simply pass the file to the bootm and the read done on it will read the
> entire flash partition. This series starts by some cleanup and then add an
> elf_open function to load the elf file size only based on the elf header.
> A special handling for the elf file is also added in bootm data to allow
> using directly the elf file structure. Finally the mips bootm is modified
> to use bootm_load_os directly instead of manual elf loading.
> 
> Changes v2 -> v3
>  - Integrate elf loading in bootm_load_os
>  - Add patch to remove now unused elf_load_image/elf_release_image
>  - Use malloc instead of xmalloc and check return value
> 
> Changes v1 -> v2
>  - Add BOOTM_ELF config to select elf support and add checks in code
>  - Add an elf_get_mem_size function to avoid computing elf size in bootm.c
>  - Use xmalloc and read_full in elf_open instead of xzalloc/read
>  - Fix data->elf NULL reset
>  - Remove elf struct entirely from mips bootm code
> 
> Clement Leger (7):
>   common: elf: add computation of elf boundaries
>   common: elf: fix warning on 32 bits architectures
>   common: elf: split init to be reused from other function
>   common: elf: add elf_open, elf_close and elf_load
>   common: bootm: add support for elf file loading
>   mips: lib: bootm: use bootm elf loading capabilities
>   common: elf: remove elf_load_image/elf_release_image
> 
>  arch/mips/lib/bootm.c |  31 +++++-------
>  common/Kconfig        |   8 ++++
>  common/bootm.c        |  33 +++++++++++++
>  common/elf.c          | 107 ++++++++++++++++++++++++++++++++++++------
>  include/bootm.h       |   3 ++
>  include/elf.h         |  16 ++++++-
>  6 files changed, 161 insertions(+), 37 deletions(-)
> 
> -- 
> 2.17.1
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v3 0/7] elf: add better bootm support

[Clément Leger]

Hi Oleksij,

----- On 29 Apr, 2020, at 10:34, Oleksij Rempel o.rempel@pengutronix.de wrote:

> Hi Clement,
> 
> i tested you patches on my MIPS board. I get following ooops:
> ===============================================================================
> barebox@DPTechnics DPT-Module:/ iomem
> 0x00000000 - 0xffffffff (size 0x00000000) iomem
>  0x18020000 - 0x18020013 (size 0x00000014) 18020000.uart@18020000.of
>  0x18040000 - 0x18040033 (size 0x00000034) 18040000.gpio@18040000.of
>  0x18050000 - 0x180500ff (size 0x00000100) 18050000.pll-controller@18050000.of
>  0x18060008 - 0x1806000f (size 0x00000008) 18060008.wdt@18060008.of
>  0x18070000 - 0x180700ff (size 0x00000100) 19000000.ethernet@19000000.of
>  0x19000000 - 0x190001ff (size 0x00000200) 19000000.ethernet@19000000.of
>  0x1f000000 - 0x1f00000f (size 0x00000010) 1f000000.spi@1f000000.of
>  0x80000000 - 0x83ffffff (size 0x04000000) kseg0_ram0
>    0x80000000 - 0x80007fff (size 0x00008000) vector
>    0x83788000 - 0x8378ffff (size 0x00008000) stack
>    0x83790000 - 0x83f8ffff (size 0x00800000) malloc space
>    0x83f90000 - 0x83fe66df (size 0x000566e0) barebox
>    0x83fe66e0 - 0x83feb0bf (size 0x000049e0) barebox data
>    0x83ff30c0 - 0x83ff5fe7 (size 0x00002f28) bss
> barebox@DPTechnics DPT-Module:/ boot net
> Booting entry 'net'
> eth0: DHCP client bound to address 192.168.26.229
> Entry Point:  00000000
> 
> Loading ELF '/mnt/tftp/ore-linux-dpt-module'
> Loading devicetree from '/mnt/tftp/ore-oftree-dpt-module'
> Starting application at 0x80000000, dts 0x837c325c...
> 
> Ooops, TLB miss on load or ifetch!
> 
> $ 0   : 00000000 00000020 00000000 80010000
> $ 4   : fffffffe 83a80000 00010000 00008000
> $ 8   : 83790008 8379b440 8379b440 8379b5ec
> $12   : 83790000 00010000 00000060 00000011
> $16   : 837c3728 ffffffea 83a80000 80000000
> $20   : 00000000 00000000 00000000 00000000
> $24   : 00000000 00000000
> $28   : 83e6c000 8378fc30 83fdfa20 83fd55cc
> Hi    : 00063c6c
> Lo    : 60000000
> epc   : 00000000
> ra    : 83fd55cc
> Status: 10000002
> Cause : 40008008
> Config: 80208483
> 
> ### ERROR ### Please RESET the board ###
> ===============================================================================
> 
> The same test without patches looks like this:
> rebox@DPTechnics DPT-Module:/ boot net
> Booting entry 'net'
> eth0: DHCP client bound to address 192.168.26.229
> 
> Loading ELF '/mnt/tftp/ore-linux-dpt-module'
> Loading devicetree from '/mnt/tftp/ore-oftree-dpt-module'
> Starting application at 0x80980000, dts 0x837c2e54...
> [    0.000000] Linux version 5.7.0-rc1-rt7-00316-g9b716caac68c9
> (ptxdist@ptxdist) (gcc version 9.2.1 20191130 (OSELAS.Toolchain-2019.09.1
> 9-20191130), GNU ld (GNU Binutils) 2.32) #1 PREEMPT 2020-04-23T06:16:46+00:00
> [    0.000000] printk: bootconsole [early0] enabled
> [
> 
> The main difference is:
> Starting application at 0x80980000, dts 0x837c2e54... <<- works
> Starting application at 0x80000000, dts 0x837c325c... <<- oops

Ok I got it, the error come from the fact that elf->entry is initialized
in load_elf_image_phdr but used before that. I will send a V4 to fix this.
It worked on our elf loader since the code is loaded at 0x0 and the entry
point is at 0 :/.
I will fix that by initializing elf->entry in elf_open instead of elf_load.

BTW, if you have an elf that I can try, since I already built a mips 
barbeox, I will be able to test it on qemu.

Thanks for testing,

Clément

> 
> Even with applied patch to protect exception vectore code, we still
> overwriting it:
>    0x80000000 - 0x80007fff (size 0x00008000) vector
> 
> Any ideas?
> 
> On Tue, Apr 28, 2020 at 09:50:53AM +0200, Clement Leger wrote:
>> Currently, when booting an elf file using "bootm /dev/mtdx", bootm will
>> simply pass the file to the bootm and the read done on it will read the
>> entire flash partition. This series starts by some cleanup and then add an
>> elf_open function to load the elf file size only based on the elf header.
>> A special handling for the elf file is also added in bootm data to allow
>> using directly the elf file structure. Finally the mips bootm is modified
>> to use bootm_load_os directly instead of manual elf loading.
>> 
>> Changes v2 -> v3
>>  - Integrate elf loading in bootm_load_os
>>  - Add patch to remove now unused elf_load_image/elf_release_image
>>  - Use malloc instead of xmalloc and check return value
>> 
>> Changes v1 -> v2
>>  - Add BOOTM_ELF config to select elf support and add checks in code
>>  - Add an elf_get_mem_size function to avoid computing elf size in bootm.c
>>  - Use xmalloc and read_full in elf_open instead of xzalloc/read
>>  - Fix data->elf NULL reset
>>  - Remove elf struct entirely from mips bootm code
>> 
>> Clement Leger (7):
>>   common: elf: add computation of elf boundaries
>>   common: elf: fix warning on 32 bits architectures
>>   common: elf: split init to be reused from other function
>>   common: elf: add elf_open, elf_close and elf_load
>>   common: bootm: add support for elf file loading
>>   mips: lib: bootm: use bootm elf loading capabilities
>>   common: elf: remove elf_load_image/elf_release_image
>> 
>>  arch/mips/lib/bootm.c |  31 +++++-------
>>  common/Kconfig        |   8 ++++
>>  common/bootm.c        |  33 +++++++++++++
>>  common/elf.c          | 107 ++++++++++++++++++++++++++++++++++++------
>>  include/bootm.h       |   3 ++
>>  include/elf.h         |  16 ++++++-
>>  6 files changed, 161 insertions(+), 37 deletions(-)
>> 
>> --
>> 2.17.1
>> 
>> 
>> _______________________________________________
>> barebox mailing list
>> barebox@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/barebox
>> 
> 
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox