| Message ID | 20230824182233.50760-1-stewart.hildebrand@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ImageBuilder] uboot-script-gen: use size from arm64 Image header | expand |
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 >
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.
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`
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(-)