[U-Boot] [PATCH 2/6] image: Implement a function to load Android Images.

[Simon Glass]

Hi Alex,

On 2 April 2017 at 02:49, Alex Deymo <deymo at google.com> wrote:
> This patch implements the logic needed to load an Android boot image
> from storage, since the size and kernel address in Android images is
> defined in its header.
>
> Signed-off-by: Alex Deymo <deymo at google.com>
> ---
>  common/image-android.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/image.h        | 19 +++++++++++++++++++
>  2 files changed, 70 insertions(+)
>
> diff --git a/common/image-android.c b/common/image-android.c
> index c668407817..f040f5b400 100644
> --- a/common/image-android.c
> +++ b/common/image-android.c
> @@ -8,6 +8,7 @@
>  #include <image.h>
>  #include <android_image.h>
>  #include <malloc.h>
> +#include <mapmem.h>
>  #include <errno.h>
>
>  #define ANDROID_IMAGE_DEFAULT_KERNEL_ADDR      0x10008000
> @@ -146,6 +147,56 @@ int android_image_get_ramdisk(const struct andr_img_hdr *hdr,
>         return 0;
>  }
>
> +long android_image_load(struct blk_desc *dev_desc,

Can you pass a udevice * instead of dev_desc?

> +                       const disk_partition_t *part_info,
> +                       unsigned long load_address,
> +                       unsigned long max_size) {
> +       void *buf;
> +       long blk_cnt, blk_read = 0;
> +
> +       if (max_size < part_info->blksz)
> +               return -1;
> +
> +       /* We don't know the size of the Android image before reading the header

Comment style:

/*
 * We don't know ...
 * ...
 */

> +        * so we don't limit the size of the mapped memory.
> +        */
> +       buf = map_sysmem(load_address, 0 /* size */);

This is fine, or you could provide a guess here, since there really
isn't any mapping going on. You could use max_size, for example.

> +
> +       /* Read the Android header first and then read the rest. */
> +       if (blk_dread(dev_desc, part_info->start, 1, buf) != 1)
> +               blk_read = -1;

This should be blk_read(). Unfortunately that function is not defined.
It should call the device's read() method. Can you add a patch to
define this function for read/write/erase/select_hwpart? See pch.h for
an example of how this is done in the header file.

That way you can get a proper error number.
> +
> +       if (!blk_read && android_image_check_header(buf) != 0) {
> +               printf("** Invalid Android Image header **\n");
> +               blk_read = -1;

-EINVAL perhaps?

> +       }
> +       if (!blk_read) {
> +               blk_cnt = (android_image_get_end(buf) - (ulong)buf +
> +                          part_info->blksz - 1) / part_info->blksz;
> +               if (blk_cnt * part_info->blksz > max_size) {
> +                       debug("Android Image too big (%lu bytes, max %lu)\n",
> +                             android_image_get_end(buf) - (ulong)buf,
> +                             max_size);
> +                       blk_read = -1;
> +               } else {
> +                       debug("Loading Android Image (%lu blocks) to 0x%lx... ",
> +                             blk_cnt, load_address);
> +                       blk_read = blk_dread(dev_desc, part_info->start,
> +                                            blk_cnt, buf);
> +               }
> +       }
> +
> +       unmap_sysmem(buf);
> +       if (blk_read < 0)
> +               return blk_read;
> +
> +       debug("%lu blocks read: %s\n",
> +             blk_read, (blk_read == blk_cnt) ? "OK" : "ERROR");
> +       if (blk_read != blk_cnt)
> +               return -1;
> +       return blk_read;
> +}
> +
>  #if !defined(CONFIG_SPL_BUILD)
>  /**
>   * android_print_contents - prints out the contents of the Android format image
> diff --git a/include/image.h b/include/image.h
> index 2372518960..de448a9a01 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -1241,6 +1241,25 @@ ulong android_image_get_end(const struct andr_img_hdr *hdr);
>  ulong android_image_get_kload(const struct andr_img_hdr *hdr);
>  void android_print_contents(const struct andr_img_hdr *hdr);
>
> +/** android_image_load - Load an Android Image from storage.
> + *
> + * Load an Android Image based on the header size in the storage. Return the
> + * number of bytes read from storage, which could be bigger than the actual
> + * Android Image as described in the header size. In case of error reading the
> + * image or if the image size needed to be read from disk is bigger than the
> + * the passed |max_size| a negative number is returned.

We normally do @max_size in U-Boot.

> + *
> + * @dev_desc:          The device where to read the image from
> + * @part_info:         The partition in |dev_desc| where to read the image from
> + * @load_address:      The address where the image will be loaded
> + * @max_size:          The maximum loaded size, in bytes
> + * @return the number of bytes read or a negative number in case of error.

Is it bytes or blocks?

> + */
> +long android_image_load(struct blk_desc *dev_desc,
> +                       const disk_partition_t *part_info,
> +                       unsigned long load_address,
> +                       unsigned long max_size);
> +
>  #endif /* CONFIG_ANDROID_BOOT_IMAGE */
>
>  /**
> --
> 2.12.2.564.g063fe858b8-goog
>

Regards,
Simon