Message ID | 20240226183553.11615-1-Maxim.Moskalets@kaspersky.com |
---|---|
State | Changes Requested |
Delegated to: | Tom Rini |
Headers | show |
Series | [v4] cmd: add FDT setup for bootelf by flag | expand |
Hello Tom, On 26.02.2024 21:35, Maxim Moskalets wrote: > From: Maxim Moskalets <maximmosk4@gmail.com> > > Added the ability to use FDT for ELF applications, required to run some > OS. To make FDT setup, you need to set the -d fdt_addr_r cmd option for > bootelf command. > > Signed-off-by: Maxim Moskalets <Maxim.Moskalets@kaspersky.com> > > Cc: Tom Rini <trini@konsulko.com> > --- > cmd/elf.c | 27 ++++++++++++++++++++++++--- > 1 file changed, 24 insertions(+), 3 deletions(-) > > diff --git a/cmd/elf.c b/cmd/elf.c > index b7b9f506a5..2fa28448c0 100644 > --- a/cmd/elf.c > +++ b/cmd/elf.c > @@ -38,6 +38,8 @@ static unsigned long do_bootelf_exec(ulong (*entry)(int, char * const[]), > /* Interpreter command to boot an arbitrary ELF image from memory */ > int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > { > + struct bootm_headers img = {0}; > + unsigned long fdt_addr = 0; /* Address of the FDT */ > unsigned long addr; /* Address of the ELF image */ > unsigned long rc; /* Return value from user code */ > char *sload = NULL; > @@ -46,13 +48,23 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > /* Consume 'bootelf' */ > argc--; argv++; > > - /* Check for flag. */ > + /* Check for [-p|-s] flag. */ > if (argc >= 1 && (argv[0][0] == '-' && \ > (argv[0][1] == 'p' || argv[0][1] == 's'))) { > sload = argv[0]; > /* Consume flag. */ > argc--; argv++; > } > + > + /* Check for [-d fdt_addr_r] option. */ > + if ((argc >= 2) && (argv[0][0] == '-') && (argv[0][1] == 'd')) { > + if (strict_strtoul(argv[1], 16, &fdt_addr) != 0) > + return CMD_RET_USAGE; > + /* Consume option. */ > + argc -= 2; > + argv += 2; > + } > + > /* Check for address. */ > if (argc >= 1 && strict_strtoul(argv[0], 16, &addr) != -EINVAL) { > /* Consume address */ > @@ -68,6 +80,14 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > else > addr = load_elf_image_shdr(addr); > > + if (fdt_addr) { > + printf("## Setting up FDT at 0x%08lx ...\n", fdt_addr); > + flush(); > + > + if (image_setup_libfdt(&img, (void *)fdt_addr, NULL)) > + return 1; > + } > + > if (!env_get_autostart()) > return rcode; > > @@ -298,9 +318,10 @@ int do_bootvx(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) > U_BOOT_CMD( > bootelf, CONFIG_SYS_MAXARGS, 0, do_bootelf, > "Boot from an ELF image in memory", > - "[-p|-s] [address]\n" > + "[-p|-s] [-d fdt_addr_r] [address]\n" > "\t- load ELF image at [address] via program headers (-p)\n" > - "\t or via section headers (-s)" > + "\t or via section headers (-s)\n" > + "\t- setup FDT image at [fdt_addr_r] (-d)" > ); > > U_BOOT_CMD( Is it ok to merge?
On Mon, Feb 26, 2024 at 09:35:53PM +0300, Maxim Moskalets wrote: > From: Maxim Moskalets <maximmosk4@gmail.com> > > Added the ability to use FDT for ELF applications, required to run some > OS. To make FDT setup, you need to set the -d fdt_addr_r cmd option for > bootelf command. > > Signed-off-by: Maxim Moskalets <Maxim.Moskalets@kaspersky.com> > > Cc: Tom Rini <trini@konsulko.com> > --- > cmd/elf.c | 27 ++++++++++++++++++++++++--- > 1 file changed, 24 insertions(+), 3 deletions(-) This breaks building on a number of platforms such as xilinx_zynqmp_mini_nand_single.
On 07.03.2024 15:41, Tom Rini wrote: > On Mon, Feb 26, 2024 at 09:35:53PM +0300, Maxim Moskalets wrote: > >> From: Maxim Moskalets <maximmosk4@gmail.com> >> >> Added the ability to use FDT for ELF applications, required to run some >> OS. To make FDT setup, you need to set the -d fdt_addr_r cmd option for >> bootelf command. >> >> Signed-off-by: Maxim Moskalets <Maxim.Moskalets@kaspersky.com> >> >> Cc: Tom Rini <trini@konsulko.com> >> --- >> cmd/elf.c | 27 ++++++++++++++++++++++++--- >> 1 file changed, 24 insertions(+), 3 deletions(-) > This breaks building on a number of platforms such as > xilinx_zynqmp_mini_nand_single. > There's an unresolved reference to LMB functions. This code compiles without a patch (i.e. it is not disabled by the preprocessor or externally), but when linking, since this function is not called, it is discarded (you can check on a clean build with the command aarch64-linux-gnu-objdump -d u-boot | grep image_setup_libfdt). I think it's worth fixing this bug too.
On Thu, Mar 07, 2024 at 10:35:35PM +0300, Maxim M. Moskalets wrote: > On 07.03.2024 15:41, Tom Rini wrote: > > On Mon, Feb 26, 2024 at 09:35:53PM +0300, Maxim Moskalets wrote: > > > > > From: Maxim Moskalets <maximmosk4@gmail.com> > > > > > > Added the ability to use FDT for ELF applications, required to run some > > > OS. To make FDT setup, you need to set the -d fdt_addr_r cmd option for > > > bootelf command. > > > > > > Signed-off-by: Maxim Moskalets <Maxim.Moskalets@kaspersky.com> > > > > > > Cc: Tom Rini <trini@konsulko.com> > > > --- > > > cmd/elf.c | 27 ++++++++++++++++++++++++--- > > > 1 file changed, 24 insertions(+), 3 deletions(-) > > This breaks building on a number of platforms such as > > xilinx_zynqmp_mini_nand_single. > > > There's an unresolved reference to LMB functions. This code compiles > without a patch (i.e. it is not disabled by the preprocessor or externally), > but when linking, since this function is not called, it is discarded > (you can check on a clean build with the command > aarch64-linux-gnu-objdump -d u-boot | grep image_setup_libfdt). > I think it's worth fixing this bug too. Yes, you need to re-structure this patch so the existing configurations still build. You might need to guard some areas with CONFIG_IS_ENABLED(...some existing symbol) before you call image_setup_libfdt.
diff --git a/cmd/elf.c b/cmd/elf.c index b7b9f506a5..2fa28448c0 100644 --- a/cmd/elf.c +++ b/cmd/elf.c @@ -38,6 +38,8 @@ static unsigned long do_bootelf_exec(ulong (*entry)(int, char * const[]), /* Interpreter command to boot an arbitrary ELF image from memory */ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { + struct bootm_headers img = {0}; + unsigned long fdt_addr = 0; /* Address of the FDT */ unsigned long addr; /* Address of the ELF image */ unsigned long rc; /* Return value from user code */ char *sload = NULL; @@ -46,13 +48,23 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) /* Consume 'bootelf' */ argc--; argv++; - /* Check for flag. */ + /* Check for [-p|-s] flag. */ if (argc >= 1 && (argv[0][0] == '-' && \ (argv[0][1] == 'p' || argv[0][1] == 's'))) { sload = argv[0]; /* Consume flag. */ argc--; argv++; } + + /* Check for [-d fdt_addr_r] option. */ + if ((argc >= 2) && (argv[0][0] == '-') && (argv[0][1] == 'd')) { + if (strict_strtoul(argv[1], 16, &fdt_addr) != 0) + return CMD_RET_USAGE; + /* Consume option. */ + argc -= 2; + argv += 2; + } + /* Check for address. */ if (argc >= 1 && strict_strtoul(argv[0], 16, &addr) != -EINVAL) { /* Consume address */ @@ -68,6 +80,14 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) else addr = load_elf_image_shdr(addr); + if (fdt_addr) { + printf("## Setting up FDT at 0x%08lx ...\n", fdt_addr); + flush(); + + if (image_setup_libfdt(&img, (void *)fdt_addr, NULL)) + return 1; + } + if (!env_get_autostart()) return rcode; @@ -298,9 +318,10 @@ int do_bootvx(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) U_BOOT_CMD( bootelf, CONFIG_SYS_MAXARGS, 0, do_bootelf, "Boot from an ELF image in memory", - "[-p|-s] [address]\n" + "[-p|-s] [-d fdt_addr_r] [address]\n" "\t- load ELF image at [address] via program headers (-p)\n" - "\t or via section headers (-s)" + "\t or via section headers (-s)\n" + "\t- setup FDT image at [fdt_addr_r] (-d)" ); U_BOOT_CMD(