[ImageBuilder][PATCH v3 0/3] Add extra ImageBuilder features

[ImageBuilder][PATCH v3 0/3] Add extra ImageBuilder features

[Andrei Cherechesu (OSS)]

From: Andrei Cherechesu <andrei.cherechesu@nxp.com>

Hello,

This the v3 version for the patches which have not yet been commited.

Changes in v3:
- Dropped the [PATCH v2 1/4] from the previous series since it's already
been commited
- For PATCH 1/3: No changes
- For PATCH 2/3: Dropped the "-a" parameter and instead sourced the name
of the extra commands file from the ImageBuilder config file, from the
APPEND_EXTRA_CMDS variable. Also described it in README.md.
- For PATCH 3/3: Fixed commit message that still mentioned the "-s"
parameter, which was dropped in v2.

Andrei Cherechesu (3):
  uboot-script-gen: Dynamically compute addr and size when loading
    binaries
  uboot-script-gen: Enable appending extra commands to boot script
  uboot-script-gen: Enable not adding boot command to script

 README.md                |  10 ++-
 scripts/uboot-script-gen | 159 +++++++++++++++++++++++++++++++++------
 2 files changed, 145 insertions(+), 24 deletions(-)

-- 
2.35.1

[ImageBuilder][PATCH v3 1/3] uboot-script-gen: Dynamically compute addr and size when loading binaries

[Andrei Cherechesu (OSS)]

From: Andrei Cherechesu <andrei.cherechesu@nxp.com>

Normally, the script would precompute the sizes of the loaded binaries
and addresses where they are loaded before generating the script,
and the sizes and addresses that needed to be provided to Xen via
/chosen would be hardcoded in the boot script.

Added option via "-s" parameter to use the ${filesize} variable
in u-boot, which is set automatically after a *load command.
The value stored by filesize is now stored in a u-boot env variable
with the name corresponding to the binary that was loaded before.
The newly set variables are now used in setting the /chosen node,
instead of the hardcoded values.

Also, the loading addresses for the files are dynamically computed
and aligned to 0x200000 using the `setexpr` u-boot command. Basically,
if the option is used, there are zero hardcoded addresses inside the
boot script, and everything is determined based on the MEMORY_START
parameter and each binary's size.

If the "-s" parameter is not used, the script does not store the
binaries' sizes and addresses in variables and uses the precomputed
ones when advertising them in the /chosen node.

Signed-off-by: Andrei Cherechesu <andrei.cherechesu@nxp.com>
---
 scripts/uboot-script-gen | 136 ++++++++++++++++++++++++++++++++-------
 1 file changed, 114 insertions(+), 22 deletions(-)

diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index 8f08cd6..f8d2fb0 100755
--- a/scripts/uboot-script-gen
+++ b/scripts/uboot-script-gen
@@ -4,6 +4,9 @@ offset=$((2*1024*1024))
 filesize=0
 prog_req=(mkimage file fdtput mktemp awk)
 
+padding_mask=`printf "0x%X\n" $(($offset - 1))`
+padding_mask_inv=`printf "0x%X\n" $((~$padding_mask))`
+
 function cleanup_and_return_err()
 {
     rm -f $UBOOT_SOURCE $UBOOT_SCRIPT
@@ -91,10 +94,18 @@ function add_device_tree_kernel()
     local size=$3
     local bootargs=$4
 
-    dt_mknode "$path" "module$addr"
-    dt_set "$path/module$addr" "compatible" "str_a" "multiboot,kernel multiboot,module"
-    dt_set "$path/module$addr" "reg" "hex"  "0x0 $addr 0x0 $(printf "0x%x" $size)"
-    dt_set "$path/module$addr" "bootargs" "str" "$bootargs"
+    if test "$dynamic_loading_opt"
+    then
+        dt_mknode "$path" "module\${"$addr"}"
+        dt_set "$path/module\${"$addr"}" "compatible" "str_a" "multiboot,kernel multiboot,module"
+        dt_set "$path/module\${"$addr"}" "reg" "hex"  "0x0 0x\${"$addr"} 0x0 0x\${"$size"}"
+        dt_set "$path/module\${"$addr"}" "bootargs" "str" "$bootargs"
+    else
+        dt_mknode "$path" "module$addr"
+        dt_set "$path/module$addr" "compatible" "str_a" "multiboot,kernel multiboot,module"
+        dt_set "$path/module$addr" "reg" "hex"  "0x0 $addr 0x0 $(printf "0x%x" $size)"
+        dt_set "$path/module$addr" "bootargs" "str" "$bootargs"
+    fi
 }
 
 
@@ -104,9 +115,16 @@ function add_device_tree_ramdisk()
     local addr=$2
     local size=$3
 
-    dt_mknode "$path"  "module$addr"
-    dt_set "$path/module$addr" "compatible" "str_a" "multiboot,ramdisk multiboot,module"
-    dt_set "$path/module$addr" "reg" "hex"  "0x0 $addr 0x0 $(printf "0x%x" $size)"
+    if test "$dynamic_loading_opt"
+    then
+        dt_mknode "$path" "module\${"$addr"}"
+        dt_set "$path/module\${"$addr"}" "compatible" "str_a" "multiboot,ramdisk multiboot,module"
+        dt_set "$path/module\${"$addr"}" "reg" "hex"  "0x0 0x\${"$addr"} 0x0 0x\${"$size"}"
+    else
+        dt_mknode "$path" "module$addr"
+        dt_set "$path/module$addr" "compatible" "str_a" "multiboot,ramdisk multiboot,module"
+        dt_set "$path/module$addr" "reg" "hex"  "0x0 $addr 0x0 $(printf "0x%x" $size)"
+    fi
 }
 
 
