[U-Boot,v3,6/9] spl: fit: add support for post-processing of images

From: Daniel Allred <d-allred@ti.com>

The next stage boot loader image and the selected FDT can be post-
processed by board/platform/device-specific code, which can include
modifying the size and altering the starting source address before
copying these binary blobs to their final destination. This might be
desired to do things like strip headers or footers attached to the
images before they were packaged into the FIT, or to perform operations
such as decryption or authentication. Introduce new configuration
option CONFIG_SPL_FIT_IMAGE_POST_PROCESS to allow controlling this
feature. If enabled, a platform-specific post-process function must
be provided.

Signed-off-by: Daniel Allred <d-allred@ti.com>
Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
---
 Kconfig              | 14 ++++++++++++++
 common/spl/spl_fit.c | 21 ++++++++++++++++-----
 include/image.h      | 17 +++++++++++++++++
 3 files changed, 47 insertions(+), 5 deletions(-)

On 27 June 2016 at 07:19, Andreas Dannenberg <dannenberg@ti.com> wrote:
> From: Daniel Allred <d-allred@ti.com>
>
> The next stage boot loader image and the selected FDT can be post-
> processed by board/platform/device-specific code, which can include
> modifying the size and altering the starting source address before
> copying these binary blobs to their final destination. This might be
> desired to do things like strip headers or footers attached to the
> images before they were packaged into the FIT, or to perform operations
> such as decryption or authentication. Introduce new configuration
> option CONFIG_SPL_FIT_IMAGE_POST_PROCESS to allow controlling this
> feature. If enabled, a platform-specific post-process function must
> be provided.
>
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> ---
>  Kconfig              | 14 ++++++++++++++
>  common/spl/spl_fit.c | 21 ++++++++++++++++-----
>  include/image.h      | 17 +++++++++++++++++
>  3 files changed, 47 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Nit below in case you do a new version.

>
> diff --git a/Kconfig b/Kconfig
> index 3ceff25..2afbaaf 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -313,6 +313,20 @@ config SPL_LOAD_FIT
>           particular it can handle selecting from multiple device tree
>           and passing the correct one to U-Boot.
>
> +config SPL_FIT_IMAGE_POST_PROCESS
> +       bool "Enable post-processing of FIT artifacts after loading by the SPL"
> +       depends on SPL_LOAD_FIT && TI_SECURE_DEVICE
> +       help
> +         Allows doing any sort of manipulation to blobs after they got extracted
> +         from the U-Boot FIT image like stripping off headers or modifying the
> +         size of the blob, verification, authentication, decryption etc. in a
> +         platform or board specific way. In order to use this feature a platform
> +         or board-specific implementation of board_fit_image_post_process() must
> +         be provided. Also, anything done during this post-processing step would
> +         need to be comprehended in how the images were prepared before being
> +         injected into the FIT creation (i.e. the blobs would have been pre-
> +         processed before being added to the FIT image).
> +
>  config SYS_CLK_FREQ
>         depends on ARC || ARCH_SUNXI
>         int "CPU clock frequency"
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 9874708..069e94d 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -132,7 +132,7 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>         int data_offset, data_size;
>         int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
>         int src_sector;
> -       void *dst;
> +       void *dst, *src;
>
>         /*
>          * Figure out where the external images start. This is the base for the
> @@ -206,8 +206,13 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>                 return -EIO;
>         debug("image: dst=%p, data_offset=%x, size=%x\n", dst, data_offset,
>               data_size);
> -       memcpy(dst, dst + get_aligned_image_overhead(info, data_offset),
> -              data_size);
> +       src = dst + get_aligned_image_overhead(info, data_offset);
> +
> +#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
> +       board_fit_image_post_process((void **)&src, (size_t *)&data_size);
> +#endif
> +
> +       memcpy(dst, src, data_size);
>
>         /* Figure out which device tree the board wants to use */
>         fdt_len = spl_fit_select_fdt(fit, images, &fdt_offset);
> @@ -236,8 +241,14 @@ int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
>          */
>         debug("fdt: dst=%p, data_offset=%x, size=%x\n", dst, fdt_offset,
>               fdt_len);
> -       memcpy(load_ptr + data_size,
> -              dst + get_aligned_image_overhead(info, fdt_offset), fdt_len);
> +       src = dst + get_aligned_image_overhead(info, fdt_offset);
> +       dst = load_ptr + data_size;
> +
> +#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
> +       board_fit_image_post_process((void **)&src, (size_t *)&fdt_len);
> +#endif
> +
> +       memcpy(dst, src, fdt_len);
>
>         return 0;
>  }
> diff --git a/include/image.h b/include/image.h
> index d788c26..93d39e1 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -1173,4 +1173,21 @@ void android_print_contents(const struct andr_img_hdr *hdr);
>   */
>  int board_fit_config_name_match(const char *name);
>
> +#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
> +/**
> + * board_fit_image_post_process() - Do any post-process on FIT binary data
> + *
> + * This is used to do any sort of image manipulation, verification, decryption
> + * etc. in a platform or board specific way. Obviously, anything done here would
> + * need to be comprehended in how the images were prepared before being injected
> + * into the FIT creation (i.e. the binary blobs would have been pre-processed
> + * before being added to the FIT image).
> + *
> + * @image: pointer to the image start pointer
> + * @size: pointer to the image size
> + * @return no return value (failure should be handled internally)
> + */
> +void board_fit_image_post_process(void **p_image, size_t *p_size);
> +#endif /* CONFIG_SPL_FIT_IMAGE_POST_PROCESS */

We don't need #ifdef in header files - it just makes the code harder
to read, and we'll still get a build error (with correct line number
info) if someone uses it when they should not.

> +
>  #endif /* __IMAGE_H__ */
> --
> 2.6.4
>

Regards,
Simon

Hi Simon, please see below...

On Tue, Jun 28, 2016 at 08:28:07PM -0700, Simon Glass wrote:
> On 27 June 2016 at 07:19, Andreas Dannenberg <dannenberg@ti.com> wrote:
> > diff --git a/include/image.h b/include/image.h
> > index d788c26..93d39e1 100644
> > --- a/include/image.h
> > +++ b/include/image.h
> > @@ -1173,4 +1173,21 @@ void android_print_contents(const struct andr_img_hdr *hdr);
> >   */
> >  int board_fit_config_name_match(const char *name);
> >
> > +#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
> > +/**
> > + * board_fit_image_post_process() - Do any post-process on FIT binary data
> > + *
> > + * This is used to do any sort of image manipulation, verification, decryption
> > + * etc. in a platform or board specific way. Obviously, anything done here would
> > + * need to be comprehended in how the images were prepared before being injected
> > + * into the FIT creation (i.e. the binary blobs would have been pre-processed
> > + * before being added to the FIT image).
> > + *
> > + * @image: pointer to the image start pointer
> > + * @size: pointer to the image size
> > + * @return no return value (failure should be handled internally)
> > + */
> > +void board_fit_image_post_process(void **p_image, size_t *p_size);
> > +#endif /* CONFIG_SPL_FIT_IMAGE_POST_PROCESS */
> 
> We don't need #ifdef in header files - it just makes the code harder
> to read, and we'll still get a build error (with correct line number
> info) if someone uses it when they should not.

You are right that's technically not needed, but rather I was following
how other prototypes are defined in that header file (like the ones that
get declared when CONFIG_ANDROID_BOOT_IMAGE is defined).

For now I'm not planning to re-spin the patch series, but if there is
additional feedback I can take care of this one as well.

Thanks and Regards,

--
Andreas Dannenberg
Texas Instruments Inc

On Mon, Jun 27, 2016 at 09:19:21AM -0500, Andreas Dannenberg wrote:

> From: Daniel Allred <d-allred@ti.com>
> 
> The next stage boot loader image and the selected FDT can be post-
> processed by board/platform/device-specific code, which can include
> modifying the size and altering the starting source address before
> copying these binary blobs to their final destination. This might be
> desired to do things like strip headers or footers attached to the
> images before they were packaged into the FIT, or to perform operations
> such as decryption or authentication. Introduce new configuration
> option CONFIG_SPL_FIT_IMAGE_POST_PROCESS to allow controlling this
> feature. If enabled, a platform-specific post-process function must
> be provided.
> 
> Signed-off-by: Daniel Allred <d-allred@ti.com>
> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!