diff mbox series

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

Message ID 20230912204345.7271-1-stewart.hildebrand@amd.com (mailing list archive)
State Superseded
Headers show
Series [ImageBuilder,v2] uboot-script-gen: use size from arm64 Image header | expand

Commit Message

Stewart Hildebrand Sept. 12, 2023, 8:43 p.m. UTC
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@amd.com>
---
v1->v2:
* add in-code comments
* use variables more
---
 scripts/uboot-script-gen | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Michal Orzel Sept. 13, 2023, 7:27 a.m. UTC | #1
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@amd.com>
The patch works, thanks!
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

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}'

~Michal
Stewart Hildebrand Sept. 13, 2023, 1:31 p.m. UTC | #2
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@amd.com>
> The patch works, thanks!
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> 
> 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.

Stew
Michal Orzel Sept. 13, 2023, 1:36 p.m. UTC | #3
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@amd.com>
>> The patch works, thanks!
>> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
>>
>> 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
diff mbox series

Patch

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 }')
+
+    # Check for valid arm64 header magic value 0x644d5241
+    if [ "${arm64_header_magic}" = "644d5241" ]
+    then
+        # Read effective size, which may be larger than the filesize due to noload sections, e.g. bss
+        local arm64_header_size=$(od -j 16 -N 8 -t u8 ${filename} | head -n 1 | awk -F' ' '{ print $2 }')
+
+        if [ "${arm64_header_size}" -gt "${size}" ]
+        then
+            size=${arm64_header_size}
+        fi
+    fi
+
     memaddr=$(( $memaddr + $size + $offset - 1))
     memaddr=$(( $memaddr & ~($offset - 1) ))
     memaddr=`printf "0x%X\n" $memaddr`