@@ -116,9 +134,16 @@ function add_device_tree_passthrough()
     local addr=$2
     local size=$3
 
-    dt_mknode "$path"  "module$addr"
-    dt_set "$path/module$addr" "compatible" "str_a" "multiboot,device-tree multiboot,module"
-    dt_set "$path/module$addr" "reg" "hex"  "0x0 $addr 0x0 $(printf "0x%x" $size)"
+    if test "$dynamic_loading_opt"
+    then
+        dt_mknode "$path" "module\${"$addr"}"
+        dt_set "$path/module\${"$addr"}" "compatible" "str_a" "multiboot,device-tree multiboot,module"
+        dt_set "$path/module\${"$addr"}" "reg" "hex"  "0x0 0x\${"$addr"} 0x0 0x\${"$size"}"
+    else
+        dt_mknode "$path" "module$addr"
+        dt_set "$path/module$addr" "compatible" "str_a" "multiboot,device-tree multiboot,module"
+        dt_set "$path/module$addr" "reg" "hex"  "0x0 $addr 0x0 $(printf "0x%x" $size)"
+    fi
 }
 
 function add_device_tree_mem()
@@ -186,7 +211,12 @@ function xen_device_tree_editing()
     then
         dt_mknode "/chosen" "dom0"
         dt_set "/chosen/dom0" "compatible" "str_a" "xen,linux-zimage xen,multiboot-module multiboot,module"
-        dt_set "/chosen/dom0" "reg" "hex" "0x0 $dom0_kernel_addr 0x0 $(printf "0x%x" $dom0_kernel_size)"
+        if test "$dynamic_loading_opt"
+        then
+            dt_set "/chosen/dom0" "reg" "hex" "0x0 0x\${dom0_linux_addr} 0x0 0x\${dom0_linux_size}"
+        else
+            dt_set "/chosen/dom0" "reg" "hex" "0x0 $dom0_kernel_addr 0x0 $(printf "0x%x" $dom0_kernel_size)"
+        fi
         dt_set "/chosen" "xen,dom0-bootargs" "str" "$DOM0_CMD"
     fi
 
@@ -194,7 +224,12 @@ function xen_device_tree_editing()
     then
         dt_mknode "/chosen" "dom0-ramdisk"
         dt_set "/chosen/dom0-ramdisk" "compatible" "str_a" "xen,linux-initrd xen,multiboot-module multiboot,module"
-        dt_set "/chosen/dom0-ramdisk" "reg" "hex" "0x0 $ramdisk_addr 0x0 $(printf "0x%x" $ramdisk_size)"
+        if test "$dynamic_loading_opt"
+        then
+            dt_set "/chosen/dom0-ramdisk" "reg" "hex" "0x0 0x\${dom0_ramdisk_addr} 0x0 0x\${dom0_ramdisk_size}"
+        else
+            dt_set "/chosen/dom0-ramdisk" "reg" "hex" "0x0 $ramdisk_addr 0x0 $(printf "0x%x" $ramdisk_size)"
+        fi
     fi
 
     i=0
@@ -241,14 +276,29 @@ function xen_device_tree_editing()
             dt_set "/chosen/domU$i" "colors" "hex" "$(printf "0x%x" $bitcolors)"
         fi
 
-        add_device_tree_kernel "/chosen/domU$i" ${domU_kernel_addr[$i]} ${domU_kernel_size[$i]} "${DOMU_CMD[$i]}"
+        if test "$dynamic_loading_opt"
+        then
+            add_device_tree_kernel "/chosen/domU$i" "domU${i}_kernel_addr" "domU${i}_kernel_size" "${DOMU_CMD[$i]}"
+        else
+            add_device_tree_kernel "/chosen/domU$i" ${domU_kernel_addr[$i]} ${domU_kernel_size[$i]} "${DOMU_CMD[$i]}"
+        fi
         if test "${domU_ramdisk_addr[$i]}"
         then
-            add_device_tree_ramdisk "/chosen/domU$i" ${domU_ramdisk_addr[$i]} ${domU_ramdisk_size[$i]}
+            if test "$dynamic_loading_opt"
+            then
+                add_device_tree_ramdisk "/chosen/domU$i" "domU${i}_ramdisk_addr" "domU${i}_ramdisk_size"
+            else
+                add_device_tree_ramdisk "/chosen/domU$i" ${domU_ramdisk_addr[$i]} ${domU_ramdisk_size[$i]}
+            fi
         fi
         if test "${domU_passthrough_dtb_addr[$i]}"
         then
-            add_device_tree_passthrough "/chosen/domU$i" ${domU_passthrough_dtb_addr[$i]} ${domU_passthrough_dtb_size[$i]}
+            if test "$dynamic_loading_opt"
+            then
+                add_device_tree_passthrough "/chosen/domU$i" "domU${i}_fdt_addr" "domU${i}_fdt_size"
+            else
+                add_device_tree_passthrough "/chosen/domU$i" ${domU_passthrough_dtb_addr[$i]} ${domU_passthrough_dtb_size[$i]}
+            fi
         fi
         i=$(( $i + 1 ))
     done
@@ -271,7 +321,12 @@ function device_tree_editing()
 
     if test $UBOOT_SOURCE
     then
