diff mbox series

[1/8] spl: fit: Drop 'length' argument to board_spl_fit_post_load()

Message ID 20201216000944.2832585-2-mr.nuke.me@gmail.com
State Superseded
Delegated to: Tom Rini
Headers show
Series spl: fit: Play nicely with OP-TEE and Linux | expand

Commit Message

Alex G. Dec. 16, 2020, 12:09 a.m. UTC
The size is derived from the FIT image itself. Any alignment
requirements are machine-specific and known by the board code. Thus
the total length can be derived from the FIT image and knowledge of
the platform. The 'length' argument is redundant. Remove it.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 arch/arm/mach-imx/spl.c | 5 +++--
 common/spl/spl_fit.c    | 4 ++--
 include/spl.h           | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

Comments

Peng Fan Dec. 16, 2020, 7:13 a.m. UTC | #1
> Subject: [PATCH 1/8] spl: fit: Drop 'length' argument to
> board_spl_fit_post_load()
> 
> The size is derived from the FIT image itself. Any alignment requirements are
> machine-specific and known by the board code. Thus the total length can be
> derived from the FIT image and knowledge of the platform. The 'length'
> argument is redundant. Remove it.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  arch/arm/mach-imx/spl.c | 5 +++--
>  common/spl/spl_fit.c    | 4 ++--
>  include/spl.h           | 2 +-
>  3 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c index
> aa2686bb92..11255798d3 100644
> --- a/arch/arm/mach-imx/spl.c
> +++ b/arch/arm/mach-imx/spl.c
> @@ -18,6 +18,7 @@
>  #include <asm/mach-imx/hab.h>
>  #include <asm/mach-imx/boot_mode.h>
>  #include <g_dnl.h>
> +#include <linux/libfdt.h>
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> @@ -318,9 +319,9 @@ ulong board_spl_fit_size_align(ulong size)
>  	return size;
>  }
> 
> -void board_spl_fit_post_load(ulong load_addr, size_t length)
> +void board_spl_fit_post_load(const void *fit)
>  {
> -	u32 offset = length - CONFIG_CSF_SIZE;
> +	u32 offset = ALIGN(fdt_totalsize(fit), 0x1000);
> 
>  	if (imx_hab_authenticate_image(load_addr,
>  				       offset + IVT_SIZE + CSF_PAD_SIZE, diff --git
> a/common/spl/spl_fit.c b/common/spl/spl_fit.c index
> 795e2922ce..1b4a7f6b15 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define CONFIG_SYS_BOOTM_LEN	(64 << 20)
>  #endif
> 
> -__weak void board_spl_fit_post_load(ulong load_addr, size_t length)
> +__weak void board_spl_fit_post_load(const void *fit)
>  {
>  }
> 
> @@ -722,7 +722,7 @@ int spl_load_simple_fit(struct spl_image_info
> *spl_image,
>  	spl_image->flags |= SPL_FIT_FOUND;
> 
>  #ifdef CONFIG_IMX_HAB
> -	board_spl_fit_post_load((ulong)fit, size);
> +	board_spl_fit_post_load(fit);
>  #endif
> 
>  	return 0;
> diff --git a/include/spl.h b/include/spl.h index 374a295fa3..f63829a99e
> 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -632,7 +632,7 @@ int board_return_to_bootrom(struct spl_image_info
> *spl_image,
>   * board_spl_fit_post_load - allow process images after loading finished
>   *
>   */
> -void board_spl_fit_post_load(ulong load_addr, size_t length);
> +void board_spl_fit_post_load(const void *fit);
> 
>  /**
>   * board_spl_fit_size_align - specific size align before processing payload
> --

Looks good to me! 

Reviewed-by: Peng Fan <peng.fan@nxp.com>

BTW: has this been tested?

Thanks,
Peng.
Alex G. Dec. 16, 2020, 2:32 p.m. UTC | #2
On 12/16/20 1:13 AM, Peng Fan wrote:
>> Subject: [PATCH 1/8] spl: fit: Drop 'length' argument to
>> board_spl_fit_post_load()
>>
>> The size is derived from the FIT image itself. Any alignment requirements are
>> machine-specific and known by the board code. Thus the total length can be
>> derived from the FIT image and knowledge of the platform. The 'length'
>> argument is redundant. Remove it.
>>
>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>> ---
>>   arch/arm/mach-imx/spl.c | 5 +++--
>>   common/spl/spl_fit.c    | 4 ++--
>>   include/spl.h           | 2 +-
>>   3 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c index
>> aa2686bb92..11255798d3 100644
>> --- a/arch/arm/mach-imx/spl.c
>> +++ b/arch/arm/mach-imx/spl.c
>> @@ -18,6 +18,7 @@
>>   #include <asm/mach-imx/hab.h>
>>   #include <asm/mach-imx/boot_mode.h>
>>   #include <g_dnl.h>
>> +#include <linux/libfdt.h>
>>
>>   DECLARE_GLOBAL_DATA_PTR;
>>
>> @@ -318,9 +319,9 @@ ulong board_spl_fit_size_align(ulong size)
>>   	return size;
>>   }
>>
>> -void board_spl_fit_post_load(ulong load_addr, size_t length)
>> +void board_spl_fit_post_load(const void *fit)
>>   {
>> -	u32 offset = length - CONFIG_CSF_SIZE;
>> +	u32 offset = ALIGN(fdt_totalsize(fit), 0x1000);
>>
>>   	if (imx_hab_authenticate_image(load_addr,
>>   				       offset + IVT_SIZE + CSF_PAD_SIZE, diff --git
>> a/common/spl/spl_fit.c b/common/spl/spl_fit.c index
>> 795e2922ce..1b4a7f6b15 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>   #define CONFIG_SYS_BOOTM_LEN	(64 << 20)
>>   #endif
>>
>> -__weak void board_spl_fit_post_load(ulong load_addr, size_t length)
>> +__weak void board_spl_fit_post_load(const void *fit)
>>   {
>>   }
>>
>> @@ -722,7 +722,7 @@ int spl_load_simple_fit(struct spl_image_info
>> *spl_image,
>>   	spl_image->flags |= SPL_FIT_FOUND;
>>
>>   #ifdef CONFIG_IMX_HAB
>> -	board_spl_fit_post_load((ulong)fit, size);
>> +	board_spl_fit_post_load(fit);
>>   #endif
>>
>>   	return 0;
>> diff --git a/include/spl.h b/include/spl.h index 374a295fa3..f63829a99e
>> 100644
>> --- a/include/spl.h
>> +++ b/include/spl.h
>> @@ -632,7 +632,7 @@ int board_return_to_bootrom(struct spl_image_info
>> *spl_image,
>>    * board_spl_fit_post_load - allow process images after loading finished
>>    *
>>    */
>> -void board_spl_fit_post_load(ulong load_addr, size_t length);
>> +void board_spl_fit_post_load(const void *fit);
>>
>>   /**
>>    * board_spl_fit_size_align - specific size align before processing payload
>> --
> 
> Looks good to me!
> 
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> 
> BTW: has this been tested?

Hi Peng,

It would be great to get some independent test results, since I don't 
have any IMX hardware.

Alex

> Thanks,
> Peng.
>
Simon Glass Dec. 19, 2020, 2:28 a.m. UTC | #3
Hi Alexandru,

On Tue, 15 Dec 2020 at 17:09, Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:
>
> The size is derived from the FIT image itself. Any alignment
> requirements are machine-specific and known by the board code. Thus
> the total length can be derived from the FIT image and knowledge of
> the platform. The 'length' argument is redundant. Remove it.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  arch/arm/mach-imx/spl.c | 5 +++--
>  common/spl/spl_fit.c    | 4 ++--
>  include/spl.h           | 2 +-
>  3 files changed, 6 insertions(+), 5 deletions(-)

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

>
> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
> index aa2686bb92..11255798d3 100644
> --- a/arch/arm/mach-imx/spl.c
> +++ b/arch/arm/mach-imx/spl.c
> @@ -18,6 +18,7 @@
>  #include <asm/mach-imx/hab.h>
>  #include <asm/mach-imx/boot_mode.h>
>  #include <g_dnl.h>
> +#include <linux/libfdt.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -318,9 +319,9 @@ ulong board_spl_fit_size_align(ulong size)
>         return size;
>  }
>
> -void board_spl_fit_post_load(ulong load_addr, size_t length)
> +void board_spl_fit_post_load(const void *fit)
>  {
> -       u32 offset = length - CONFIG_CSF_SIZE;
> +       u32 offset = ALIGN(fdt_totalsize(fit), 0x1000);
>
>         if (imx_hab_authenticate_image(load_addr,
>                                        offset + IVT_SIZE + CSF_PAD_SIZE,
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 795e2922ce..1b4a7f6b15 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define CONFIG_SYS_BOOTM_LEN   (64 << 20)
>  #endif
>
> -__weak void board_spl_fit_post_load(ulong load_addr, size_t length)
> +__weak void board_spl_fit_post_load(const void *fit)
>  {
>  }
>
> @@ -722,7 +722,7 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
>         spl_image->flags |= SPL_FIT_FOUND;
>
>  #ifdef CONFIG_IMX_HAB
> -       board_spl_fit_post_load((ulong)fit, size);
> +       board_spl_fit_post_load(fit);
>  #endif
>
>         return 0;
> diff --git a/include/spl.h b/include/spl.h
> index 374a295fa3..f63829a99e 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -632,7 +632,7 @@ int board_return_to_bootrom(struct spl_image_info *spl_image,
>   * board_spl_fit_post_load - allow process images after loading finished
>   *
>   */
> -void board_spl_fit_post_load(ulong load_addr, size_t length);
> +void board_spl_fit_post_load(const void *fit);

Can you please add a comment for this?


>
>  /**
>   * board_spl_fit_size_align - specific size align before processing payload
> --
> 2.26.2
>
diff mbox series

Patch

diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index aa2686bb92..11255798d3 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -18,6 +18,7 @@ 
 #include <asm/mach-imx/hab.h>
 #include <asm/mach-imx/boot_mode.h>
 #include <g_dnl.h>
+#include <linux/libfdt.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -318,9 +319,9 @@  ulong board_spl_fit_size_align(ulong size)
 	return size;
 }
 
-void board_spl_fit_post_load(ulong load_addr, size_t length)
+void board_spl_fit_post_load(const void *fit)
 {
-	u32 offset = length - CONFIG_CSF_SIZE;
+	u32 offset = ALIGN(fdt_totalsize(fit), 0x1000);
 
 	if (imx_hab_authenticate_image(load_addr,
 				       offset + IVT_SIZE + CSF_PAD_SIZE,
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 795e2922ce..1b4a7f6b15 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -26,7 +26,7 @@  DECLARE_GLOBAL_DATA_PTR;
 #define CONFIG_SYS_BOOTM_LEN	(64 << 20)
 #endif
 
-__weak void board_spl_fit_post_load(ulong load_addr, size_t length)
+__weak void board_spl_fit_post_load(const void *fit)
 {
 }
 
@@ -722,7 +722,7 @@  int spl_load_simple_fit(struct spl_image_info *spl_image,
 	spl_image->flags |= SPL_FIT_FOUND;
 
 #ifdef CONFIG_IMX_HAB
-	board_spl_fit_post_load((ulong)fit, size);
+	board_spl_fit_post_load(fit);
 #endif
 
 	return 0;
diff --git a/include/spl.h b/include/spl.h
index 374a295fa3..f63829a99e 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -632,7 +632,7 @@  int board_return_to_bootrom(struct spl_image_info *spl_image,
  * board_spl_fit_post_load - allow process images after loading finished
  *
  */
-void board_spl_fit_post_load(ulong load_addr, size_t length);
+void board_spl_fit_post_load(const void *fit);
 
 /**
  * board_spl_fit_size_align - specific size align before processing payload