diff mbox series

[v4] cmd: add FDT setup for bootelf by flag

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

Commit Message

Maxim M. Moskalets Feb. 26, 2024, 6:35 p.m. UTC
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(-)

Comments

Maxim M. Moskalets Feb. 29, 2024, 7:44 p.m. UTC | #1
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?
Tom Rini March 7, 2024, 12:41 p.m. UTC | #2
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.
Maxim M. Moskalets March 7, 2024, 7:35 p.m. UTC | #3
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.
Tom Rini March 7, 2024, 8:26 p.m. UTC | #4
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 mbox series

Patch

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(