-        echo "fdt addr $device_tree_addr" >> $UBOOT_SOURCE
+        if test $dynamic_loading_opt
+        then
+            echo "fdt addr \${host_fdt_addr}" >> $UBOOT_SOURCE
+        else
+            echo "fdt addr $device_tree_addr" >> $UBOOT_SOURCE
+        fi
         echo "fdt resize 1024" >> $UBOOT_SOURCE
 
         if test $NUM_DT_OVERLAY && test $NUM_DT_OVERLAY -gt 0
@@ -306,7 +361,7 @@ function add_size()
 function load_file()
 {
     local filename=$1
-    local fit_scr_name=$2
+    local binary_name=$2
 
     local absolute_path="$(realpath --no-symlinks $filename)"
     local base="$(realpath $PWD)"/
@@ -314,11 +369,30 @@ function load_file()
 
     if test "$FIT"
     then
-        echo "imxtract \$fit_addr $fit_scr_name $memaddr" >> $UBOOT_SOURCE
+        echo "imxtract \$fit_addr $binary_name $memaddr" >> $UBOOT_SOURCE
     else
-        echo "$LOAD_CMD $memaddr ${prepend_path:+$prepend_path/}$relative_path" >> $UBOOT_SOURCE
+        if test "$dynamic_loading_opt"
+        then
+            echo "$LOAD_CMD \${curr_addr} ${prepend_path:+$prepend_path/}$relative_path" >> $UBOOT_SOURCE
+        else
+            echo "$LOAD_CMD $memaddr ${prepend_path:+$prepend_path/}$relative_path" >> $UBOOT_SOURCE
+        fi
     fi
     add_size $filename
+
+    if test "$dynamic_loading_opt" && test ! "$FIT"
+    then
+        # Store each binary's load addr and size
+        local binary_name_addr="${binary_name}_addr"
+        local binary_name_size="${binary_name}_size"
+        echo "setenv $binary_name_addr \${curr_addr}" >> $UBOOT_SOURCE
+        echo "setenv $binary_name_size \${filesize}" >> $UBOOT_SOURCE
+        
+        # Compute load addr for next binary dynamically
+        echo "setexpr curr_addr \${curr_addr} \+ \${filesize}" >> $UBOOT_SOURCE
+        echo "setexpr curr_addr \${curr_addr} \+ \${padding_mask}" >> $UBOOT_SOURCE
+        echo "setexpr curr_addr \${curr_addr} \& \${padding_mask_inv}" >> $UBOOT_SOURCE
+    fi
 }
 
 function check_file_type()
@@ -536,6 +610,14 @@ generate_uboot_images()
 
 xen_file_loading()
 {
+    if test "$dynamic_loading_opt"
+    then
+        local curr_addr=`printf "%x\n" $memaddr`
+        echo "setenv curr_addr $curr_addr" >> $UBOOT_SOURCE
+        echo "setenv padding_mask $padding_mask" >> $UBOOT_SOURCE
+        echo "setenv padding_mask_inv $padding_mask_inv" >> $UBOOT_SOURCE
+    fi
+
     if test "$DOM0_KERNEL"
     then
         check_compressed_file_type $DOM0_KERNEL "executable"
@@ -891,7 +973,7 @@ function print_help
 {
     script=`basename "$0"`
     echo "usage:"
-    echo "	$script -c CONFIG_FILE -d DIRECTORY [-t LOAD_CMD] [-o FILE] [-k KEY_DIR/HINT [-u U-BOOT_DTB]] [-e] [-f] [-p PREPEND_PATH]"
+    echo "	$script -c CONFIG_FILE -d DIRECTORY [-t LOAD_CMD] [-o FILE] [-k KEY_DIR/HINT [-u U-BOOT_DTB]] [-e] [-f] [-p PREPEND_PATH] [-s]"
     echo "	$script -h"
     echo "where:"
     echo "	CONFIG_FILE - configuration file"
@@ -908,6 +990,7 @@ function print_help
     echo "	U-BOOT_DTB - u-boot control dtb so that the public key gets added to it"
     echo "	-f - enable generating a FIT image"
     echo "	PREPEND_PATH - path to be appended before file names to match deploy location within rootfs"
+    echo "	-s - enable dynamic loading of binaries by storing their addresses and sizes u-boot env variables"
     echo "	-h - prints out the help message and exits "
     echo "Defaults:"
     echo "	CONFIG_FILE=$cfg_file, UBOOT_TYPE=\"LOAD_CMD\" env var, DIRECTORY=$uboot_dir"
@@ -915,7 +998,7 @@ function print_help
     echo "	$script -c ../config -d ./build42 -t \"scsi load 1:1\""
 }
 
-while getopts ":c:t:d:ho:k:u:fp:" opt; do
+while getopts ":c:t:d:ho:k:u:fp:s" opt; do
     case ${opt} in
     t )
         case $OPTARG in
@@ -957,6 +1040,9 @@ while getopts ":c:t:d:ho:k:u:fp:" opt; do
     p )
         prepend_path="$OPTARG"
         ;;
+    s )
+        dynamic_loading_opt=y
+        ;;
     h )
         print_help
         exit 0
@@ -1151,7 +1237,13 @@ device_tree_editing $device_tree_addr
 
 # disable device tree reloation
 echo "setenv fdt_high 0xffffffffffffffff" >> $UBOOT_SOURCE
-echo "$BOOT_CMD $kernel_addr - $device_tree_addr" >> $UBOOT_SOURCE
+
+if test "$dynamic_loading_opt"
+then
+    echo "$BOOT_CMD \${host_kernel_addr} - \${host_fdt_addr}" >> $UBOOT_SOURCE
+else
+    echo "$BOOT_CMD $kernel_addr - $device_tree_addr" >> $UBOOT_SOURCE
+fi
 
 if test "$FIT"
 then
