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

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

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@xxxxxxx>
>> ---
>>  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.