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

[PATCH v4 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.

Compilation for both mips and arm has been tested but run on qemu-malta was not
possible. Changing the MALLOC_SIZE to allow loading a kernel always lead to a 
non-bootable system. Changes have been tested on kvx architecture for which
bootm support has been added and will be submitted.

Changes v3 -> v4
 - Fix init of elf entry address to be used by bootm_load_elf

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          | 111 +++++++++++++++++++++++++++++++++++-------
 include/bootm.h       |   3 ++
 include/elf.h         |  16 +++++-
 6 files changed, 163 insertions(+), 39 deletions(-)

-- 
2.17.1


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

[PATCH v4 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 v4 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 v4 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 v4 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  | 104 ++++++++++++++++++++++++++++++++++++++++++++++++--
 include/elf.h |   8 ++++
 2 files changed, 109 insertions(+), 3 deletions(-)

diff --git a/common/elf.c b/common/elf.c
index 5534632b2..949e953b0 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;
@@ -85,8 +90,6 @@ static int load_elf_image_phdr(struct elf_image *elf)
 	void *phdr = (void *) (buf + elf_hdr_e_phoff(elf, buf));
 	int i, ret;
 
-	elf->entry = elf_hdr_e_entry(elf, buf);
-
 	for (i = 0; i < elf_hdr_e_phnum(elf, buf) ; ++i) {
 		void *src = buf + elf_phdr_p_offset(elf, phdr);
 
@@ -121,11 +124,19 @@ static int elf_check_image(struct elf_image *elf)
 
 static int elf_check_init(struct elf_image *elf, void *buf)
 {
+	int ret;
+
 	elf->buf = buf;
 	elf->low_addr = (void *) (unsigned long) -1;
 	elf->high_addr = 0;
 
-	return elf_check_image(elf);
+	ret = elf_check_image(elf);
+	if (ret)
+		return ret;
+
+	elf->entry = elf_hdr_e_entry(elf, elf->buf);
+
+	return 0;
 }
 
 struct elf_image *elf_load_image(void *buf)
@@ -158,3 +169,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 v4 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 v4 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 v4 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. Fix comment style
during this modification.

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

diff --git a/common/elf.c b/common/elf.c
index 949e953b0..3242788f0 100644
--- a/common/elf.c
+++ b/common/elf.c
@@ -94,8 +94,10 @@ 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;
 
@@ -139,37 +141,6 @@ static int elf_check_init(struct elf_image *elf, void *buf)
 	return 0;
 }
 
-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;
@@ -254,5 +225,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 v4 0/7] elf: add better bootm support

[Oleksij Rempel]

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

Hi Clement,

suddenly it is still exploding. I'll try to investigate the reason.

On Fri, May 08, 2020 at 07:04:04PM +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.
> 
> Compilation for both mips and arm has been tested but run on qemu-malta was not
> possible. Changing the MALLOC_SIZE to allow loading a kernel always lead to a 
> non-bootable system. Changes have been tested on kvx architecture for which
> bootm support has been added and will be submitted.
> 
> Changes v3 -> v4
>  - Fix init of elf entry address to be used by bootm_load_elf
> 
> 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          | 111 +++++++++++++++++++++++++++++++++++-------
>  include/bootm.h       |   3 ++
>  include/elf.h         |  16 +++++-
>  6 files changed, 163 insertions(+), 39 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 v4 0/7] elf: add better bootm support

[Oleksij Rempel]

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

On Sat, May 09, 2020 at 04:51:42PM +0200, Oleksij Rempel wrote:
> Hi Clement,
> 
> suddenly it is still exploding. I'll try to investigate the reason.

common/elf.c:
  load_elf_image_phdr()
    elf_hdr_e_phnum() <-- returns 0
    for (i = 0; i < elf_hdr_e_phnum(elf, buf) ; ++i) {
      ... so we will never get here.

Probably we should add a sanity check here and be more verbose in this
case.

here is my elf image:
https://github.com/olerem/barebox/blob/new-elf/ore-linux-dpt-module

> On Fri, May 08, 2020 at 07:04:04PM +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.
> > 
> > Compilation for both mips and arm has been tested but run on qemu-malta was not
> > possible. Changing the MALLOC_SIZE to allow loading a kernel always lead to a 
> > non-bootable system. Changes have been tested on kvx architecture for which
> > bootm support has been added and will be submitted.
> > 
> > Changes v3 -> v4
> >  - Fix init of elf entry address to be used by bootm_load_elf
> > 
> > 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          | 111 +++++++++++++++++++++++++++++++++++-------
> >  include/bootm.h       |   3 ++
> >  include/elf.h         |  16 +++++-
> >  6 files changed, 163 insertions(+), 39 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 |



-- 
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 v4 0/7] elf: add better bootm support

