[PATCH 2/3] cmd: pxe: support INITRD and FDT selection with FIT

[Neil Armstrong]

Hi Patrick,

On 28/10/2022 11:01, Patrick Delaunay wrote:
> Since the commit d5ba6188dfbf ("cmd: pxe_utils: Check fdtcontroladdr
> in label_boot") the FDT or the FDTDIR label is required in extlinux.conf
> and the fallback done by bootm command when only the device tree present
> in this command parameters is no more performed when FIT is used for
> kernel.
> 
> When the label FDT or FDTDIR are absent or if the device tree file is
> absent, the PXE command in U-Boot uses the default U-Boot device tree
> selected by fdtcontroladdr = gd->fdt_blob, it is the "Scenario 3".
> 
> With this scenario the bootm FIP fallback is no more possible with
> the extlinux.conf when only "kernel" label is present and is a FIP:
> 
>    kernel <path>#<conf>[#<extra-conf[#...]]
> 
> As the U-Boot FDT is always provided in the third bootm argument,
> the device tree found in FIP is not used as fallback, it was done
> previously in boot_get_fdt().
> 
> This patch adds a new field kernel_label to save the full kernel label.
> The FDT bootm parameters use the kernel address (to avoid to load a
> second time the same FIP) and the config when this full label is reused
> for "fdt" or "initrd" label.
> 
> This FIP support in extlinux.conf is restored when the "FDT" label
> can be found and select the same FIP (identical file and configuration):
> 
>    kernel <path>#<conf>[#<extra-conf[#...]]
>    fdt <path>#<conf>[#<extra-conf[#...]]
> 
> The patch add also this possibility for initrd.
> 
>    initrd <path>#<conf>[#<extra-conf[#...]]


Thanks for providing this solution, indeed it solves the original
issue and permits specifying different DT and INITRD configs which
is neat.

I haven't tested it yet, but so far:

Reviewed-by: Neil Armstrong <neil.armstrong at linaro.org>

> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay at foss.st.com>
> ---
> 
>   boot/pxe_utils.c    | 17 ++++++++++++++---
>   doc/README.pxe      |  8 ++++++++
>   include/pxe_utils.h |  2 ++
>   3 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
> index 844ab72252bf..756b201eda91 100644
> --- a/boot/pxe_utils.c
> +++ b/boot/pxe_utils.c
> @@ -259,6 +259,7 @@ static struct pxe_label *label_create(void)
>   static void label_destroy(struct pxe_label *label)
>   {
>   	free(label->name);
> +	free(label->kernel_label);
>   	free(label->kernel);
>   	free(label->config);
>   	free(label->append);
> @@ -543,9 +544,11 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label)
>   		kernel_addr = fit_addr;
>   	}
>   
> -	if (label->initrd) {
> +	/* For FIT, the label can be identical to kernel one */
> +	if (label->initrd && !strcmp(label->kernel_label, label->initrd)) {
> +		initrd_addr_str =  kernel_addr;
> +	} else if (label->initrd) {
>   		ulong size;
> -
>   		if (get_relfile_envaddr(ctx, label->initrd, "ramdisk_addr_r",
>   					&size) < 0) {
>   			printf("Skipping %s for failure retrieving initrd\n",
> @@ -623,8 +626,11 @@ static int label_boot(struct pxe_context *ctx, struct pxe_label *label)
>   	 */
>   	bootm_argv[3] = env_get("fdt_addr_r");
>   
> +	/* For FIT, the label can be identical to kernel one */
> +	if (label->fdt && !strcmp(label->kernel_label, label->fdt)) {
> +		bootm_argv[3] = kernel_addr;
>   	/* if fdt label is defined then get fdt from server */
> -	if (bootm_argv[3]) {
> +	} else if (bootm_argv[3]) {
>   		char *fdtfile = NULL;
>   		char *fdtfilefree = NULL;
>   
> @@ -1165,6 +1171,11 @@ static int parse_label_kernel(char **c, struct pxe_label *label)
>   	if (err < 0)
>   		return err;
>   
> +	/* copy the kernel label to compare with FDT / INITRD when FIT is used */
> +	label->kernel_label = strdup(label->kernel);
> +	if (!label->kernel_label)
> +		return -ENOMEM;
> +
>   	s = strstr(label->kernel, "#");
>   	if (!s)
>   		return 1;
> diff --git a/doc/README.pxe b/doc/README.pxe
> index d14d2bdcc9b0..172201093d02 100644
> --- a/doc/README.pxe
> +++ b/doc/README.pxe
> @@ -179,11 +179,19 @@ initrd <path>	    - if this label is chosen, use tftp to retrieve the initrd
>   		      at <path>. it will be stored at the address indicated in
>   		      the initrd_addr_r environment variable, and that address
>   		      will be passed to bootm.
> +		      For FIT image, the initrd can be provided with the same value than
> +		      kernel, including configuration:
> +		        <path>#<conf>[#<extra-conf[#...]]
> +		      In this case, kernel_addr_r is passed to bootm.
>   
>   fdt <path>	    - if this label is chosen, use tftp to retrieve the fdt blob
>   		      at <path>. it will be stored at the address indicated in
>   		      the fdt_addr_r environment variable, and that address will
>   		      be passed to bootm.
> +		      For FIT image, the device tree can be provided with the same value
> +		      than kernel, including configuration:
> +		        <path>#<conf>[#<extra-conf[#...]]
> +		      In this case, kernel_addr_r is passed to bootm.
>   
>   devicetree <path>   - if this label is chosen, use tftp to retrieve the fdt blob
>   		      at <path>. it will be stored at the address indicated in
> diff --git a/include/pxe_utils.h b/include/pxe_utils.h
> index 4a73b2aace34..1e5e8424f53f 100644
> --- a/include/pxe_utils.h
> +++ b/include/pxe_utils.h
> @@ -28,6 +28,7 @@
>    * Create these with the 'label_create' function given below.
>    *
>    * name - the name of the menu as given on the 'menu label' line.
> + * kernel_label - the kernel label, including FIT config if present.
>    * kernel - the path to the kernel file to use for this label.
>    * append - kernel command line to use when booting this label
>    * initrd - path to the initrd to use for this label.
> @@ -40,6 +41,7 @@ struct pxe_label {
>   	char num[4];
>   	char *name;
>   	char *menu;
> +	char *kernel_label;
>   	char *kernel;
>   	char *config;
>   	char *append;