Gracefully handle 64-bit Load Address and Entry Point Address mkimage parameters
PATCH 1/1 Gracefully handle 64-bit Load Address and Entry Point Address mkimage parameters
From: no one <[email protected]>
Recent MIPS Linux kernels are using a 64-bit value for the load address (0xffffffff80010000) for the Creator CI20 board kernel. When this argument was passed to the mkimage program running on a 32-bit machine such as the Creator CI20 board the load address was incorrectly obtained from the first half of the argument, 0xffffffff by the strtoul. The mkimage should be able to tolerate the longer, 64-bit signed version of the arguments with the use of strtoull.
--- tools/mkimage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
tools/mkimage.c +2 -2
@@ -129,7 +129,7 @@ int main(int argc, char **argv)case 'a' :if (--argc <=0 )usage (); - params.addr =strtoul (*++argv, &ptr,16 ); + params.addr =strtoull (*++argv, &ptr,16 );if (*ptr) { fprintf (stderr, "%s: invalid load address %s\n",
@@ -146,7 +146,7 @@ int main(int argc, char **argv)case 'e' :if (--argc <=0 )usage (); - params.ep =strtoul (*++argv, &ptr,16 ); + params.ep =strtoull (*++argv, &ptr,16 );if (*ptr) { fprintf (stderr, "%s: invalid entry point %s\n",
Signature · 1 line
2.5.0
PATCH 1/1 Gracefully handle 64-bit Load Address and Entry Point Address mkimage parameters
Dear William,
Quoted 8 lines · In message <[email protected]> you wrote:
Recent MIPS Linux kernels are using a 64-bit value for the load address (
0xffffffff80010000) for the Creator CI20 board kernel. When this argument was passed to the mkimage program running on a 32-bit machine such as the Creator CI20 board the load address was incorrectly obtained from the first half of the argument,0xffffffffby the strtoul. The mkimage should be able to tolerate the longer, 64-bit signed version of the arguments with the use of strtoull.
Hm... I think we have multiple problems here.
First, the old legacy uImage header usersd 32 bit data types for the addresses. This is a binary data structure, and there is no way to extend it for 64 bit addresses without breaking compatibility.
tools/mkimage.c int main(int argc, char **argv) · line 132
Quoted 2 lines
tools/mkimage.c +1
-1- params.addr =strtoul (*++argv, &ptr,16 ); + params.addr =strtoull (*++argv, &ptr,16 );
I don't see what you win here... strtoull() returns unsigned long long, but params.addr is an unsigned int, so the value will be truncated to 32 bit. Are you sure this is what you want?
tools/mkimage.c int main(int argc, char **argv) · line 149
Quoted 8 lines
tools/mkimage.c +1
-1fprintf (stderr, "%s: invalid load address %s\n",@@ -146,7 +146,7 @@ int main(int argc, char **argv)case 'e' :if (--argc <=0 )usage (); - params.ep =strtoul (*++argv, &ptr,16 ); + params.ep =strtoull (*++argv, &ptr,16 );
Ditto...
Also please note that your patch triggers a number of checkpatch warnings and an error, especially:
WARNING: space prohibited between function name and open parenthesis '(' #108: FILE: tools/mkimage.c:132:
+ params.addr = strtoull (*++argv, &ptr, 16);
WARNING: space prohibited between function name and open parenthesis '(' #117: FILE: tools/mkimage.c:149:
+ params.ep = strtoull (*++argv, &ptr, 16);
ERROR: Missing Signed-off-by: line(s)
Best regards,
Wolfgang Denk
Signature · 6 lines
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
PoB = "Prisoner of Bill" -- those held captive, unwillingly or other-
wise, by the contemptible Microsoft monopoly.
-- Tom Christiansen in <[email protected]>PATCH 1/1 Gracefully handle 64-bit Load Address and Entry Point Address mkimage parameters
Quoted 12 lines · Wolfgang Denk
Dear William,
Quoted 8 lines · In message <[email protected]> you wrote:
Recent MIPS Linux kernels are using a 64-bit value for the load address (
0xffffffff80010000) for the Creator CI20 board kernel. When this argument was passed to the mkimage program running on a 32-bit machine such as the Creator CI20 board the load address was incorrectly obtained from the first half of the argument,0xffffffffby the strtoul. The mkimage should be able to tolerate the longer, 64-bit signed version of the arguments with the use of strtoull.
Quoted 1 line
Hm... I think we have multiple problems here.
Hi,
The write up in the patch wasn't as clear as it should have been. I have revised the patch to try to make the reason for this patch clearer. The MIP Linux kernel is using sign-extending 32-bit values to 64-bit values, so the truncation of the strtoll values to 32-bits is actually desired. The revised patch now passes the linux kernel checkpatch.pl checks. I will be resending the patch in a moment.
-Will
Quoted 43 lines
First, the old legacy uImage header usersd 32 bit data types for the addresses. This is a binary data structure, and there is no way to extend it for 64 bit addresses without breaking compatibility.
tools/mkimage.c int main(int argc, char **argv) · line 132
Quoted 2 lines
tools/mkimage.c +1
-1- params.addr =strtoul (*++argv, &ptr,16 ); + params.addr =strtoull (*++argv, &ptr,16 );I don't see what you win here...
strtoull()returns unsigned long long, but params.addr is an unsigned int, so the value will be truncated to 32 bit. Are you sure this is what you want?tools/mkimage.c int main(int argc, char **argv) · line 149
Quoted 8 lines
tools/mkimage.c +1
-1fprintf (stderr, "%s: invalid load address %s\n",@@ -146,7 +146,7 @@ int main(int argc, char **argv)case 'e' :if (--argc <=0 )usage (); - params.ep =strtoul (*++argv, &ptr,16 ); + params.ep =strtoull (*++argv, &ptr,16 );Ditto...
Also please note that your patch triggers a number of checkpatch warnings and an error, especially:
WARNING: space prohibited between function name and open parenthesis '(' #108: FILE:
tools/mkimage.c:132:
+ params.addr = strtoull (*++argv, &ptr, 16);WARNING: space prohibited between function name and open parenthesis '(' #117: FILE:
tools/mkimage.c:149:
+ params.ep = strtoull (*++argv, &ptr, 16);ERROR: Missing Signed-off-by: line(s)
Best regards,
Wolfgang Denk