[Clément Leger]

Hi Oleksij,

----- On 9 May, 2020, at 18:51, Oleksij Rempel o.rempel@pengutronix.de wrote:

> On Sat, May 09, 2020 at 04:51:42PM +0200, Oleksij Rempel wrote:
>> Hi Clement,
>> 
>> suddenly it is still exploding. I'll try to investigate the reason.
> 
> common/elf.c:
>  load_elf_image_phdr()
>    elf_hdr_e_phnum() <-- returns 0
>    for (i = 0; i < elf_hdr_e_phnum(elf, buf) ; ++i) {
>      ... so we will never get here.
> 
> Probably we should add a sanity check here and be more verbose in this
> case.

There is something odd because your elf is correct and has 2 program
headers. So this should not be a problem (but a sanity check makes
sense !).

I suspect that the elf size computation is wrong with your elf file
and leads to incompletely loaded elf file but I tried to compute
it using the sequence of code used in barebox and everything seems ok.
Maybe there is something with the endianess/elfclass going wrong.
I tried again on kvx to be sure I did not made a mistake but it works
and the elf entry address is correctly used.
I'll try to add handling for different endianess and load your elf
file on kvx to debug that.

> 
> here is my elf image:
> https://github.com/olerem/barebox/blob/new-elf/ore-linux-dpt-module

I succeeded in building Distrokit images but I can't get it under barebox.
There is only 4M oof RAM and each time I tries to modify it, it won't boot
anymore.
I tried integrating the kernel (and your eore-linux-dpt-module elf) in
the environment but it also won't boot. Do you have any idea on how to
do it ?
Even if the kernel won't start under qemu , at least I'll be able to
understand what is wrong.

Thanks for testing,

Clément

> 
>> On Fri, May 08, 2020 at 07:04:04PM +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.
>> > 
>> > Compilation for both mips and arm has been tested but run on qemu-malta was not
>> > possible. Changing the MALLOC_SIZE to allow loading a kernel always lead to a
>> > non-bootable system. Changes have been tested on kvx architecture for which
>> > bootm support has been added and will be submitted.
>> > 
>> > Changes v3 -> v4
>> >  - Fix init of elf entry address to be used by bootm_load_elf
>> > 
>> > 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          | 111 +++++++++++++++++++++++++++++++++++-------
>> >  include/bootm.h       |   3 ++
>> >  include/elf.h         |  16 +++++-
>> >  6 files changed, 163 insertions(+), 39 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 |
> 
> 
> 
> --
> 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

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

[Oleksij Rempel]

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

Hi Clement,

just in case it makes a difference. I load this file over tftp (boot
net)

On Sat, May 09, 2020 at 09:24:55PM +0200, Clément Leger wrote:
> Hi Oleksij,
> 
> ----- On 9 May, 2020, at 18:51, Oleksij Rempel o.rempel@pengutronix.de wrote:
> 
> > On Sat, May 09, 2020 at 04:51:42PM +0200, Oleksij Rempel wrote:
> >> Hi Clement,
> >> 
> >> suddenly it is still exploding. I'll try to investigate the reason.
> > 
> > common/elf.c:
> >  load_elf_image_phdr()
> >    elf_hdr_e_phnum() <-- returns 0
> >    for (i = 0; i < elf_hdr_e_phnum(elf, buf) ; ++i) {
> >      ... so we will never get here.
> > 
> > Probably we should add a sanity check here and be more verbose in this
> > case.
> 
> There is something odd because your elf is correct and has 2 program
> headers. So this should not be a problem (but a sanity check makes
> sense !).
> 
> I suspect that the elf size computation is wrong with your elf file
> and leads to incompletely loaded elf file but I tried to compute
> it using the sequence of code used in barebox and everything seems ok.
> Maybe there is something with the endianess/elfclass going wrong.
> I tried again on kvx to be sure I did not made a mistake but it works
> and the elf entry address is correctly used.
> I'll try to add handling for different endianess and load your elf
> file on kvx to debug that.
> 
> > 
> > here is my elf image:
> > https://github.com/olerem/barebox/blob/new-elf/ore-linux-dpt-module
> 
> I succeeded in building Distrokit images but I can't get it under barebox.
> There is only 4M oof RAM and each time I tries to modify it, it won't boot
> anymore.
> I tried integrating the kernel (and your eore-linux-dpt-module elf) in
> the environment but it also won't boot. Do you have any idea on how to
> do it ?
> Even if the kernel won't start under qemu , at least I'll be able to
> understand what is wrong.
> 
> Thanks for testing,
> 
> Clément
> 
> > 
> >> On Fri, May 08, 2020 at 07:04:04PM +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.
> >> > 
> >> > Compilation for both mips and arm has been tested but run on qemu-malta was not
> >> > possible. Changing the MALLOC_SIZE to allow loading a kernel always lead to a
> >> > non-bootable system. Changes have been tested on kvx architecture for which
> >> > bootm support has been added and will be submitted.
> >> > 
> >> > Changes v3 -> v4
> >> >  - Fix init of elf entry address to be used by bootm_load_elf
> >> > 
> >> > 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          | 111 +++++++++++++++++++++++++++++++++++-------
> >> >  include/bootm.h       |   3 ++
> >> >  include/elf.h         |  16 +++++-
> >> >  6 files changed, 163 insertions(+), 39 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 |
> > 
> > 
> > 
> > --
> > 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

-- 
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 v4 0/7] elf: add better bootm support

[Clément Leger]

Hi Oleksij,

----- On 10 May, 2020, at 06:31, Oleksij Rempel o.rempel@pengutronix.de wrote:

> Hi Clement,
> 
> just in case it makes a difference. I load this file over tftp (boot
> net)

I manage to load your elf file on kvx using bootm (I do not have net boot)
and here is what I get:

--------------------------------------------
Board: KONIC 200 (K200)
malloc space: 0x110306050 -> 0x200000000 (size 3.7 GiB)
export: No such file or directory

Hit any to stop autoboot:    3
barebox:/ bootm env/ore-linux-dpt-module 
Entry Point:  80980000

Loading ELF 'env/ore-linux-dpt-module'
Loading phdr to 0x0000000080980000 (2740416 bytes)
...
--------------------------------------------

So one segment is loaded which is correct according to readelf.
elf file is correct and can be loaded using bootm.

Could you try loading it using bootm directly ?
This will confirm if it's related to net boot or not (although I don't
know what are the underlying differences).

Thanks,

Clément

> 
> On Sat, May 09, 2020 at 09:24:55PM +0200, Clément Leger wrote:
>> Hi Oleksij,
>> 
>> ----- On 9 May, 2020, at 18:51, Oleksij Rempel o.rempel@pengutronix.de wrote:
>> 
>> > On Sat, May 09, 2020 at 04:51:42PM +0200, Oleksij Rempel wrote:
>> >> Hi Clement,
>> >> 
>> >> suddenly it is still exploding. I'll try to investigate the reason.
>> > 
>> > common/elf.c:
>> >  load_elf_image_phdr()
>> >    elf_hdr_e_phnum() <-- returns 0
>> >    for (i = 0; i < elf_hdr_e_phnum(elf, buf) ; ++i) {
>> >      ... so we will never get here.
>> > 
>> > Probably we should add a sanity check here and be more verbose in this
>> > case.
>> 
>> There is something odd because your elf is correct and has 2 program
>> headers. So this should not be a problem (but a sanity check makes
>> sense !).
>> 
>> I suspect that the elf size computation is wrong with your elf file
>> and leads to incompletely loaded elf file but I tried to compute
>> it using the sequence of code used in barebox and everything seems ok.
>> Maybe there is something with the endianess/elfclass going wrong.
>> I tried again on kvx to be sure I did not made a mistake but it works
>> and the elf entry address is correctly used.
>> I'll try to add handling for different endianess and load your elf
>> file on kvx to debug that.
>> 
>> > 
>> > here is my elf image:
>> > https://github.com/olerem/barebox/blob/new-elf/ore-linux-dpt-module
>> 
>> I succeeded in building Distrokit images but I can't get it under barebox.
>> There is only 4M oof RAM and each time I tries to modify it, it won't boot
>> anymore.
>> I tried integrating the kernel (and your eore-linux-dpt-module elf) in
>> the environment but it also won't boot. Do you have any idea on how to
>> do it ?
>> Even if the kernel won't start under qemu , at least I'll be able to
>> understand what is wrong.
>> 
>> Thanks for testing,
>> 
>> Clément
>> 
>> > 
>> >> On Fri, May 08, 2020 at 07:04:04PM +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.
>> >> > 
>> >> > Compilation for both mips and arm has been tested but run on qemu-malta was not
>> >> > possible. Changing the MALLOC_SIZE to allow loading a kernel always lead to a
>> >> > non-bootable system. Changes have been tested on kvx architecture for which
>> >> > bootm support has been added and will be submitted.
>> >> > 
>> >> > Changes v3 -> v4
>> >> >  - Fix init of elf entry address to be used by bootm_load_elf
>> >> > 
>> >> > 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          | 111 +++++++++++++++++++++++++++++++++++-------
>> >> >  include/bootm.h       |   3 ++
>> >> >  include/elf.h         |  16 +++++-
>> >> >  6 files changed, 163 insertions(+), 39 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 |
>> > 
>> > 
>> > 
>> > --
>> > 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
> 
> --
> 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

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

[Oleksij Rempel]

[-- Attachment #1.1.1: Type: text/plain, Size: 8279 bytes --]

Hi Clement,

This changes are needed to make your patch partially work on my system:
https://github.com/olerem/barebox/commit/90ad4f3cd8cbcfd361ff780bcfce1573ae73053d

I found following issues:
- the elf->list was not initiated
- my system has only 64MiB RAM and only 8MiB is available for barebox
alloc. Since you patch is reading complete file in to alloc, i was not
able to start kernel which was already copied to alloc by:
cp /mnt/tftp/kernel .
This is still not fixed, but with some sanity checks the user is
notified about it.
- lseek(fd, 0, SEEK_SET); is not working on tftp mount point. This was
the reason why elf header actually passed check, but had a trash in
elf->buf later.

On systems with limited amount of RAM, it would be better to read the
header and request RAM regions on first stand, and then directly read
them from file in to the requested regions.

And there is one more thing, by using "dryrun" function (boot(m) -d), i
would expect to be able to see reserved regions with "iomem" command.
Currently it is not the case.

Am 10.05.20 um 13:31 schrieb Clément Leger:
> Hi Oleksij,
> 
> ----- On 10 May, 2020, at 06:31, Oleksij Rempel o.rempel@pengutronix.de wrote:
> 
>> Hi Clement,
>>
>> just in case it makes a difference. I load this file over tftp (boot
>> net)
> 
> I manage to load your elf file on kvx using bootm (I do not have net boot)
> and here is what I get:
> 
> --------------------------------------------
> Board: KONIC 200 (K200)
> malloc space: 0x110306050 -> 0x200000000 (size 3.7 GiB)
> export: No such file or directory
> 
> Hit any to stop autoboot:    3
> barebox:/ bootm env/ore-linux-dpt-module 
> Entry Point:  80980000
> 
> Loading ELF 'env/ore-linux-dpt-module'
> Loading phdr to 0x0000000080980000 (2740416 bytes)
> ...
> --------------------------------------------
> 
> So one segment is loaded which is correct according to readelf.
> elf file is correct and can be loaded using bootm.
> 
> Could you try loading it using bootm directly ?
> This will confirm if it's related to net boot or not (although I don't
> know what are the underlying differences).
> 
> Thanks,
> 
> Clément
> 
>>
>> On Sat, May 09, 2020 at 09:24:55PM +0200, Clément Leger wrote:
>>> Hi Oleksij,
>>>
>>> ----- On 9 May, 2020, at 18:51, Oleksij Rempel o.rempel@pengutronix.de wrote:
>>>
>>>> On Sat, May 09, 2020 at 04:51:42PM +0200, Oleksij Rempel wrote:
>>>>> Hi Clement,
>>>>>
>>>>> suddenly it is still exploding. I'll try to investigate the reason.
>>>>
>>>> common/elf.c:
>>>>  load_elf_image_phdr()
>>>>    elf_hdr_e_phnum() <-- returns 0
>>>>    for (i = 0; i < elf_hdr_e_phnum(elf, buf) ; ++i) {
>>>>      ... so we will never get here.
>>>>
>>>> Probably we should add a sanity check here and be more verbose in this
>>>> case.
>>>
>>> There is something odd because your elf is correct and has 2 program
>>> headers. So this should not be a problem (but a sanity check makes
>>> sense !).
>>>
>>> I suspect that the elf size computation is wrong with your elf file
>>> and leads to incompletely loaded elf file but I tried to compute
>>> it using the sequence of code used in barebox and everything seems ok.
>>> Maybe there is something with the endianess/elfclass going wrong.
>>> I tried again on kvx to be sure I did not made a mistake but it works
>>> and the elf entry address is correctly used.
>>> I'll try to add handling for different endianess and load your elf
>>> file on kvx to debug that.
>>>
>>>>
>>>> here is my elf image:
>>>> https://github.com/olerem/barebox/blob/new-elf/ore-linux-dpt-module
>>>
>>> I succeeded in building Distrokit images but I can't get it under barebox.
>>> There is only 4M oof RAM and each time I tries to modify it, it won't boot
>>> anymore.
>>> I tried integrating the kernel (and your eore-linux-dpt-module elf) in
>>> the environment but it also won't boot. Do you have any idea on how to
>>> do it ?
>>> Even if the kernel won't start under qemu , at least I'll be able to
>>> understand what is wrong.
>>>
>>> Thanks for testing,
>>>
>>> Clément
>>>
>>>>
>>>>> On Fri, May 08, 2020 at 07:04:04PM +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.
>>>>>>
>>>>>> Compilation for both mips and arm has been tested but run on qemu-malta was not
>>>>>> possible. Changing the MALLOC_SIZE to allow loading a kernel always lead to a
>>>>>> non-bootable system. Changes have been tested on kvx architecture for which
>>>>>> bootm support has been added and will be submitted.
>>>>>>
>>>>>> Changes v3 -> v4
>>>>>>  - Fix init of elf entry address to be used by bootm_load_elf
>>>>>>
>>>>>> 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          | 111 +++++++++++++++++++++++++++++++++++-------
>>>>>>  include/bootm.h       |   3 ++
>>>>>>  include/elf.h         |  16 +++++-
>>>>>>  6 files changed, 163 insertions(+), 39 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 |
>>>>
>>>>
>>>>
>>>> --
>>>> 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
>>
>> --
>> 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
> 


-- 
Regards,
Oleksij


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 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 v4 0/7] elf: add better bootm support

[Clément Leger]

Hi Oleksij,

----- On 6 Jun, 2020, at 12:11, Oleksij Rempel linux@rempel-privat.de wrote:

> Hi Clement,
> 
> This changes are needed to make your patch partially work on my system:
> https://github.com/olerem/barebox/commit/90ad4f3cd8cbcfd361ff780bcfce1573ae73053d

Thanks for investigating and sorry for the remainign errors.
Some xzalloc checks can probably be omitted or I think I should replace
the existing xzalloc by calloc to allow failing on elf loading
(which seems more sane).

> 
> I found following issues:
> - the elf->list was not initiated

My bad, I missed it while moving elf init to a specific function...
I started to wonder what I had tested side but It worked because
the list is only used in elf_release_regions (which wasn't called).

> - my system has only 64MiB RAM and only 8MiB is available for barebox
> alloc. Since you patch is reading complete file in to alloc, i was not
> able to start kernel which was already copied to alloc by:
> cp /mnt/tftp/kernel .

Indeed, strangely, mips code used to do a read_file before loading the
elf file in the mips bootm code. There is probably something different
but I can't quite get it. Anyway, it will be way better by not reading
the whole file.

> This is still not fixed, but with some sanity checks the user is
> notified about it.
> - lseek(fd, 0, SEEK_SET); is not working on tftp mount point. This was
> the reason why elf header actually passed check, but had a trash in
> elf->buf later.

Arg, and I did not checked lseek return value... I will use open/close
as you did.

> 
> On systems with limited amount of RAM, it would be better to read the
> header and request RAM regions on first stand, and then directly read
> them from file in to the requested regions.

Ok, I will modify the elf loader to do so. Apparently, tftp supports
forward lseek so it should be possible.

> 
> And there is one more thing, by using "dryrun" function (boot(m) -d), i
> would expect to be able to see reserved regions with "iomem" command.
> Currently it is not the case.

I can modify that but I guess this is the default behavior no ?
In the bootm command:

	ret = handler->bootm(data);
	if (data->dryrun)
		printf("Dryrun. Aborted\n");

err_out:
	if (data->os_res)
		release_sdram_region(data->os_res);
	if (data->initrd_res)
		release_sdram_region(data->initrd_res);

So if the dryrun argument was given, the complete err_out path is taken
and lead to releasing all regions. But I can do it if needed.

Thanks again for testing and sorry for letting such things pass in
the patches.

Clément

> 
> Am 10.05.20 um 13:31 schrieb Clément Leger:
>> Hi Oleksij,
>> 
>> ----- On 10 May, 2020, at 06:31, Oleksij Rempel o.rempel@pengutronix.de wrote:
>> 
>>> Hi Clement,
>>>
>>> just in case it makes a difference. I load this file over tftp (boot
>>> net)
>> 
>> I manage to load your elf file on kvx using bootm (I do not have net boot)
>> and here is what I get:
>> 
>> --------------------------------------------
>> Board: KONIC 200 (K200)
>> malloc space: 0x110306050 -> 0x200000000 (size 3.7 GiB)
>> export: No such file or directory
>> 
>> Hit any to stop autoboot:    3
>> barebox:/ bootm env/ore-linux-dpt-module
>> Entry Point:  80980000
>> 
>> Loading ELF 'env/ore-linux-dpt-module'
>> Loading phdr to 0x0000000080980000 (2740416 bytes)
>> ...
>> --------------------------------------------
>> 
>> So one segment is loaded which is correct according to readelf.
>> elf file is correct and can be loaded using bootm.
>> 
>> Could you try loading it using bootm directly ?
>> This will confirm if it's related to net boot or not (although I don't
>> know what are the underlying differences).
>> 
>> Thanks,
>> 
>> Clément
>> 
>>>
>>> On Sat, May 09, 2020 at 09:24:55PM +0200, Clément Leger wrote:
>>>> Hi Oleksij,
>>>>
>>>> ----- On 9 May, 2020, at 18:51, Oleksij Rempel o.rempel@pengutronix.de wrote:
>>>>
>>>>> On Sat, May 09, 2020 at 04:51:42PM +0200, Oleksij Rempel wrote:
>>>>>> Hi Clement,
>>>>>>
>>>>>> suddenly it is still exploding. I'll try to investigate the reason.
>>>>>
>>>>> common/elf.c:
>>>>>  load_elf_image_phdr()
>>>>>    elf_hdr_e_phnum() <-- returns 0
>>>>>    for (i = 0; i < elf_hdr_e_phnum(elf, buf) ; ++i) {
>>>>>      ... so we will never get here.
>>>>>
>>>>> Probably we should add a sanity check here and be more verbose in this
>>>>> case.
>>>>
>>>> There is something odd because your elf is correct and has 2 program
>>>> headers. So this should not be a problem (but a sanity check makes
>>>> sense !).
>>>>
>>>> I suspect that the elf size computation is wrong with your elf file
>>>> and leads to incompletely loaded elf file but I tried to compute
>>>> it using the sequence of code used in barebox and everything seems ok.
>>>> Maybe there is something with the endianess/elfclass going wrong.
>>>> I tried again on kvx to be sure I did not made a mistake but it works
>>>> and the elf entry address is correctly used.
>>>> I'll try to add handling for different endianess and load your elf
>>>> file on kvx to debug that.
>>>>
>>>>>
>>>>> here is my elf image:
>>>>> https://github.com/olerem/barebox/blob/new-elf/ore-linux-dpt-module
>>>>
>>>> I succeeded in building Distrokit images but I can't get it under barebox.
>>>> There is only 4M oof RAM and each time I tries to modify it, it won't boot
>>>> anymore.
>>>> I tried integrating the kernel (and your eore-linux-dpt-module elf) in
>>>> the environment but it also won't boot. Do you have any idea on how to
>>>> do it ?
>>>> Even if the kernel won't start under qemu , at least I'll be able to
>>>> understand what is wrong.
>>>>
>>>> Thanks for testing,
>>>>
>>>> Clément
>>>>
>>>>>
>>>>>> On Fri, May 08, 2020 at 07:04:04PM +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.
>>>>>>>
>>>>>>> Compilation for both mips and arm has been tested but run on qemu-malta was not
>>>>>>> possible. Changing the MALLOC_SIZE to allow loading a kernel always lead to a
>>>>>>> non-bootable system. Changes have been tested on kvx architecture for which
>>>>>>> bootm support has been added and will be submitted.
>>>>>>>
>>>>>>> Changes v3 -> v4
>>>>>>>  - Fix init of elf entry address to be used by bootm_load_elf
>>>>>>>
>>>>>>> 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          | 111 +++++++++++++++++++++++++++++++++++-------
>>>>>>>  include/bootm.h       |   3 ++
>>>>>>>  include/elf.h         |  16 +++++-
>>>>>>>  6 files changed, 163 insertions(+), 39 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 |
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> 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
>>>
>>> --
>>> 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
>> 
> 
> 
> --
> Regards,
> Oleksij

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