| Message ID | 20240605184334.13973-1-Maxim.Moskalets@kaspersky.com |
|---|---|
| State | Accepted |
| Commit | 46b6a3e6c71de8622634d5aa8dca6a216148981b |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [v5] cmd: move ELF load and boot to lib/elf.c | expand |
On 6/5/24 20:43, Maxim Moskalets wrote: > From: Maxim Moskalets <maximmosk4@gmail.com> > > Loading and running the ELF image is the responsibility of the > library and should not be associated with the command line interface. > > It is also required to run ELF images from FIT with the bootm command > so as not to depend on the command line interface. > > Signed-off-by: Maxim Moskalets <maximmosk4@gmail.com> In future versions please, include a change log separated from the commit message by ---, e.g. --- v6: foo changed to bar v5: xxx removed --- Best regards Heinrich > --- > cmd/elf.c | 58 +++++++++++++++++++-------------------------------- > include/elf.h | 10 +++++++++ > lib/elf.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 86 insertions(+), 36 deletions(-) > > diff --git a/cmd/elf.c b/cmd/elf.c > index a02361f9f5..32b7462f92 100644 > --- a/cmd/elf.c > +++ b/cmd/elf.c > @@ -19,21 +19,6 @@ > #include <linux/linkage.h> > #endif > > -/* Allow ports to override the default behavior */ > -static unsigned long do_bootelf_exec(ulong (*entry)(int, char * const[]), > - int argc, char *const argv[]) > -{ > - unsigned long ret; > - > - /* > - * pass address parameter as argv[0] (aka command name), > - * and all remaining args > - */ > - ret = entry(argc, argv); > - > - return ret; > -} > - > /* Interpreter command to boot an arbitrary ELF image from memory */ > int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > { > @@ -43,8 +28,8 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > #endif > unsigned long addr; /* Address of the ELF image */ > unsigned long rc; /* Return value from user code */ > - char *sload = NULL; > - int rcode = 0; > + int rcode = CMD_RET_SUCCESS; > + Bootelf_flags flags = {0}; > > /* Consume 'bootelf' */ > argc--; argv++; > @@ -52,7 +37,10 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > /* Check for [-p|-s] flag. */ > if (argc >= 1 && (argv[0][0] == '-' && \ > (argv[0][1] == 'p' || argv[0][1] == 's'))) { > - sload = argv[0]; > + if (argv[0][1] == 'p') > + flags.phdr = 1; > + log_debug("Using ELF header format %s\n", > + flags.phdr ? "phdr" : "shdr"); > /* Consume flag. */ > argc--; argv++; > } > @@ -75,17 +63,9 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > } else > addr = image_load_addr; > > - if (!valid_elf_image(addr)) > - return 1; > - > - if (sload && sload[1] == 'p') > - addr = load_elf_image_phdr(addr); > - else > - addr = load_elf_image_shdr(addr); > - > #if CONFIG_IS_ENABLED(CMD_ELF_FDT_SETUP) > if (fdt_addr) { > - printf("## Setting up FDT at 0x%08lx ...\n", fdt_addr); > + log_debug("Setting up FDT at 0x%08lx ...\n", fdt_addr); > flush(); > > if (image_setup_libfdt(&img, (void *)fdt_addr, NULL)) > @@ -93,21 +73,27 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > } > #endif > > - if (!env_get_autostart()) > - return rcode; > - > - printf("## Starting application at 0x%08lx ...\n", addr); > - flush(); > + if (env_get_autostart()) { > + flags.autostart = 1; > + log_debug("Starting application at 0x%08lx ...\n", addr); > + flush(); > + } > > /* > * pass address parameter as argv[0] (aka command name), > - * and all remaining args > + * and all remaining arguments > */ > - rc = do_bootelf_exec((void *)addr, argc, argv); > + rc = bootelf(addr, flags, argc, argv); > if (rc != 0) > - rcode = 1; > + rcode = CMD_RET_FAILURE; > > - printf("## Application terminated, rc = 0x%lx\n", rc); > + if (flags.autostart) > + { > + if (ENOEXEC == errno) > + log_err("Invalid ELF image\n"); > + else > + log_debug("## Application terminated, rc = 0x%lx\n", rc); > + } > > return rcode; > } > diff --git a/include/elf.h b/include/elf.h > index a4ba74d8ab..b88e6cf403 100644 > --- a/include/elf.h > +++ b/include/elf.h > @@ -12,6 +12,12 @@ > #ifndef __ASSEMBLY__ > #include "compiler.h" > > +/* Flag param bits for bootelf() function */ > +typedef struct { > + unsigned phdr : 1; /* load via program (not section) headers */ > + unsigned autostart : 1; /* Start ELF after loading */ > +} Bootelf_flags; > + > /* This version doesn't work for 64-bit ABIs - Erik */ > > /* These typedefs need to be handled better */ > @@ -700,6 +706,10 @@ unsigned long elf_hash(const unsigned char *name); > #define R_RISCV_RELATIVE 3 > > #ifndef __ASSEMBLY__ > +unsigned long bootelf_exec(ulong (*entry)(int, char * const[]), > + int argc, char *const argv[]); > +unsigned long bootelf(unsigned long addr, Bootelf_flags flags, > + int argc, char *const argv[]); > int valid_elf_image(unsigned long addr); > unsigned long load_elf64_image_phdr(unsigned long addr); > unsigned long load_elf64_image_shdr(unsigned long addr); > diff --git a/lib/elf.c b/lib/elf.c > index 9a794f9cba..dc13935e10 100644 > --- a/lib/elf.c > +++ b/lib/elf.c > @@ -7,6 +7,7 @@ > #include <cpu_func.h> > #include <elf.h> > #include <env.h> > +#include <errno.h> > #include <net.h> > #include <vxworks.h> > #ifdef CONFIG_X86 > @@ -15,6 +16,59 @@ > #include <linux/linkage.h> > #endif > > +/** > + * bootelf_exec() - start the ELF image execution. > + * > + * @entry: address of entry point of ELF. > + * > + * May by used to allow ports to override the default behavior. > + */ > +unsigned long bootelf_exec(ulong (*entry)(int, char * const[]), > + int argc, char *const argv[]) > +{ > + return entry(argc, argv); > +} > + > +/** > + * bootelf() - Boot ELF from memory. > + * > + * @addr: Loading address of ELF in memory. > + * @flags: Bits like ELF_PHDR to control boot details. > + * @argc: May be used to pass command line arguments (maybe unused). > + * Necessary for backward compatibility with the CLI command. > + * If unused, must be 0. > + * @argv: see @argc. If unused, must be NULL. > + * Return: Number returned by ELF application. > + * > + * Sets errno = ENOEXEC if the ELF image is not valid. > + */ > +unsigned long bootelf(unsigned long addr, Bootelf_flags flags, > + int argc, char *const argv[]) > +{ > + unsigned long entry_addr; > + char *args[] = {"", NULL}; > + > + errno = 0; > + > + if (!valid_elf_image(addr)) { > + errno = ENOEXEC; > + return 1; > + } > + > + entry_addr = flags.phdr ? load_elf_image_phdr(addr) > + : load_elf_image_shdr(addr); > + > + if (!flags.autostart) > + return 0; > + > + if (!argc && !argv) { > + argc = 1; > + argv = args; > + } > + > + return bootelf_exec((void *)entry_addr, argc, argv); > +} > + > /* > * A very simple ELF64 loader, assumes the image is valid, returns the > * entry point address.
On Thu, Jun 06, 2024 at 09:02:43AM +0200, Heinrich Schuchardt wrote: > On 6/5/24 20:43, Maxim Moskalets wrote: > > From: Maxim Moskalets <maximmosk4@gmail.com> > > > > Loading and running the ELF image is the responsibility of the > > library and should not be associated with the command line interface. > > > > It is also required to run ELF images from FIT with the bootm command > > so as not to depend on the command line interface. > > > > Signed-off-by: Maxim Moskalets <maximmosk4@gmail.com> > > In future versions please, include a change log separated from the > commit message by ---, e.g. > > --- > v6: > foo changed to bar > v5: > xxx removed > --- > > Best regards > > Heinrich Do you have any other feedback, Heinrich?
On 06.06.2024 17:21, Tom Rini wrote: > On Thu, Jun 06, 2024 at 09:02:43AM +0200, Heinrich Schuchardt wrote: >> On 6/5/24 20:43, Maxim Moskalets wrote: >>> From: Maxim Moskalets <maximmosk4@gmail.com> >>> >>> Loading and running the ELF image is the responsibility of the >>> library and should not be associated with the command line interface. >>> >>> It is also required to run ELF images from FIT with the bootm command >>> so as not to depend on the command line interface. >>> >>> Signed-off-by: Maxim Moskalets <maximmosk4@gmail.com> >> In future versions please, include a change log separated from the >> commit message by ---, e.g. >> >> --- >> v6: >> foo changed to bar >> v5: >> xxx removed >> --- >> >> Best regards >> >> Heinrich > Do you have any other feedback, Heinrich? > I will take these comments into account in future patches. Are there any other comments? Best regards Maxim
On Wed, 05 Jun 2024 21:43:34 +0300, Maxim Moskalets wrote: > Loading and running the ELF image is the responsibility of the > library and should not be associated with the command line interface. > > It is also required to run ELF images from FIT with the bootm command > so as not to depend on the command line interface. > > > [...] Applied to u-boot/next, thanks!
diff --git a/cmd/elf.c b/cmd/elf.c index a02361f9f5..32b7462f92 100644 --- a/cmd/elf.c +++ b/cmd/elf.c @@ -19,21 +19,6 @@ #include <linux/linkage.h> #endif -/* Allow ports to override the default behavior */ -static unsigned long do_bootelf_exec(ulong (*entry)(int, char * const[]), - int argc, char *const argv[]) -{ - unsigned long ret; - - /* - * pass address parameter as argv[0] (aka command name), - * and all remaining args - */ - ret = entry(argc, argv); - - return ret; -} - /* Interpreter command to boot an arbitrary ELF image from memory */ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { @@ -43,8 +28,8 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) #endif unsigned long addr; /* Address of the ELF image */ unsigned long rc; /* Return value from user code */ - char *sload = NULL; - int rcode = 0; + int rcode = CMD_RET_SUCCESS; + Bootelf_flags flags = {0}; /* Consume 'bootelf' */ argc--; argv++; @@ -52,7 +37,10 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) /* Check for [-p|-s] flag. */ if (argc >= 1 && (argv[0][0] == '-' && \ (argv[0][1] == 'p' || argv[0][1] == 's'))) { - sload = argv[0]; + if (argv[0][1] == 'p') + flags.phdr = 1; + log_debug("Using ELF header format %s\n", + flags.phdr ? "phdr" : "shdr"); /* Consume flag. */ argc--; argv++; } @@ -75,17 +63,9 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) } else addr = image_load_addr; - if (!valid_elf_image(addr)) - return 1; - - if (sload && sload[1] == 'p') - addr = load_elf_image_phdr(addr); - else - addr = load_elf_image_shdr(addr); - #if CONFIG_IS_ENABLED(CMD_ELF_FDT_SETUP) if (fdt_addr) { - printf("## Setting up FDT at 0x%08lx ...\n", fdt_addr); + log_debug("Setting up FDT at 0x%08lx ...\n", fdt_addr); flush(); if (image_setup_libfdt(&img, (void *)fdt_addr, NULL)) @@ -93,21 +73,27 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) } #endif - if (!env_get_autostart()) - return rcode; - - printf("## Starting application at 0x%08lx ...\n", addr); - flush(); + if (env_get_autostart()) { + flags.autostart = 1; + log_debug("Starting application at 0x%08lx ...\n", addr); + flush(); + } /* * pass address parameter as argv[0] (aka command name), - * and all remaining args + * and all remaining arguments */ - rc = do_bootelf_exec((void *)addr, argc, argv); + rc = bootelf(addr, flags, argc, argv); if (rc != 0) - rcode = 1; + rcode = CMD_RET_FAILURE; - printf("## Application terminated, rc = 0x%lx\n", rc); + if (flags.autostart) + { + if (ENOEXEC == errno) + log_err("Invalid ELF image\n"); + else + log_debug("## Application terminated, rc = 0x%lx\n", rc); + } return rcode; } diff --git a/include/elf.h b/include/elf.h index a4ba74d8ab..b88e6cf403 100644 --- a/include/elf.h +++ b/include/elf.h @@ -12,6 +12,12 @@ #ifndef __ASSEMBLY__ #include "compiler.h" +/* Flag param bits for bootelf() function */ +typedef struct { + unsigned phdr : 1; /* load via program (not section) headers */ + unsigned autostart : 1; /* Start ELF after loading */ +} Bootelf_flags; + /* This version doesn't work for 64-bit ABIs - Erik */ /* These typedefs need to be handled better */ @@ -700,6 +706,10 @@ unsigned long elf_hash(const unsigned char *name); #define R_RISCV_RELATIVE 3 #ifndef __ASSEMBLY__ +unsigned long bootelf_exec(ulong (*entry)(int, char * const[]), + int argc, char *const argv[]); +unsigned long bootelf(unsigned long addr, Bootelf_flags flags, + int argc, char *const argv[]); int valid_elf_image(unsigned long addr); unsigned long load_elf64_image_phdr(unsigned long addr); unsigned long load_elf64_image_shdr(unsigned long addr); diff --git a/lib/elf.c b/lib/elf.c index 9a794f9cba..dc13935e10 100644 --- a/lib/elf.c +++ b/lib/elf.c @@ -7,6 +7,7 @@ #include <cpu_func.h> #include <elf.h> #include <env.h> +#include <errno.h> #include <net.h> #include <vxworks.h> #ifdef CONFIG_X86 @@ -15,6 +16,59 @@ #include <linux/linkage.h> #endif +/** + * bootelf_exec() - start the ELF image execution. + * + * @entry: address of entry point of ELF. + * + * May by used to allow ports to override the default behavior. + */ +unsigned long bootelf_exec(ulong (*entry)(int, char * const[]), + int argc, char *const argv[]) +{ + return entry(argc, argv); +} + +/** + * bootelf() - Boot ELF from memory. + * + * @addr: Loading address of ELF in memory. + * @flags: Bits like ELF_PHDR to control boot details. + * @argc: May be used to pass command line arguments (maybe unused). + * Necessary for backward compatibility with the CLI command. + * If unused, must be 0. + * @argv: see @argc. If unused, must be NULL. + * Return: Number returned by ELF application. + * + * Sets errno = ENOEXEC if the ELF image is not valid. + */ +unsigned long bootelf(unsigned long addr, Bootelf_flags flags, + int argc, char *const argv[]) +{ + unsigned long entry_addr; + char *args[] = {"", NULL}; + + errno = 0; + + if (!valid_elf_image(addr)) { + errno = ENOEXEC; + return 1; + } + + entry_addr = flags.phdr ? load_elf_image_phdr(addr) + : load_elf_image_shdr(addr); + + if (!flags.autostart) + return 0; + + if (!argc && !argv) { + argc = 1; + argv = args; + } + + return bootelf_exec((void *)entry_addr, argc, argv); +} + /* * A very simple ELF64 loader, assumes the image is valid, returns the * entry point address.