-- 
2.35.1

[ImageBuilder][PATCH v3 2/3] uboot-script-gen: Enable appending extra commands to boot script

[Andrei Cherechesu (OSS)]

From: Andrei Cherechesu <andrei.cherechesu@nxp.com>

Added the parsing for APPEND_EXTRA_CMDS variable, which enables the
user to specify the path to a text file that contains, on each line,
u-boot commands that will be added to the generated script as
"fixups", before the boot command.

The file specified by the APPEND_EXTRA_CMDS variable parameter will be
copied as-is in the generated script.

Signed-off-by: Andrei Cherechesu <andrei.cherechesu@nxp.com>
---
 README.md                |  5 +++++
 scripts/uboot-script-gen | 11 +++++++++++
 2 files changed, 16 insertions(+)

diff --git a/README.md b/README.md
index cb15ca5..3566a6d 100644
--- a/README.md
+++ b/README.md
@@ -64,6 +64,7 @@ BOOT_AUX_FILE[1]="uboot.cfg"
 
 UBOOT_SOURCE="boot.source"
 UBOOT_SCRIPT="boot.scr"
+APPEND_EXTRA_CMDS="extra.txt"
 FDTEDIT="imagebuilder.dtb"
 FIT="boot.fit"
 FIT_ENC_KEY_DIR="dir/key"
@@ -197,6 +198,10 @@ Where:
   as you can pass -o FILENAME to uboot-script-gen as a command line
   parameter
 
+- APPEND_EXTRA_CMDS: is optional and specifies the path to a text file
+  containing extra u-boot commands to be added to the boot script before
+  the boot command. Useful for running custom fixup commands.
+
 - FDTEDIT is an optional and is off by default.  Specifies the output
   modified dtb, used for reference and fdt_std.
 
diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index f8d2fb0..f72551a 100755
--- a/scripts/uboot-script-gen
+++ b/scripts/uboot-script-gen
@@ -416,6 +416,10 @@ function check_file_type()
     elif [ "$type" = "Device Tree Blob" ]
     then
         type="Device Tree Blob\|data"
+
+    elif [ "$type" = "text" ]
+    then
+        type="ASCII text"
     fi
 
     file -L $filename | grep "$type" &> /dev/null
@@ -1238,6 +1242,13 @@ device_tree_editing $device_tree_addr
 # disable device tree reloation
 echo "setenv fdt_high 0xffffffffffffffff" >> $UBOOT_SOURCE
 
+# append extra u-boot commands (fixups) to script before boot command
+if test "$APPEND_EXTRA_CMDS"
+then
+    check_file_type "$APPEND_EXTRA_CMDS" "text"
+    cat $APPEND_EXTRA_CMDS >> $UBOOT_SOURCE
+fi
+
 if test "$dynamic_loading_opt"
 then
     echo "$BOOT_CMD \${host_kernel_addr} - \${host_fdt_addr}" >> $UBOOT_SOURCE
-- 
2.35.1

[ImageBuilder][PATCH v3 3/3] uboot-script-gen: Enable not adding boot command to script

[Andrei Cherechesu (OSS)]

From: Andrei Cherechesu <andrei.cherechesu@nxp.com>

If the "BOOT_CMD" variable is set to "none" inside the config
file, the boot command (i.e. "booti") will not by added to the
generated script, to allow the user to customize the u-boot env
or the device-tree after executing the script commands and before
actually booting.

Added commands to store the addresses where the Xen image and
device-tree file are loaded, in 'host_kernel_addr' and 'host_fdt_addr'
variables, if BOOTCMD = "none".

The `booti` command can then be executed as part of the 'bootcmd' variable
in u-boot, which should contain:
	1. fetching the generated u-boot script
	2. executing the script
	3. running `booti ${host_kernel_addr} - ${host_fdt_addr}` command

Signed-off-by: Andrei Cherechesu <andrei.cherechesu@nxp.com>
---
 README.md                |  5 ++++-
 scripts/uboot-script-gen | 16 +++++++++++++---
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/README.md b/README.md
index 3566a6d..07ad432 100644
--- a/README.md
+++ b/README.md
@@ -81,7 +81,10 @@ Where:
 
 - BOOT_CMD specifies the u-boot command used to boot the binaries.
   By default, it is 'booti'. The acceptable values are 'booti', 'bootm'
-  and 'bootefi'.
+  and 'bootefi' and 'none'. If the value is 'none', the BOOT_CMD is not
+  added to the boot script, and the addresses for the Xen binary and the
+  DTB are stored in 'host_kernel_addr' and 'host_fdt_addr' u-boot
+  env variables respectively, to be used manually when booting.
 
 - DEVICE_TREE specifies the DTB file to load.
 
diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index f72551a..6a8a2b8 100755
--- a/scripts/uboot-script-gen
+++ b/scripts/uboot-script-gen
@@ -966,7 +966,7 @@ function check_depends()
 
 function check_boot_cmd()
 {
-    if ! [[ " bootm booti bootefi " =~ " ${BOOT_CMD}" ]]
+    if ! [[ " bootm booti bootefi none " =~ " ${BOOT_CMD}" ]]
     then
         echo "\"BOOT_CMD=$BOOT_CMD\" is not valid"
         exit 1
@@ -1251,9 +1251,19 @@ fi
 
 if test "$dynamic_loading_opt"
 then
-    echo "$BOOT_CMD \${host_kernel_addr} - \${host_fdt_addr}" >> $UBOOT_SOURCE
+    if [ "$BOOT_CMD" != "none" ]
+    then
+        echo "$BOOT_CMD \${host_kernel_addr} - \${host_fdt_addr}" >> $UBOOT_SOURCE
+    fi
 else
-    echo "$BOOT_CMD $kernel_addr - $device_tree_addr" >> $UBOOT_SOURCE
+    if [ "$BOOT_CMD" != "none" ]
+    then
+        echo "$BOOT_CMD $kernel_addr - $device_tree_addr" >> $UBOOT_SOURCE
+    else
+        # skip boot command but store load addresses to be used later
+        echo "setenv host_kernel_addr $kernel_addr" >> $UBOOT_SOURCE
+        echo "setenv host_fdt_addr $device_tree_addr" >> $UBOOT_SOURCE
+    fi
 fi
 
 if test "$FIT"
-- 
2.35.1

Re: [ImageBuilder][PATCH v3 2/3] uboot-script-gen: Enable appending extra commands to boot script

[Stefano Stabellini]

On Wed, 13 Jul 2022, Andrei Cherechesu (OSS) wrote:
> From: Andrei Cherechesu <andrei.cherechesu@nxp.com>
> 
> Added the parsing for APPEND_EXTRA_CMDS variable, which enables the
> user to specify the path to a text file that contains, on each line,
> u-boot commands that will be added to the generated script as
> "fixups", before the boot command.
> 
> The file specified by the APPEND_EXTRA_CMDS variable parameter will be
> copied as-is in the generated script.
> 
> Signed-off-by: Andrei Cherechesu <andrei.cherechesu@nxp.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

> ---
>  README.md                |  5 +++++
>  scripts/uboot-script-gen | 11 +++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/README.md b/README.md
> index cb15ca5..3566a6d 100644
> --- a/README.md
> +++ b/README.md
> @@ -64,6 +64,7 @@ BOOT_AUX_FILE[1]="uboot.cfg"
>  
>  UBOOT_SOURCE="boot.source"
>  UBOOT_SCRIPT="boot.scr"
> +APPEND_EXTRA_CMDS="extra.txt"
>  FDTEDIT="imagebuilder.dtb"
>  FIT="boot.fit"
>  FIT_ENC_KEY_DIR="dir/key"
> @@ -197,6 +198,10 @@ Where:
>    as you can pass -o FILENAME to uboot-script-gen as a command line
>    parameter
>  
> +- APPEND_EXTRA_CMDS: is optional and specifies the path to a text file
> +  containing extra u-boot commands to be added to the boot script before
> +  the boot command. Useful for running custom fixup commands.
> +
>  - FDTEDIT is an optional and is off by default.  Specifies the output
>    modified dtb, used for reference and fdt_std.
>  
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index f8d2fb0..f72551a 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -416,6 +416,10 @@ function check_file_type()
>      elif [ "$type" = "Device Tree Blob" ]
>      then
>          type="Device Tree Blob\|data"
> +
> +    elif [ "$type" = "text" ]
> +    then
> +        type="ASCII text"
>      fi
>  
>      file -L $filename | grep "$type" &> /dev/null
> @@ -1238,6 +1242,13 @@ device_tree_editing $device_tree_addr
>  # disable device tree reloation
>  echo "setenv fdt_high 0xffffffffffffffff" >> $UBOOT_SOURCE
>  
> +# append extra u-boot commands (fixups) to script before boot command
> +if test "$APPEND_EXTRA_CMDS"
> +then
> +    check_file_type "$APPEND_EXTRA_CMDS" "text"
> +    cat $APPEND_EXTRA_CMDS >> $UBOOT_SOURCE
> +fi
> +
>  if test "$dynamic_loading_opt"
>  then
>      echo "$BOOT_CMD \${host_kernel_addr} - \${host_fdt_addr}" >> $UBOOT_SOURCE
> -- 
> 2.35.1
> 

Re: [ImageBuilder][PATCH v3 3/3] uboot-script-gen: Enable not adding boot command to script

[Stefano Stabellini]

On Wed, 13 Jul 2022, Andrei Cherechesu (OSS) wrote:
> From: Andrei Cherechesu <andrei.cherechesu@nxp.com>
> 
> If the "BOOT_CMD" variable is set to "none" inside the config
> file, the boot command (i.e. "booti") will not by added to the
> generated script, to allow the user to customize the u-boot env
> or the device-tree after executing the script commands and before
> actually booting.
> 
> Added commands to store the addresses where the Xen image and
> device-tree file are loaded, in 'host_kernel_addr' and 'host_fdt_addr'
> variables, if BOOTCMD = "none".
> 
> The `booti` command can then be executed as part of the 'bootcmd' variable
> in u-boot, which should contain:
> 	1. fetching the generated u-boot script
> 	2. executing the script
> 	3. running `booti ${host_kernel_addr} - ${host_fdt_addr}` command
> 
> Signed-off-by: Andrei Cherechesu <andrei.cherechesu@nxp.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Given that this patch is non-controversial, I committed it before patch
#1. The small part affecting the "$dynamic_loading_opt" case will need
rebasing.


> ---
>  README.md                |  5 ++++-
>  scripts/uboot-script-gen | 16 +++++++++++++---
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/README.md b/README.md
> index 3566a6d..07ad432 100644
> --- a/README.md
> +++ b/README.md
> @@ -81,7 +81,10 @@ Where:
>  
>  - BOOT_CMD specifies the u-boot command used to boot the binaries.
>    By default, it is 'booti'. The acceptable values are 'booti', 'bootm'
> -  and 'bootefi'.
> +  and 'bootefi' and 'none'. If the value is 'none', the BOOT_CMD is not
> +  added to the boot script, and the addresses for the Xen binary and the
> +  DTB are stored in 'host_kernel_addr' and 'host_fdt_addr' u-boot
> +  env variables respectively, to be used manually when booting.
>  
>  - DEVICE_TREE specifies the DTB file to load.
>  
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index f72551a..6a8a2b8 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -966,7 +966,7 @@ function check_depends()
>  
>  function check_boot_cmd()
>  {
> -    if ! [[ " bootm booti bootefi " =~ " ${BOOT_CMD}" ]]
> +    if ! [[ " bootm booti bootefi none " =~ " ${BOOT_CMD}" ]]
>      then
>          echo "\"BOOT_CMD=$BOOT_CMD\" is not valid"
>          exit 1
> @@ -1251,9 +1251,19 @@ fi
>  
>  if test "$dynamic_loading_opt"
>  then
> -    echo "$BOOT_CMD \${host_kernel_addr} - \${host_fdt_addr}" >> $UBOOT_SOURCE
> +    if [ "$BOOT_CMD" != "none" ]
> +    then
> +        echo "$BOOT_CMD \${host_kernel_addr} - \${host_fdt_addr}" >> $UBOOT_SOURCE
> +    fi
>  else
> -    echo "$BOOT_CMD $kernel_addr - $device_tree_addr" >> $UBOOT_SOURCE
> +    if [ "$BOOT_CMD" != "none" ]
> +    then
> +        echo "$BOOT_CMD $kernel_addr - $device_tree_addr" >> $UBOOT_SOURCE
> +    else
> +        # skip boot command but store load addresses to be used later
> +        echo "setenv host_kernel_addr $kernel_addr" >> $UBOOT_SOURCE
> +        echo "setenv host_fdt_addr $device_tree_addr" >> $UBOOT_SOURCE
> +    fi
>  fi
>  
>  if test "$FIT"
> -- 
> 2.35.1
> 

Re: [ImageBuilder][PATCH v3 1/3] uboot-script-gen: Dynamically compute addr and size when loading binaries

[Stefano Stabellini]

On Wed, 13 Jul 2022, Andrei Cherechesu (OSS) wrote:
> From: Andrei Cherechesu <andrei.cherechesu@nxp.com>
> 
> Normally, the script would precompute the sizes of the loaded binaries
> and addresses where they are loaded before generating the script,
> and the sizes and addresses that needed to be provided to Xen via
> /chosen would be hardcoded in the boot script.
> 
> Added option via "-s" parameter to use the ${filesize} variable
> in u-boot, which is set automatically after a *load command.
> The value stored by filesize is now stored in a u-boot env variable
> with the name corresponding to the binary that was loaded before.
> The newly set variables are now used in setting the /chosen node,
> instead of the hardcoded values.
> 
> Also, the loading addresses for the files are dynamically computed
> and aligned to 0x200000 using the `setexpr` u-boot command. Basically,
> if the option is used, there are zero hardcoded addresses inside the
> boot script, and everything is determined based on the MEMORY_START
> parameter and each binary's size.
> 
> If the "-s" parameter is not used, the script does not store the
> binaries' sizes and addresses in variables and uses the precomputed
> ones when advertising them in the /chosen node.
> 
> Signed-off-by: Andrei Cherechesu <andrei.cherechesu@nxp.com>

This patch is difficult :-)

I like the idea but it makes the code significantly more complex due to
the additional $dynamic_loading_opt case handled everywhere. Initially I
thought about only retain the code path using u-boot variables, at least
after loading the files. However, I realize that it would break the
FDTEDIT case, which I think it would be good to keep working. Also it is
nice to be able to edit the device tree at build time putting in the
right values.

So I tried to eliminated most of the new "if" statements in another way.
The best I could come up with is the below, using eval and additional
bash variables to look up the address and size based on the variable
name (e.g. dom0_kernel). If we want the address, we use the value of
$dom0_kernel_addr, if we want the u-boot variable we use ${dom0_kernel}.

This is untested, just to show the idea. What do you think? Is it
better? Do you prefer the original patch? Other ideas or opinions? 


diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index 18c0ce1..cee22f6 100755
--- a/scripts/uboot-script-gen
+++ b/scripts/uboot-script-gen
@@ -4,6 +4,9 @@ offset=$((2*1024*1024))
 filesize=0
 prog_req=(mkimage file fdtput mktemp awk)
 
+padding_mask=`printf "0x%X\n" $(($offset - 1))`
+padding_mask_inv=`printf "0x%X\n" $((~$padding_mask))`
+
 function cleanup_and_return_err()
 {
     rm -f $UBOOT_SOURCE $UBOOT_SCRIPT
@@ -28,6 +31,7 @@ function dt_mknode()
 #   str
 #   str_a
 #   bool
+#   var
 function dt_set()
 {
     local path=$1
@@ -35,11 +39,21 @@ function dt_set()
     local data_type=$3
     local data=$4
 
+    if test $data_type = "var"
+    then
+        eval data_addr_var="$data"_addr
+        eval data_addr=\$"$data_addr_var"
+        eval data_size_var="$data"_size
+        eval data_size=\$"$data_size_var"
+    fi
 
     if test "$UBOOT_SOURCE" && test ! "$FIT"
     then
         var=${var/\#/\\#}
-        if test $data_type = "hex" || test $data_type = "int"
+        if test $data_type = "var"
+        then
+            echo "fdt set $path $var <0x0 0x\${"$data_addr_var"} 0x0 0x\${"$data_size_var"}>" >> $UBOOT_SOURCE
+        elif test $data_type = "hex" || test $data_type = "int"
         then
             echo "fdt set $path $var <$data>" >> $UBOOT_SOURCE
         elif test $data_type = "str_a"
@@ -63,7 +77,10 @@ function dt_set()
 
     if test $FDTEDIT
     then
-        if test $data_type = "hex"
+        if test $data_type = "var"
+        then
+            fdtput $FDTEDIT -p -t x $path $var 0x0 "$data_addr" 0x0 "$data_size"
+        elif test $data_type = "hex"
         then
             fdtput $FDTEDIT -p -t x $path $var $data
         elif test $data_type = "int"
@@ -87,38 +104,35 @@ function dt_set()
 function add_device_tree_kernel()
 {
     local path=$1
-    local addr=$2
-    local size=$3
-    local bootargs=$4
+    local varname=$2
+    local bootargs=$3
 
-    dt_mknode "$path" "module$addr"
-    dt_set "$path/module$addr" "compatible" "str_a" "multiboot,kernel multiboot,module"
-    dt_set "$path/module$addr" "reg" "hex"  "0x0 $addr 0x0 $(printf "0x%x" $size)"
-    dt_set "$path/module$addr" "bootargs" "str" "$bootargs"
+    dt_mknode "$path" "module-$varname"
+    dt_set "$path/module-$varname" "compatible" "str_a" "multiboot,kernel multiboot,module"
+    dt_set "$path/module-$varname" "reg" "var"  "$varname"
+    dt_set "$path/module-$varname" "bootargs" "str" "$bootargs"
 }
 
 
 function add_device_tree_ramdisk()
 {
     local path=$1
-    local addr=$2
-    local size=$3
+    local varname=$2
 
-    dt_mknode "$path"  "module$addr"
-    dt_set "$path/module$addr" "compatible" "str_a" "multiboot,ramdisk multiboot,module"
-    dt_set "$path/module$addr" "reg" "hex"  "0x0 $addr 0x0 $(printf "0x%x" $size)"
+    dt_mknode "$path" "module-$varname"
+    dt_set "$path/module-$varname" "compatible" "str_a" "multiboot,ramdisk multiboot,module"
+    dt_set "$path/module-$varname" "reg" "var"  "$varname"
 }
 
 
 function add_device_tree_passthrough()
 {
     local path=$1
-    local addr=$2
-    local size=$3
+    local varname=$2
 
-    dt_mknode "$path"  "module$addr"
-    dt_set "$path/module$addr" "compatible" "str_a" "multiboot,device-tree multiboot,module"
-    dt_set "$path/module$addr" "reg" "hex"  "0x0 $addr 0x0 $(printf "0x%x" $size)"
+    dt_mknode "$path" "module-$varname"
+    dt_set "$path/module-$varname" "compatible" "str_a" "multiboot,device-tree multiboot,module"
+    dt_set "$path/module-$varname" "reg" "var"  "$varname"
 }
 
 function add_device_tree_mem()
@@ -186,7 +200,7 @@ function xen_device_tree_editing()
     then
         dt_mknode "/chosen" "dom0"
         dt_set "/chosen/dom0" "compatible" "str_a" "xen,linux-zimage xen,multiboot-module multiboot,module"
-        dt_set "/chosen/dom0" "reg" "hex" "0x0 $dom0_kernel_addr 0x0 $(printf "0x%x" $dom0_kernel_size)"
+        dt_set "/chosen/dom0" "reg" "var" "dom0_linux"
         dt_set "/chosen" "xen,dom0-bootargs" "str" "$DOM0_CMD"
     fi
 
@@ -194,7 +208,7 @@ function xen_device_tree_editing()
     then
         dt_mknode "/chosen" "dom0-ramdisk"
         dt_set "/chosen/dom0-ramdisk" "compatible" "str_a" "xen,linux-initrd xen,multiboot-module multiboot,module"
-        dt_set "/chosen/dom0-ramdisk" "reg" "hex" "0x0 $ramdisk_addr 0x0 $(printf "0x%x" $ramdisk_size)"
+        dt_set "/chosen/dom0-ramdisk" "reg" "var" "dom0_ramdisk"
     fi
 
     i=0
@@ -241,14 +255,14 @@ function xen_device_tree_editing()
             dt_set "/chosen/domU$i" "colors" "hex" "$(printf "0x%x" $bitcolors)"
         fi
 
-        add_device_tree_kernel "/chosen/domU$i" ${domU_kernel_addr[$i]} ${domU_kernel_size[$i]} "${DOMU_CMD[$i]}"
+        add_device_tree_kernel "/chosen/domU$i" "domU${i}_kernel" "${DOMU_CMD[$i]}"
         if test "${domU_ramdisk_addr[$i]}"
         then
-            add_device_tree_ramdisk "/chosen/domU$i" ${domU_ramdisk_addr[$i]} ${domU_ramdisk_size[$i]}
+            add_device_tree_ramdisk "/chosen/domU$i" "domU${i}_ramdisk"
         fi
         if test "${domU_passthrough_dtb_addr[$i]}"
         then
-            add_device_tree_passthrough "/chosen/domU$i" ${domU_passthrough_dtb_addr[$i]} ${domU_passthrough_dtb_size[$i]}
+            add_device_tree_passthrough "/chosen/domU$i" "domU${i}_fdt"
         fi
         i=$(( $i + 1 ))
     done
@@ -271,7 +285,7 @@ function device_tree_editing()
 
     if test $UBOOT_SOURCE
     then
-        echo "fdt addr $device_tree_addr" >> $UBOOT_SOURCE
+        echo "fdt addr \${host_fdt_addr}" >> $UBOOT_SOURCE
         echo "fdt resize 1024" >> $UBOOT_SOURCE
 
         if test $NUM_DT_OVERLAY && test $NUM_DT_OVERLAY -gt 0
@@ -296,11 +310,25 @@ function device_tree_editing()
 function add_size()
 {
     local filename=$1
+    local fit_scr_name=$2
+    local binary_name_addr="${fit_scr_name}_addr"
+    local binary_name_size="${fit_scr_name}_size"
+    eval "$fit_scr_name"_addr=$memaddr
+
+    echo "setenv $binary_name_addr \${memaddr}" >> $UBOOT_SOURCE
+    echo "setenv $binary_name_size \${filesize}" >> $UBOOT_SOURCE
+    # Compute load addr for next binary dynamically
+    echo "setexpr memaddr \${memaddr} \+ \${filesize}" >> $UBOOT_SOURCE
+    echo "setexpr memaddr \${memaddr} \+ $padding_mask" >> $UBOOT_SOURCE
+    echo "setexpr memaddr \${memaddr} \& $padding_mask_inv" >> $UBOOT_SOURCE
+
     local size=`stat -L --printf="%s" $filename`
     memaddr=$(( $memaddr + $size + $offset - 1))
     memaddr=$(( $memaddr & ~($offset - 1) ))
     memaddr=`printf "0x%X\n" $memaddr`
     filesize=$size
+
+    eval "$fit_scr_name"_size=`printf "0x%X\n" $size`
 }
 
 function load_file()
@@ -315,10 +343,13 @@ function load_file()
     if test "$FIT"
     then
         echo "imxtract \$fit_addr $fit_scr_name $memaddr" >> $UBOOT_SOURCE
+    elif test "$dynamic_loading_opt"
+    then
+        echo "$LOAD_CMD \${memaddr} ${prepend_path:+$prepend_path/}$relative_path" >> $UBOOT_SOURCE
     else
         echo "$LOAD_CMD $memaddr ${prepend_path:+$prepend_path/}$relative_path" >> $UBOOT_SOURCE
     fi
-    add_size $filename
+    add_size $filename $fit_scr_name
 }
 
 function check_file_type()
@@ -899,7 +930,7 @@ function print_help
 {
     script=`basename "$0"`
     echo "usage:"
-    echo "	$script -c CONFIG_FILE -d DIRECTORY [-t LOAD_CMD] [-o FILE] [-k KEY_DIR/HINT [-u U-BOOT_DTB]] [-e] [-f] [-p PREPEND_PATH]"
+    echo "	$script -c CONFIG_FILE -d DIRECTORY [-t LOAD_CMD] [-o FILE] [-k KEY_DIR/HINT [-u U-BOOT_DTB]] [-e] [-f] [-p PREPEND_PATH] [-s]"
     echo "	$script -h"
     echo "where:"
     echo "	CONFIG_FILE - configuration file"
@@ -916,6 +947,7 @@ function print_help
     echo "	U-BOOT_DTB - u-boot control dtb so that the public key gets added to it"
     echo "	-f - enable generating a FIT image"
     echo "	PREPEND_PATH - path to be appended before file names to match deploy location within rootfs"
+    echo "	-s - enable dynamic loading of binaries by storing their addresses and sizes u-boot env variables"
     echo "	-h - prints out the help message and exits "
     echo "Defaults:"
     echo "	CONFIG_FILE=$cfg_file, UBOOT_TYPE=\"LOAD_CMD\" env var, DIRECTORY=$uboot_dir"
@@ -923,7 +955,7 @@ function print_help
     echo "	$script -c ../config -d ./build42 -t \"scsi load 1:1\""
 }
 
-while getopts ":c:t:d:ho:k:u:fp:" opt; do
+while getopts ":c:t:d:ho:k:u:fp:s" opt; do
     case ${opt} in
     t )
         case $OPTARG in
@@ -965,6 +997,9 @@ while getopts ":c:t:d:ho:k:u:fp:" opt; do
     p )
         prepend_path="$OPTARG"
         ;;
+    s )
+        dynamic_loading_opt=y
+        ;;
     h )
         print_help
         exit 0
@@ -1126,6 +1161,7 @@ uboot_addr=$memaddr
 # 2MB are enough for a uboot script
 memaddr=$(( $memaddr + $offset ))
 memaddr=`printf "0x%X\n" $memaddr`
+echo "setenv memaddr $memaddr" >> $UBOOT_SOURCE
 
 if test "$os" = "xen"
 then
@@ -1169,11 +1205,7 @@ fi
 
 if [ "$BOOT_CMD" != "none" ]
 then
-    echo "$BOOT_CMD $kernel_addr - $device_tree_addr" >> $UBOOT_SOURCE
-else
-    # skip boot command but store load addresses to be used later
-    echo "setenv host_kernel_addr $kernel_addr" >> $UBOOT_SOURCE
-    echo "setenv host_fdt_addr $device_tree_addr" >> $UBOOT_SOURCE
+    echo "$BOOT_CMD \${host_kernel_addr} - \${host_fdt_addr}" >> $UBOOT_SOURCE
 fi
 
 if test "$FIT"