[U-Boot] [PATCH v2] common: Fix load and entry addresses in FIT image

[Simon Glass]

Hi York,

On 4 September 2015 at 07:59, York Sun <yorksun at freescale.com> wrote:
>
>
> On 09/03/2015 10:55 PM, Simon Glass wrote:
>> Hi York,
>>
>> On 3 September 2015 at 09:07, York Sun <yorksun at freescale.com> wrote:
>>> FIT image supports more than 32 bits in addresses by using #address-cell
>>> field. However the address length is not handled when parsing FIT images.
>>> Beside, the variable used to host address has "ulong" type. It is OK for
>>> the target, but not always enough for host tools such as mkimage. This
>>> patch replaces "ulong" with "phys_addr_t" to make sure the address is
>>> correct for both the target and the host.
>>>
>>> Signed-off-by: York Sun <yorksun at freescale.com>
>>>
>>> ---
>>>
>>> Changes in v2:
>>>   Make a common function for both load and entry addresses.
>>>   Simplify calculation of addresses in a similar way as fdtdec_get_number()
>>>   fdtdec_get_number() is not used, or too many files need to be included
>>>     and/or twisted for host tool
>>>   Continue to use %08llx for print format for load and entry addresses
>>>     because %pa does not always work for host tool (mkimage)
>>>
>>>  common/bootm.c     |   13 +++++----
>>>  common/image-fit.c |   79 ++++++++++++++++++++++++++++------------------------
>>>  include/bootm.h    |    6 ++--
>>>  include/image.h    |   12 +++++---
>>>  4 files changed, 61 insertions(+), 49 deletions(-)
>>>
>>> diff --git a/common/bootm.c b/common/bootm.c
>>> index 667c934..0672e73 100644
>>> --- a/common/bootm.c
>>> +++ b/common/bootm.c
>>> @@ -325,9 +325,9 @@ static int handle_decomp_error(int comp_type, size_t uncomp_size,
>>>         return BOOTM_ERR_RESET;
>>>  }
>>>
>>> -int bootm_decomp_image(int comp, ulong load, ulong image_start, int type,
>>> -                      void *load_buf, void *image_buf, ulong image_len,
>>> -                      uint unc_len, ulong *load_end)
>>> +int bootm_decomp_image(int comp, phys_addr_t load, phys_addr_t image_start,
>>> +                      int type, void *load_buf, void *image_buf,
>>> +                      ulong image_len, uint unc_len, ulong *load_end)
>>>  {
>>>         int ret = 0;
>>>
>>> @@ -883,7 +883,8 @@ void memmove_wd(void *to, void *from, size_t len, ulong chunksz)
>>>  static int bootm_host_load_image(const void *fit, int req_image_type)
>>>  {
>>>         const char *fit_uname_config = NULL;
>>> -       ulong data, len;
>>> +       phys_addr_t *data = NULL;
>>> +       ulong len;
>>>         bootm_headers_t images;
>>>         int noffset;
>>>         ulong load_end;
>>> @@ -897,7 +898,7 @@ static int bootm_host_load_image(const void *fit, int req_image_type)
>>>         noffset = fit_image_load(&images, (ulong)fit,
>>>                 NULL, &fit_uname_config,
>>>                 IH_ARCH_DEFAULT, req_image_type, -1,
>>> -               FIT_LOAD_IGNORED, &data, &len);
>>> +               FIT_LOAD_IGNORED, data, &len);
>>>         if (noffset < 0)
>>>                 return noffset;
>>>         if (fit_image_get_type(fit, noffset, &image_type)) {
>>> @@ -912,7 +913,7 @@ static int bootm_host_load_image(const void *fit, int req_image_type)
>>>
>>>         /* Allow the image to expand by a factor of 4, should be safe */
>>>         load_buf = malloc((1 << 20) + len * 4);
>>> -       ret = bootm_decomp_image(imape_comp, 0, data, image_type, load_buf,
>>> +       ret = bootm_decomp_image(imape_comp, 0, *data, image_type, load_buf,
>>>                                  (void *)data, len, CONFIG_SYS_BOOTM_LEN,
>>>                                  &load_end);
>>>         free(load_buf);
>>> diff --git a/common/image-fit.c b/common/image-fit.c
>>> index 28f7aa8..7a0c9a9 100644
>>> --- a/common/image-fit.c
>>> +++ b/common/image-fit.c
>>> @@ -358,7 +358,7 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
>>>         char *desc;
>>>         uint8_t type, arch, os, comp;
>>>         size_t size;
>>> -       ulong load, entry;
>>> +       phys_addr_t load, entry;
>>>         const void *data;
>>>         int noffset;
>>>         int ndepth;
>>> @@ -428,17 +428,17 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
>>>                 if (ret)
>>>                         printf("unavailable\n");
>>>                 else
>>> -                       printf("0x%08lx\n", load);
>>> +                       printf("0x%08llx\n", (uint64_t)load);
>>>         }
>>>
>>>         if ((type == IH_TYPE_KERNEL) || (type == IH_TYPE_STANDALONE) ||
>>>             (type == IH_TYPE_RAMDISK)) {
>>> -               fit_image_get_entry(fit, image_noffset, &entry);
>>> +               ret = fit_image_get_entry(fit, image_noffset, &entry);
>>>                 printf("%s  Entry Point:  ", p);
>>>                 if (ret)
>>>                         printf("unavailable\n");
>>>                 else
>>> -                       printf("0x%08lx\n", entry);
>>> +                       printf("0x%08llx\n", (uint64_t)entry);
>>
>> Yes of course %pa does not work on the host - I didn't think of that.
>>
>> I'm still not thrilled with everything being promoted to 64-bit. Do
>> you think using a #define in inttypes.h or similar might work, similar
>> to how LBAF works in ide.h?
>>
>> #if BITS_PER_LONG == 64
>> #define PRIpa "%08l"
>> #else
>> #define PRIpa "%08l"
>> #endif
>>
>> The odd thing is that they are both the same for ARM (unsigned long).
>> What arch are you using?
>
> This one works for me
>
> #if BITS_PER_LONG == 64
> #define PRIpa  "lx"
> #else
> #define PRIpa  "llx"
> #endif
>
> The trick here is host tool. I was testing on armv8.

I'm hoping that we can use a 32-bit parameter for 32-bit machines and
64-bit for 64-bit machines (and host).

Regards,
Simon