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

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Stewart,

On 13/09/2023 15:31, Stewart Hildebrand wrote:
> On 9/13/23 03:27, Michal Orzel wrote:
>> Hi Stewart,
>>
>> On 12/09/2023 22:43, 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 (e.g. 
>>> bss),
>>> 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>
>> The patch works, thanks!
>> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
>>
>> with one little suggestion...
>>
>>> ---
>>> v1->v2:
>>> * add in-code comments
>>> * use variables more
>>> ---
>>>  scripts/uboot-script-gen | 17 ++++++++++++++++-
>>>  1 file changed, 16 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
>>> index 9656a458ac00..f0972d983017 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,21 @@ function add_size()
>>>  {
>>>      local filename=$1
>>>      local size=`stat -L --printf="%s" $filename`
>>> +    # Read arm64 header magic 
>>> (https://www.kernel.org/doc/Documentation/arm64/booting.txt)
>>> +    local arm64_header_magic=$(od -j 56 -N 4 -t x4 ${filename} | head -n 1 
>>> | awk -F' ' '{ print $2 }')
>> can we simplify:
>> head -n 1 | awk -F' ' '{ print $2 }'
>> by just doing:
>> awk 'NR==1 {print $2}'
> 
> Yes. I'll send v3.
I think it can be done on commit, so please check with Stefano.

~Michal