Hi Stefano,
On 11/08/2025 22:38, Stefano Stabellini wrote:
On Wed, 6 Aug 2025, Ayan Kumar Halder wrote:
Introduce the following options :-
1. XEN_LOAD - This specifies command to load xen hypervisor binary and device
tree.
2. DOM0_LOAD - This specifies command to load Dom0 binaries.
3. DOMU_LOAD[] - This specifies command to load DomU binaries.
There can be situations where Xen, Dom0 and DomU binaries are stored in
different partitions. Thus, imagebuilder should provide a way the binaries
using different commands.
If any of the above options are not specified, LOAD_CMD is used by default.
Signed-off-by: Ayan Kumar Halder <ayan.kumar.hal...@amd.com>
The patch is correct and the new config options look good. Two things.
The following check should be removed or, better, modified to account
for the new options:
if test ! "$LOAD_CMD"
then
echo "LOAD_CMD not set, either specify it in the config or set it with the
-t option"
exit 1
fi
Thanks to this patch, it shouldn't be necessary to specify LOAD_CMD any
longer.
Actually, we should keep this check when Linux is loaded (without Xen). IOW,
@@ -1557,6 +1551,11 @@ then
elif test "$LINUX"
then
os="linux"
+ if test ! "$LOAD_CMD"
+ then
+ echo "LOAD_CMD not set, either specify it in the config or set
it with the -t option"
+ exit 1
+ fi
linux_config
find_root_dev and fit are the only two remaining users of LOAD_CMD.
While I think it makes sense to keep LOAD_CMD for fit, find_root_dev
should probably use DOM0_LOAD instead.
yes.
This incremental change (untested) should work. What do you think?
diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index 9e97944..867876f 100755
--- a/scripts/uboot-script-gen
+++ b/scripts/uboot-script-gen
@@ -824,10 +824,10 @@ function check_compressed_file_type()
function find_root_dev()
{
-
- local dev=${LOAD_CMD%:*}
+ local load_cmd=$1
+ local dev=${load_cmd%:*}
dev=${dev##* }
- local par=${LOAD_CMD#*:}
+ local par=${load_cmd#*:}
if [ -z "$dev" ] || [ -z "$par" ]
then
@@ -838,10 +838,10 @@ function find_root_dev()
par=$((par + 1))
- if [[ $LOAD_CMD =~ mmc ]]
+ if [[ $load_cmd =~ mmc ]]
then
root_dev="/dev/mmcblk${dev}p${par}"
- elif [[ $LOAD_CMD =~ scsi ]]
+ elif [[ $load_cmd =~ scsi ]]
then
# converts number to a scsi device character
dev=$((dev + 97))
@@ -912,7 +912,7 @@ function xen_config()
then
DOM0_CMD="$DOM0_CMD root=/dev/ram0"
else
- find_root_dev
+ find_root_dev "$DOM0_LOAD"
# $root_dev is set by find_root_dev
DOM0_CMD="$DOM0_CMD root=$root_dev"
fi
@@ -960,7 +960,7 @@ function linux_config()
then
LINUX_CMD="$LINUX_CMD root=/dev/ram0"
else
- find_root_dev
+ find_root_dev "$LOAD_CMD"
# $root_dev is set by find_root_dev
LINUX_CMD="$LINUX_CMD root=$root_dev"
fi
Till here the change looks ok.
@@ -990,10 +990,6 @@ generate_uboot_images()
xen_file_loading()
{
- if test -z "$DOM0_LOAD"
- then
- DOM0_LOAD="$LOAD_CMD"
- fi
This and
if test "$DOM0_KERNEL"
then
check_compressed_file_type $DOM0_KERNEL "executable\|uImage"
@@ -1065,11 +1061,6 @@ xen_file_loading()
generate_uboot_images
fi
- if test -z "${XEN_LOAD}"
- then
- XEN_LOAD="$LOAD_CMD"
- fi
-
this, I have a concern about moving the changes out of xen_file_loading()
In xen_file_loading(), we set DOM0_LOAD, XEN_LOAD and DOMU_LOAD[i]. With
this change, we set DOM0_LOAD , XEN_LOAD at the beginning of the script
and DOMU_LOAD[i] in the function. This looks a bit odd to me.
Further DOM0_LOAD and XEN_LOAD should be set only when "$XEN" is set.
I can leave the change as it is in the current patch or I can move them
to xen_config().
Please let me know your thoughts.
I have sent "[ImageBuilder v2] Add config option to use separate load
commands for Xen, DOM0 and DOMU binaries" so that it becomes a bit clear.
- Ayan
kernel_addr=$memaddr
kernel_path=$XEN
load_file "$XEN" "host_kernel" "$XEN_LOAD"
@@ -1518,12 +1509,22 @@ then
FIT="${UBOOT_SOURCE%.source}.fit"
fi
-if test ! "$LOAD_CMD"
+if test ! "$LOAD_CMD" && ! test "$XEN_LOAD"
then
echo "LOAD_CMD not set, either specify it in the config or set it with the -t
option"
exit 1
fi
+if test -z "$DOM0_LOAD"
+then
+ DOM0_LOAD="$LOAD_CMD"
+fi
+
+if test -z "${XEN_LOAD}"
+then
+ XEN_LOAD="$LOAD_CMD"
+fi
+
if test ! "$BOOT_CMD"
then
BOOT_CMD="booti"