[ImageBuilder PATCH] uboot-script-gen: use size from arm64 Image header

[ImageBuilder PATCH] uboot-script-gen: use size from arm64 Image header

[Stewart Hildebrand]

There is a corner case where the filesizes of the xen and Linux kernel images
are not sufficient. These binaries likely contain .NOLOAD sections, which are
not accounted in the filesize.

Check for the presence of an arm64 kernel image header, and get the effective
image size from the header. Use the effective image size for calculating the
next load address and for populating the size in the /chosen/dom*/reg property.

Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
---
 scripts/uboot-script-gen | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index 9656a458ac00..50fe525e7145 100755
--- a/scripts/uboot-script-gen
+++ b/scripts/uboot-script-gen
@@ -2,7 +2,7 @@
 
 offset=$((2*1024*1024))
 filesize=0
-prog_req=(mkimage file fdtput mktemp awk)
+prog_req=(mkimage file fdtput mktemp awk od)
 
 function cleanup_and_return_err()
 {
@@ -435,6 +435,17 @@ function add_size()
 {
     local filename=$1
     local size=`stat -L --printf="%s" $filename`
+
+    if [ "$(od -j 56 -N 4 -t x4 ${filename} | head -n 1 | awk -F' ' '{ print $2 }')" = "644d5241" ]
+    then
+        local size_header=$(od -j 16 -N 8 -t u8 ${filename} | head -n 1 | awk -F' ' '{ print $2 }')
+
+        if [ "${size_header}" -gt "${size}" ]
+        then
+            size=${size_header}
+        fi
+    fi
+
     memaddr=$(( $memaddr + $size + $offset - 1))
     memaddr=$(( $memaddr & ~($offset - 1) ))
     memaddr=`printf "0x%X\n" $memaddr`
-- 
2.42.0

Re: [ImageBuilder PATCH] uboot-script-gen: use size from arm64 Image header

[Stefano Stabellini]

On Thu, 24 Aug 2023, Stewart Hildebrand wrote:
> There is a corner case where the filesizes of the xen and Linux kernel images
> are not sufficient. These binaries likely contain .NOLOAD sections, which are
> not accounted in the filesize.
> 
> Check for the presence of an arm64 kernel image header, and get the effective
> image size from the header. Use the effective image size for calculating the
> next load address and for populating the size in the /chosen/dom*/reg property.
> 
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
> ---
>  scripts/uboot-script-gen | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> index 9656a458ac00..50fe525e7145 100755
> --- a/scripts/uboot-script-gen
> +++ b/scripts/uboot-script-gen
> @@ -2,7 +2,7 @@
>  
>  offset=$((2*1024*1024))
>  filesize=0
> -prog_req=(mkimage file fdtput mktemp awk)
> +prog_req=(mkimage file fdtput mktemp awk od)
>  
>  function cleanup_and_return_err()
>  {
> @@ -435,6 +435,17 @@ function add_size()
>  {
>      local filename=$1
>      local size=`stat -L --printf="%s" $filename`
> +
> +    if [ "$(od -j 56 -N 4 -t x4 ${filename} | head -n 1 | awk -F' ' '{ print $2 }')" = "644d5241" ]
> +    then
> +        local size_header=$(od -j 16 -N 8 -t u8 ${filename} | head -n 1 | awk -F' ' '{ print $2 }')
> +
> +        if [ "${size_header}" -gt "${size}" ]
> +        then
> +            size=${size_header}
> +        fi
> +    fi


Thanks Stewart this is great! Can you please add a good in-code comment
to explain what field you are reading of the header exactly and what is
the value 644d5241 you are comparing against?

Also I think it would be easier to read if you used "cut" instead of
awk and split the line a bit more like this:


    # read header field XXX
    local field_xxx =$(od -j 56 -N 4 -t x4 ${filename} | head -n 1 | cut -d " " -f2)
    # comparing against XXX
    if [ $field_xxx = "644d5241" ]
    then
        # read header field "size" which indicates ....
        local size_header=$(od -j 16 -N 8 -t u8 ${filename} | head -n 1 | cut -d " " -f2)

        if [ "${size_header}" -gt "${size}" ]
        then
            size=${size_header}
        fi
    fi


>      memaddr=$(( $memaddr + $size + $offset - 1))
>      memaddr=$(( $memaddr & ~($offset - 1) ))
>      memaddr=`printf "0x%X\n" $memaddr`
> -- 
> 2.42.0
> 

Re: [ImageBuilder PATCH] uboot-script-gen: use size from arm64 Image header

[Stewart Hildebrand]

On 8/24/23 19:19, Stefano Stabellini wrote:
> On Thu, 24 Aug 2023, Stewart Hildebrand wrote:
>> There is a corner case where the filesizes of the xen and Linux kernel images
>> are not sufficient. These binaries likely contain .NOLOAD sections, which are
>> not accounted in the filesize.
>>
>> Check for the presence of an arm64 kernel image header, and get the effective
>> image size from the header. Use the effective image size for calculating the
>> next load address and for populating the size in the /chosen/dom*/reg property.
>>
>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
>> ---
>>  scripts/uboot-script-gen | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
>> index 9656a458ac00..50fe525e7145 100755
>> --- a/scripts/uboot-script-gen
>> +++ b/scripts/uboot-script-gen
>> @@ -2,7 +2,7 @@
>>
>>  offset=$((2*1024*1024))
>>  filesize=0
>> -prog_req=(mkimage file fdtput mktemp awk)
>> +prog_req=(mkimage file fdtput mktemp awk od)
>>
>>  function cleanup_and_return_err()
>>  {
>> @@ -435,6 +435,17 @@ function add_size()
>>  {
>>      local filename=$1
>>      local size=`stat -L --printf="%s" $filename`
>> +
>> +    if [ "$(od -j 56 -N 4 -t x4 ${filename} | head -n 1 | awk -F' ' '{ print $2 }')" = "644d5241" ]
>> +    then
>> +        local size_header=$(od -j 16 -N 8 -t u8 ${filename} | head -n 1 | awk -F' ' '{ print $2 }')
>> +
>> +        if [ "${size_header}" -gt "${size}" ]
>> +        then
>> +            size=${size_header}
>> +        fi
>> +    fi
> 
> 
> Thanks Stewart this is great! Can you please add a good in-code comment
> to explain what field you are reading of the header exactly and what is
> the value 644d5241 you are comparing against?

Yes

> Also I think it would be easier to read if you used "cut" instead of
> awk and split the line a bit more like this:
> 
> 
>     # read header field XXX
>     local field_xxx =$(od -j 56 -N 4 -t x4 ${filename} | head -n 1 | cut -d " " -f2)
>     # comparing against XXX
>     if [ $field_xxx = "644d5241" ]
>     then
>         # read header field "size" which indicates ....
>         local size_header=$(od -j 16 -N 8 -t u8 ${filename} | head -n 1 | cut -d " " -f2)

od seems to output a varying amount of whitespace between the address and value. In this case, the cut command would seem to want to become cut -d" " -f14 to account for the whitespace. awk is more predictable here, so I will keep awk.