[3/4] imx: hab: Specify IVT padding size

Message ID 1520616949-11879-4-git-send-email-bryan.odonoghue@linaro.org
State New
Headers show
Series
  • imx: hab: Add helper functions for scripted HAB auth
Related show

Commit Message

Bryan O'Donoghue March 9, 2018, 5:35 p.m.
This patch adds IVT_PAD_SIZE at 0xC00. The IVT header is padded to this
size. Defining the size explicitly makes it possible to use the define to
locate the start/end of an IVT header without using magic numbers in code.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Utkarsh Gupta <utkarsh.gupta@nxp.com>
Cc: Breno Lima <breno.lima@nxp.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
---
 arch/arm/include/asm/mach-imx/hab.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Breno Matheus Lima March 15, 2018, 4:54 p.m. | #1
Hi Bryan,

2018-03-09 14:35 GMT-03:00 Bryan O'Donoghue <bryan.odonoghue@linaro.org>:
> This patch adds IVT_PAD_SIZE at 0xC00. The IVT header is padded to this
> size. Defining the size explicitly makes it possible to use the define to
> locate the start/end of an IVT header without using magic numbers in code.
>

As far as I know the 0xC00 pad size is only mandatory in U-Boot/SPL
images, for instance in some U-Boot images the first 0xC00 should
include IVT + Boot data + DCD table + padding to 0xC00.
Are you using the IVT_PAD_SIZE in your current code? Or this macro
will be used in a next series?

Thanks,
Breno Lima
Bryan O'Donoghue March 17, 2018, 11:06 a.m. | #2
On 15/03/18 16:54, Breno Matheus Lima wrote:
> Hi Bryan,
> 
> 2018-03-09 14:35 GMT-03:00 Bryan O'Donoghue <bryan.odonoghue@linaro.org>:
>> This patch adds IVT_PAD_SIZE at 0xC00. The IVT header is padded to this
>> size. Defining the size explicitly makes it possible to use the define to
>> locate the start/end of an IVT header without using magic numbers in code.
>>
> 
> As far as I know the 0xC00 pad size is only mandatory in U-Boot/SPL
> images, for instance in some U-Boot images the first 0xC00 should
> include IVT + Boot data + DCD table + padding to 0xC00.
> Are you using the IVT_PAD_SIZE in your current code? Or this macro
> will be used in a next series?
> 
> Thanks,
> Breno Lima
> 

All of my images - kernel, u-boot, optee, boot.scr are signed in the 
same first-stage format with this padding present - for simplicity.

Maybe this better named "BOOTROM_IVT_PAD_SIZE" since only the bootrom 
requires it.
Breno Matheus Lima March 19, 2018, 5:53 p.m. | #3
Hi Bryan,

2018-03-17 8:06 GMT-03:00 Bryan O'Donoghue <bryan.odonoghue@linaro.org>:
>
>
> On 15/03/18 16:54, Breno Matheus Lima wrote:
>>
>> Hi Bryan,
>>
>> 2018-03-09 14:35 GMT-03:00 Bryan O'Donoghue <bryan.odonoghue@linaro.org>:
>>>
>>> This patch adds IVT_PAD_SIZE at 0xC00. The IVT header is padded to this
>>> size. Defining the size explicitly makes it possible to use the define to
>>> locate the start/end of an IVT header without using magic numbers in
>>> code.
>>>
>>
>> As far as I know the 0xC00 pad size is only mandatory in U-Boot/SPL
>> images, for instance in some U-Boot images the first 0xC00 should
>> include IVT + Boot data + DCD table + padding to 0xC00.
>> Are you using the IVT_PAD_SIZE in your current code? Or this macro
>> will be used in a next series?
>>
>> Thanks,
>> Breno Lima
>>
>
> All of my images - kernel, u-boot, optee, boot.scr are signed in the same
> first-stage format with this padding present - for simplicity.
>
> Maybe this better named "BOOTROM_IVT_PAD_SIZE" since only the bootrom
> requires it.

As far as I know the IVT has a fixed size of 0x20, and the set of IVT
+ Boot Data + DCD cannot exceed 0xC00.

This value is calculated by the following operation in function
imximage_generate() at tools/imximage.c:
alloc_len = imximage_init_loadsize - imximage_ivt_offset;

Maybe we can rename to something like UBOOT_IMX_HDR_SIZE? We also have
to ensure this definition is being used by U-Boot code.

Thanks,
Breno Lima
Bryan O'Donoghue March 21, 2018, 4:47 a.m. | #4
On 20/03/18 01:53, Breno Matheus Lima wrote:
> Hi Bryan,
> 
> 2018-03-17 8:06 GMT-03:00 Bryan O'Donoghue <bryan.odonoghue@linaro.org>:
>>
>>
>> On 15/03/18 16:54, Breno Matheus Lima wrote:
>>>
>>> Hi Bryan,
>>>
>>> 2018-03-09 14:35 GMT-03:00 Bryan O'Donoghue <bryan.odonoghue@linaro.org>:
>>>>
>>>> This patch adds IVT_PAD_SIZE at 0xC00. The IVT header is padded to this
>>>> size. Defining the size explicitly makes it possible to use the define to
>>>> locate the start/end of an IVT header without using magic numbers in
>>>> code.
>>>>
>>>
>>> As far as I know the 0xC00 pad size is only mandatory in U-Boot/SPL
>>> images, for instance in some U-Boot images the first 0xC00 should
>>> include IVT + Boot data + DCD table + padding to 0xC00.
>>> Are you using the IVT_PAD_SIZE in your current code? Or this macro
>>> will be used in a next series?
>>>
>>> Thanks,
>>> Breno Lima
>>>
>>
>> All of my images - kernel, u-boot, optee, boot.scr are signed in the same
>> first-stage format with this padding present - for simplicity.
>>
>> Maybe this better named "BOOTROM_IVT_PAD_SIZE" since only the bootrom
>> requires it.
> 
> As far as I know the IVT has a fixed size of 0x20, and the set of IVT
> + Boot Data + DCD cannot exceed 0xC00.
> 
> This value is calculated by the following operation in function
> imximage_generate() at tools/imximage.c:
> alloc_len = imximage_init_loadsize - imximage_ivt_offset;
> 
> Maybe we can rename to something like UBOOT_IMX_HDR_SIZE? We also have
> to ensure this definition is being used by U-Boot code.
> 
> Thanks,
> Breno Lima
> 

So.

I've stupidly called this "PAD_SIZE" and we've gone off on a padding 
size discussion.

This define is supposed to represent the IVT _offset_ in that initial 
BootROM image.

Note to self: the hint is in the variable "setenv ivt_offset=some_number"

Anyway Breno - we could add padding size checks to images but, _this_ 
define is related to IVT offset so... my bad.

I'll redo this patch with a better name :(

Patch

diff --git a/arch/arm/include/asm/mach-imx/hab.h b/arch/arm/include/asm/mach-imx/hab.h
index 1bebdbe..f903e3e 100644
--- a/arch/arm/include/asm/mach-imx/hab.h
+++ b/arch/arm/include/asm/mach-imx/hab.h
@@ -205,6 +205,7 @@  bool imx_hab_is_enabled(void);
 #endif /* __ASSEMBLY__ */
 
 #define IVT_SIZE			0x20
+#define IVT_PAD_SIZE			0xC00
 #define CSF_PAD_SIZE			0x2000
 
 #endif