[U-Boot] [PATCH] Gracefully handle 64-bit signed-extended 32-bit Load addresses

[William Cohen]

To follow the MIPS 32-bit and 64-bit memory map conventions (*) recent
MIPS Linux kernels are using a 64-bit sign extended value
(0x8001) for the 32-bit load address (0x8001) of the
Creator CI20 board kernel.  When this 64-bit argument was passed to
mkimage running on a 32-bit machine such as the Creator CI20 board the
load address was incorrectly formed from the upper 32-bit sign-extend
bits (0x) by the strtoul instead of from the lower 32-bits
(0x8001).  The mkimage should be able to tolerate the longer
sign-extended 64-bit version of the 32-bit arguments with the use of
strtoull.  Use of the strtoll in place of the strtol in mkimage.c
resolves the issue of self hosted kernel builds for the Creator CI20
board (+) and (++).

(*) 
http://techpubs.sgi.com/library/dynaweb_docs/0620/SGI_Developer/books/DevDriver_PG/sgi_html/ch01.html
(+) https://github.com/MIPS/CI20_linux/issues/23
(++) https://github.com/MIPS/CI20_linux/issues/22

Signed-off-by: William Cohen <wco...@redhat.com>
---
 tools/mkimage.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index 8f8b6df..facebcd 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -129,7 +129,7 @@ int main(int argc, char **argv)
case 'a':
if (--argc <= 0)
usage ();
-   params.addr = strtoul (*++argv, , 16);
+   params.addr = strtoull(*++argv, , 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, , 16);
+   params.ep = strtoull(*++argv, , 16);
if (*ptr) {
fprintf (stderr,
"%s: invalid entry point %s\n",
-- 
2.5.0

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] Gracefully handle 64-bit Load Address and Entry Point Address mkimage parameters

[William Cohen]

On 02/15/2016 06:34 AM, Wolfgang Denk wrote:
> Dear William,
> 
> In message <1455506732-22307-1-git-send-email-wco...@redhat.com> you wrote:
>>
>> Recent MIPS Linux kernels are using a 64-bit value for the load
>> address (0x8001) 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, 0x
>> by 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.

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
> 
> 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.
> 
> 
>> -params.addr = strtoul (*++argv, , 16);
>> +params.addr = strtoull (*++argv, , 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?
> 
>>  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, , 16);
>> +params.ep = strtoull (*++argv, , 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, , 16);
> 
> WARNING: space prohibited between function name and open parenthesis '('
> #117: FILE: tools/mkimage.c:149:
> +   params.ep = strtoull (*++argv, , 16);
> 
> ERROR: Missing Signed-off-by: line(s)
> 
> 
> Best regards,
> 
> Wolfgang Denk
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] Gracefully handle 64-bit Load Address and Entry Point Address mkimage parameters

[William Cohen]

From: no one 

Recent MIPS Linux kernels are using a 64-bit value for the load
address (0x8001) 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, 0x
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(-)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index 8f8b6df..b6c2958 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -129,7 +129,7 @@ int main(int argc, char **argv)
case 'a':
if (--argc <= 0)
usage ();
-   params.addr = strtoul (*++argv, , 16);
+   params.addr = strtoull (*++argv, , 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, , 16);
+   params.ep = strtoull (*++argv, , 16);
if (*ptr) {
fprintf (stderr,
"%s: invalid entry point %s\n",
-- 
2.5.0

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot