diff mbox

[2/9] spl: fit: add support for post-processing of images

Message ID 1466480052-25004-3-git-send-email-dannenberg@ti.com
State New
Headers show

Commit Message

Andreas Dannenberg June 21, 2016, 3:34 a.m. UTC
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 desitination. This might be
desired to do things like strip headers or footers attached to the
images before they were packeaged into the FIT, or to perform operations
such as decryption or authentication.

Signed-off-by: Daniel Allred <d-allred@ti.com>

Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>

---
 common/spl/spl_fit.c | 21 ++++++++++++++++-----
 include/image.h      | 15 +++++++++++++++
 2 files changed, 31 insertions(+), 5 deletions(-)

-- 
2.6.4

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

Comments

Masahiro Yamada June 23, 2016, 2:38 a.m. UTC | #1
2016-06-21 12:34 GMT+09:00 Andreas Dannenberg <dannenberg@ti.com>:
> 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 desitination. This might be

> desired to do things like strip headers or footers attached to the

> images before they were packeaged into the FIT, or to perform operations

> such as decryption or authentication.

>

> Signed-off-by: Daniel Allred <d-allred@ti.com>

> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>

> ---


Typos?

desitination -> destination
packeaged    -> packaged


-- 
Best Regards
Masahiro Yamada
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Andreas Dannenberg June 23, 2016, 1:25 p.m. UTC | #2
On Thu, Jun 23, 2016 at 11:38:30AM +0900, Masahiro Yamada wrote:
> 2016-06-21 12:34 GMT+09:00 Andreas Dannenberg <dannenberg@ti.com>:

> > 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 desitination. This might be

> > desired to do things like strip headers or footers attached to the

> > images before they were packeaged into the FIT, or to perform operations

> > such as decryption or authentication.

> >

> > Signed-off-by: Daniel Allred <d-allred@ti.com>

> > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>

> > ---

> 

> Typos?

> 

> desitination -> destination

> packeaged    -> packaged

> 


Good catch! Missed running the spell checker here... Will fix.

Regards,

--
Andreas Dannenberg
Texas Instruments Inc
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Andreas Dannenberg June 23, 2016, 2:19 p.m. UTC | #3
On Thu, Jun 23, 2016 at 07:57:13AM -0600, Simon Glass wrote:
> Hi Andreas,

> 

> On 20 June 2016 at 21:34, 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 desitination. This might be

> > desired to do things like strip headers or footers attached to the

> > images before they were packeaged into the FIT, or to perform operations

> > such as decryption or authentication.

> >

> > Signed-off-by: Daniel Allred <d-allred@ti.com>

> > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>

> > ---

> >  common/spl/spl_fit.c | 21 ++++++++++++++++-----

> >  include/image.h      | 15 +++++++++++++++

> >  2 files changed, 31 insertions(+), 5 deletions(-)

> >

> > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c

> > index 9874708..ecbcb97 100644

> > --- a/common/spl/spl_fit.c

> > +++ b/common/spl/spl_fit.c

> > @@ -121,6 +121,10 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,

> >         return (data_size + info->bl_len - 1) / info->bl_len;

> >  }

> >

> > +void __weak board_fit_image_post_process(void **p_src, size_t *p_size)

> > +{

> > +}

> 

> Can you make this a Kconfig option instead of a weak function? It's

> hard enough to trace what a particular board is doing...


Hi Simon,
I understand your point and we did look into doing it but could not come
up with a good way (certainly I don't want to clutter this global file
with any TI stuff) but maybe we were over-thinking it :)

So (roughly) how would you like to see this Kconfig option to be
implemented? I'll be happy to change the implementation if given a
little direction.

Thanks,
Andreas


> 

> > +

> >  int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)

> >  {

> >         int sectors;

> > @@ -132,7 +136,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 +210,11 @@ 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);

> > +

> > +       board_fit_image_post_process((void **)&src, (size_t *)&data_size);

> > +

> > +       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 +243,12 @@ 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;

> > +

> > +       board_fit_image_post_process((void **)&src, (size_t *)&fdt_len);

> > +

> > +       memcpy(dst, src, fdt_len);

> >

> >         return 0;

> >  }

> > diff --git a/include/image.h b/include/image.h

> > index a8f6bd1..9536874 100644

> > --- a/include/image.h

> > +++ b/include/image.h

> > @@ -1172,4 +1172,19 @@ ulong android_image_get_kload(const struct andr_img_hdr *hdr);

> >   */

> >  int board_fit_config_name_match(const char *name);

> >

> > +/**

> > + * 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 /* __IMAGE_H__ */

> > --

> > 2.6.4

> >

> 

> Regards,

> Simon

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Andreas Dannenberg June 23, 2016, 3 p.m. UTC | #4
On Thu, Jun 23, 2016 at 08:45:54AM -0600, Simon Glass wrote:
> Hi Andreas,

> 

> On 23 June 2016 at 08:19, Andreas Dannenberg <dannenberg@ti.com> wrote:

> > On Thu, Jun 23, 2016 at 07:57:13AM -0600, Simon Glass wrote:

> >> Hi Andreas,

> >>

> >> On 20 June 2016 at 21:34, 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 desitination. This might be

> >> > desired to do things like strip headers or footers attached to the

> >> > images before they were packeaged into the FIT, or to perform operations

> >> > such as decryption or authentication.

> >> >

> >> > Signed-off-by: Daniel Allred <d-allred@ti.com>

> >> > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com>

> >> > ---

> >> >  common/spl/spl_fit.c | 21 ++++++++++++++++-----

> >> >  include/image.h      | 15 +++++++++++++++

> >> >  2 files changed, 31 insertions(+), 5 deletions(-)

> >> >

> >> > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c

> >> > index 9874708..ecbcb97 100644

> >> > --- a/common/spl/spl_fit.c

> >> > +++ b/common/spl/spl_fit.c

> >> > @@ -121,6 +121,10 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,

> >> >         return (data_size + info->bl_len - 1) / info->bl_len;

> >> >  }

> >> >

> >> > +void __weak board_fit_image_post_process(void **p_src, size_t *p_size)

> >> > +{

> >> > +}

> >>

> >> Can you make this a Kconfig option instead of a weak function? It's

> >> hard enough to trace what a particular board is doing...

> >

> > Hi Simon,

> > I understand your point and we did look into doing it but could not come

> > up with a good way (certainly I don't want to clutter this global file

> > with any TI stuff) but maybe we were over-thinking it :)

> >

> > So (roughly) how would you like to see this Kconfig option to be

> > implemented? I'll be happy to change the implementation if given a

> > little direction.

> 

> Add  Kconfig like FIT_IMAGE_POST_PROCESS

> 

> Then in the code:

> 

> if (IS_ENABLED(FIT_IMAGE_POST_PROCESS))

>    board_fit_image_post_process(...)

> 

> (or use #ifdef)

> 

> and enable it for your board.

> 

> You are already cluttering the code with TI stuff, but at least this

> way it is clear whether a particular board is using this feature.

> Perhaps others will find it useful too.


Simon,
Yes that was the idea, to allow other use cases here as well. And thanks
for the suggestion, that's actually simpler than what we had thought of
(while also helping the re-use aspect). I'm also going to make this a
SPL_FIT_IMAGE_POST_PROCESS to narrow the scope a little and to follow a
similar scheme like the other SPL FIT related options.

Regards,
Andreas

> With enough weak functions we would reach a point where no one can

> figure out what is happened without linking the code and looking at

> the link map!

> 

> Regards,

> Simon

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

Patch

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 9874708..ecbcb97 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -121,6 +121,10 @@  static int get_aligned_image_size(struct spl_load_info *info, int data_size,
 	return (data_size + info->bl_len - 1) / info->bl_len;
 }
 
+void __weak board_fit_image_post_process(void **p_src, size_t *p_size)
+{
+}
+
 int spl_load_simple_fit(struct spl_load_info *info, ulong sector, void *fit)
 {
 	int sectors;
@@ -132,7 +136,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 +210,11 @@  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);
+
+	board_fit_image_post_process((void **)&src, (size_t *)&data_size);
+
+	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 +243,12 @@  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;
+
+	board_fit_image_post_process((void **)&src, (size_t *)&fdt_len);
+
+	memcpy(dst, src, fdt_len);
 
 	return 0;
 }
diff --git a/include/image.h b/include/image.h
index a8f6bd1..9536874 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1172,4 +1172,19 @@  ulong android_image_get_kload(const struct andr_img_hdr *hdr);
  */
 int board_fit_config_name_match(const char *name);
 
+/**
+ * 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	/* __IMAGE_H__ */