[U-Boot] [PATCH 3/4] imx6: spl: Reduce SPL limit size in case CONFIG_SECURE_BOOT is enabled

2019-08-05 Thread sbabic
> In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to
> avoid a possible HAB failure event:
> - HAB Event 1 -
> event data:
> 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00
> 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00
> 0x00 0x01 0x10 0x00
> STS = HAB_FAILURE (0x33)
> RSN = HAB_INV_ADDRESS (0x22)
> CTX = HAB_CTX_TARGET (0x33)
> ENG = HAB_ENG_ANY (0x00)
> As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT
> default") the i.MX6 SPL size limit is 68KB.
> The ROM code is copying the image size defined in boot data to its
> respective load address, in case we exceed the OCRAM free region a
> HAB invalid address failure event is generated.
> The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size
> limit based on this configuration.
> Signed-off-by: Breno Lima 
> Reviewed-by: Fabio Estevam 

Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic

-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH 3/4] imx6: spl: Reduce SPL limit size in case CONFIG_SECURE_BOOT is enabled

2019-07-18 Thread Breno Matheus Lima
In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to
avoid a possible HAB failure event:

- HAB Event 1 -
event data:
0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00
0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00
0x00 0x01 0x10 0x00
STS = HAB_FAILURE (0x33)
RSN = HAB_INV_ADDRESS (0x22)
CTX = HAB_CTX_TARGET (0x33)
ENG = HAB_ENG_ANY (0x00)

As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT
default") the i.MX6 SPL size limit is 68KB.

The ROM code is copying the image size defined in boot data to its
respective load address, in case we exceed the OCRAM free region a
HAB invalid address failure event is generated.

The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size
limit based on this configuration.

Signed-off-by: Breno Lima 
---
 tools/spl_size_limit.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c
index 98ff491867..8902e30129 100644
--- a/tools/spl_size_limit.c
+++ b/tools/spl_size_limit.c
@@ -14,6 +14,9 @@ int main(int argc, char *argv[])
 
 #ifdef CONFIG_SPL_SIZE_LIMIT
spl_size_limit = CONFIG_SPL_SIZE_LIMIT;
+#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
+   spl_size_limit -= CONFIG_CSF_SIZE;
+#endif
 #ifdef CONFIG_SPL_SIZE_LIMIT_SUBTRACT_GD
spl_size_limit -= GENERATED_GBL_DATA_SIZE;
 #endif
-- 
2.17.1

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 3/4] imx6: spl: Reduce SPL limit size in case CONFIG_SECURE_BOOT is enabled

2019-09-17 Thread Jagan Teki
Hi Breno,

On Thu, Jul 18, 2019 at 6:06 PM Breno Matheus Lima  wrote:
>
> In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to
> avoid a possible HAB failure event:
>
> - HAB Event 1 -
> event data:
> 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00
> 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00
> 0x00 0x01 0x10 0x00
> STS = HAB_FAILURE (0x33)
> RSN = HAB_INV_ADDRESS (0x22)
> CTX = HAB_CTX_TARGET (0x33)
> ENG = HAB_ENG_ANY (0x00)
>
> As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT
> default") the i.MX6 SPL size limit is 68KB.
>
> The ROM code is copying the image size defined in boot data to its
> respective load address, in case we exceed the OCRAM free region a
> HAB invalid address failure event is generated.
>
> The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size
> limit based on this configuration.
>
> Signed-off-by: Breno Lima 
> ---
>  tools/spl_size_limit.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c
> index 98ff491867..8902e30129 100644
> --- a/tools/spl_size_limit.c
> +++ b/tools/spl_size_limit.c
> @@ -14,6 +14,9 @@ int main(int argc, char *argv[])
>
>  #ifdef CONFIG_SPL_SIZE_LIMIT
> spl_size_limit = CONFIG_SPL_SIZE_LIMIT;
> +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
> +   spl_size_limit -= CONFIG_CSF_SIZE;
> +#endif

But, if the target enable HAB on SPL the size would be part of SPL
limit, isn't ?

Just now I have looked at this, since one of my board (imx6_mamoj)
fails to build.

Jagan.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 3/4] imx6: spl: Reduce SPL limit size in case CONFIG_SECURE_BOOT is enabled

2019-09-18 Thread Stefano Babic
Hi Jagan, Breno,

On 17/09/19 09:13, Jagan Teki wrote:
> Hi Breno,
> 
> On Thu, Jul 18, 2019 at 6:06 PM Breno Matheus Lima  wrote:
>>
>> In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to
>> avoid a possible HAB failure event:
>>
>> - HAB Event 1 -
>> event data:
>> 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00
>> 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00
>> 0x00 0x01 0x10 0x00
>> STS = HAB_FAILURE (0x33)
>> RSN = HAB_INV_ADDRESS (0x22)
>> CTX = HAB_CTX_TARGET (0x33)
>> ENG = HAB_ENG_ANY (0x00)
>>
>> As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT
>> default") the i.MX6 SPL size limit is 68KB.
>>
>> The ROM code is copying the image size defined in boot data to its
>> respective load address, in case we exceed the OCRAM free region a
>> HAB invalid address failure event is generated.
>>
>> The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size
>> limit based on this configuration.
>>
>> Signed-off-by: Breno Lima 
>> ---
>>  tools/spl_size_limit.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c
>> index 98ff491867..8902e30129 100644
>> --- a/tools/spl_size_limit.c
>> +++ b/tools/spl_size_limit.c
>> @@ -14,6 +14,9 @@ int main(int argc, char *argv[])
>>
>>  #ifdef CONFIG_SPL_SIZE_LIMIT
>> spl_size_limit = CONFIG_SPL_SIZE_LIMIT;
>> +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
>> +   spl_size_limit -= CONFIG_CSF_SIZE;
>> +#endif
> 
> But, if the target enable HAB on SPL the size would be part of SPL
> limit, isn't ?

Indeed - it is not clear to me, too, if it is correct, even if CSF is
added later by the NXP signing tools. The patch reduces significantly
the available space for SPL, I just wondering why just mamoj is
affected. Jagan, does it work without this patch applied ?

Regards,
Stefano

> 
> Just now I have looked at this, since one of my board (imx6_mamoj)
> fails to build.
> 
> Jagan.
> 


-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 3/4] imx6: spl: Reduce SPL limit size in case CONFIG_SECURE_BOOT is enabled

2019-09-19 Thread Breno Matheus Lima
HI Stefano and Jagan,

Em qua, 18 de set de 2019 às 04:59, Stefano Babic  escreveu:
>
> Hi Jagan, Breno,
>
> On 17/09/19 09:13, Jagan Teki wrote:
> > Hi Breno,
> >
> > On Thu, Jul 18, 2019 at 6:06 PM Breno Matheus Lima  
> > wrote:
> >>
> >> In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to
> >> avoid a possible HAB failure event:
> >>
> >> - HAB Event 1 -
> >> event data:
> >> 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00
> >> 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00
> >> 0x00 0x01 0x10 0x00
> >> STS = HAB_FAILURE (0x33)
> >> RSN = HAB_INV_ADDRESS (0x22)
> >> CTX = HAB_CTX_TARGET (0x33)
> >> ENG = HAB_ENG_ANY (0x00)
> >>
> >> As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT
> >> default") the i.MX6 SPL size limit is 68KB.
> >>
> >> The ROM code is copying the image size defined in boot data to its
> >> respective load address, in case we exceed the OCRAM free region a
> >> HAB invalid address failure event is generated.
> >>
> >> The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size
> >> limit based on this configuration.
> >>
> >> Signed-off-by: Breno Lima 
> >> ---
> >>  tools/spl_size_limit.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c
> >> index 98ff491867..8902e30129 100644
> >> --- a/tools/spl_size_limit.c
> >> +++ b/tools/spl_size_limit.c
> >> @@ -14,6 +14,9 @@ int main(int argc, char *argv[])
> >>
> >>  #ifdef CONFIG_SPL_SIZE_LIMIT
> >> spl_size_limit = CONFIG_SPL_SIZE_LIMIT;
> >> +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
> >> +   spl_size_limit -= CONFIG_CSF_SIZE;
> >> +#endif
> >
> > But, if the target enable HAB on SPL the size would be part of SPL
> > limit, isn't ?
>
> Indeed - it is not clear to me, too, if it is correct, even if CSF is
> added later by the NXP signing tools. The patch reduces significantly
> the available space for SPL, I just wondering why just mamoj is
> affected. Jagan, does it work without this patch applied ?
>

When enabling CONFIG_SECURE_BOOT we increase the image length in boot
data by the size defined in CONFIG_CSF_SIZE. The HAB code will parse
the boot data structure and copy the image length defined (SPL image
plus CSF appended) to its respective load address.

HAB code is checking if the image length defined can fit in OCRAM free
region, and logs the following HAB event in case not:

- HAB Event 1 -
event data:
0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00
0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00
0x00 0x01 0x10 0x00
STS = HAB_FAILURE (0x33)
RSN = HAB_INV_ADDRESS (0x22)
CTX = HAB_CTX_TARGET (0x33)
ENG = HAB_ENG_ANY (0x00)

HAB closed targets would then fail to boot, so for that reason we
added CONFIG_CSF_SIZE into consideration.

We can reduce the default CONFIG_CSF_SIZE but it depends on the user
specific HAB setup. I did a quick test with RSA 4K keys and couldn't
achieve 0x2000 length.

Do you think we should decrease default CONFIG_CSF_SIZE? Perhaps
0x2000 plus the maximum dek blob size (0x60) would be enough for most
uses cases, users requiring more space can modify their
CONFIG_CSF_SIZE.

Thanks,
Breno Lima
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 3/4] imx6: spl: Reduce SPL limit size in case CONFIG_SECURE_BOOT is enabled

2019-09-19 Thread Stefano Babic
Hi Breno,

On 19/09/19 03:31, Breno Matheus Lima wrote:
> HI Stefano and Jagan,
> 
> Em qua, 18 de set de 2019 às 04:59, Stefano Babic  escreveu:
>>
>> Hi Jagan, Breno,
>>
>> On 17/09/19 09:13, Jagan Teki wrote:
>>> Hi Breno,
>>>
>>> On Thu, Jul 18, 2019 at 6:06 PM Breno Matheus Lima  
>>> wrote:

 In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to
 avoid a possible HAB failure event:

 - HAB Event 1 -
 event data:
 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00
 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00
 0x00 0x01 0x10 0x00
 STS = HAB_FAILURE (0x33)
 RSN = HAB_INV_ADDRESS (0x22)
 CTX = HAB_CTX_TARGET (0x33)
 ENG = HAB_ENG_ANY (0x00)

 As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT
 default") the i.MX6 SPL size limit is 68KB.

 The ROM code is copying the image size defined in boot data to its
 respective load address, in case we exceed the OCRAM free region a
 HAB invalid address failure event is generated.

 The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size
 limit based on this configuration.

 Signed-off-by: Breno Lima 
 ---
  tools/spl_size_limit.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c
 index 98ff491867..8902e30129 100644
 --- a/tools/spl_size_limit.c
 +++ b/tools/spl_size_limit.c
 @@ -14,6 +14,9 @@ int main(int argc, char *argv[])

  #ifdef CONFIG_SPL_SIZE_LIMIT
 spl_size_limit = CONFIG_SPL_SIZE_LIMIT;
 +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
 +   spl_size_limit -= CONFIG_CSF_SIZE;
 +#endif
>>>
>>> But, if the target enable HAB on SPL the size would be part of SPL
>>> limit, isn't ?
>>
>> Indeed - it is not clear to me, too, if it is correct, even if CSF is
>> added later by the NXP signing tools. The patch reduces significantly
>> the available space for SPL, I just wondering why just mamoj is
>> affected. Jagan, does it work without this patch applied ?
>>
> 
> When enabling CONFIG_SECURE_BOOT we increase the image length in boot
> data by the size defined in CONFIG_CSF_SIZE. The HAB code will parse
> the boot data structure and copy the image length defined (SPL image
> plus CSF appended) to its respective load address.
> 
> HAB code is checking if the image length defined can fit in OCRAM free
> region, and logs the following HAB event in case not:
> 
> - HAB Event 1 -
> event data:
> 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00
> 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00
> 0x00 0x01 0x10 0x00
> STS = HAB_FAILURE (0x33)
> RSN = HAB_INV_ADDRESS (0x22)
> CTX = HAB_CTX_TARGET (0x33)
> ENG = HAB_ENG_ANY (0x00)
> 
> HAB closed targets would then fail to boot, so for that reason we
> added CONFIG_CSF_SIZE into consideration.
> 

Clear - thanks for detailed explanation.

> We can reduce the default CONFIG_CSF_SIZE but it depends on the user
> specific HAB setup. I did a quick test with RSA 4K keys and couldn't
> achieve 0x2000 length.

That is much less as we have now.

> 
> Do you think we should decrease default CONFIG_CSF_SIZE?

I think yes - if we set it for the worst case, we reduce the SPL size so
much that most boards, if they enable SECURE_BOOT, won't build. I cannot
say that imx6dl_mamoj has dead code in its SPL, it is one of the board
with the "state of art" in U-Boot, with DM and OF in SPL. But this is
also something we decided to push into U-Boot. Anyway, every board
maintainer can change it and add it to the own defconfig.

Jagan, after setting CONFIG_CSF_SIZE to 0x2060 as suggested by Breno,
board builds fine - but I have no idea if it can boots. Can you check
this ?

> Perhaps
> 0x2000 plus the maximum dek blob size (0x60) would be enough for most
> uses cases, users requiring more space can modify their
> CONFIG_CSF_SIZE.

Best regards,
Stefano

-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 3/4] imx6: spl: Reduce SPL limit size in case CONFIG_SECURE_BOOT is enabled

2019-09-19 Thread Stefano Babic
On 19/09/19 07:37, Jagan Teki wrote:
> Hi Stefano,
> 
> On Wed, Sep 18, 2019 at 1:29 PM Stefano Babic  wrote:
>>
>> Hi Jagan, Breno,
>>
>> On 17/09/19 09:13, Jagan Teki wrote:
>>> Hi Breno,
>>>
>>> On Thu, Jul 18, 2019 at 6:06 PM Breno Matheus Lima  
>>> wrote:

 In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to
 avoid a possible HAB failure event:

 - HAB Event 1 -
 event data:
 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00
 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00
 0x00 0x01 0x10 0x00
 STS = HAB_FAILURE (0x33)
 RSN = HAB_INV_ADDRESS (0x22)
 CTX = HAB_CTX_TARGET (0x33)
 ENG = HAB_ENG_ANY (0x00)

 As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT
 default") the i.MX6 SPL size limit is 68KB.

 The ROM code is copying the image size defined in boot data to its
 respective load address, in case we exceed the OCRAM free region a
 HAB invalid address failure event is generated.

 The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size
 limit based on this configuration.

 Signed-off-by: Breno Lima 
 ---
  tools/spl_size_limit.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c
 index 98ff491867..8902e30129 100644
 --- a/tools/spl_size_limit.c
 +++ b/tools/spl_size_limit.c
 @@ -14,6 +14,9 @@ int main(int argc, char *argv[])

  #ifdef CONFIG_SPL_SIZE_LIMIT
 spl_size_limit = CONFIG_SPL_SIZE_LIMIT;
 +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
 +   spl_size_limit -= CONFIG_CSF_SIZE;
 +#endif
>>>
>>> But, if the target enable HAB on SPL the size would be part of SPL
>>> limit, isn't ?
>>
>> Indeed - it is not clear to me, too, if it is correct, even if CSF is
>> added later by the NXP signing tools. The patch reduces significantly
>> the available space for SPL, I just wondering why just mamoj is
>> affected. Jagan, does it work without this patch applied ?
> 
> mamoj is affected since the board enables SPL_DM, SPL_OF_CONTROL. Yes,
> the build look fine without this patch.

Anyway, SPL size does not seem to much. But dropping 0x4000 to the
available size is really a lot, and I hope we can reduce this.

Regards,
Stefano




-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 3/4] imx6: spl: Reduce SPL limit size in case CONFIG_SECURE_BOOT is enabled

2019-09-19 Thread Jagan Teki
Hi Stefano,

On Wed, Sep 18, 2019 at 1:29 PM Stefano Babic  wrote:
>
> Hi Jagan, Breno,
>
> On 17/09/19 09:13, Jagan Teki wrote:
> > Hi Breno,
> >
> > On Thu, Jul 18, 2019 at 6:06 PM Breno Matheus Lima  
> > wrote:
> >>
> >> In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to
> >> avoid a possible HAB failure event:
> >>
> >> - HAB Event 1 -
> >> event data:
> >> 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00
> >> 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00
> >> 0x00 0x01 0x10 0x00
> >> STS = HAB_FAILURE (0x33)
> >> RSN = HAB_INV_ADDRESS (0x22)
> >> CTX = HAB_CTX_TARGET (0x33)
> >> ENG = HAB_ENG_ANY (0x00)
> >>
> >> As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT
> >> default") the i.MX6 SPL size limit is 68KB.
> >>
> >> The ROM code is copying the image size defined in boot data to its
> >> respective load address, in case we exceed the OCRAM free region a
> >> HAB invalid address failure event is generated.
> >>
> >> The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size
> >> limit based on this configuration.
> >>
> >> Signed-off-by: Breno Lima 
> >> ---
> >>  tools/spl_size_limit.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c
> >> index 98ff491867..8902e30129 100644
> >> --- a/tools/spl_size_limit.c
> >> +++ b/tools/spl_size_limit.c
> >> @@ -14,6 +14,9 @@ int main(int argc, char *argv[])
> >>
> >>  #ifdef CONFIG_SPL_SIZE_LIMIT
> >> spl_size_limit = CONFIG_SPL_SIZE_LIMIT;
> >> +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
> >> +   spl_size_limit -= CONFIG_CSF_SIZE;
> >> +#endif
> >
> > But, if the target enable HAB on SPL the size would be part of SPL
> > limit, isn't ?
>
> Indeed - it is not clear to me, too, if it is correct, even if CSF is
> added later by the NXP signing tools. The patch reduces significantly
> the available space for SPL, I just wondering why just mamoj is
> affected. Jagan, does it work without this patch applied ?

mamoj is affected since the board enables SPL_DM, SPL_OF_CONTROL. Yes,
the build look fine without this patch.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 3/4] imx6: spl: Reduce SPL limit size in case CONFIG_SECURE_BOOT is enabled

2019-09-23 Thread Breno Matheus Lima
Hi Stefano and Jagan,


Em qui, 19 de set de 2019 às 05:27, Stefano Babic  escreveu:
>
> On 19/09/19 07:37, Jagan Teki wrote:
> > Hi Stefano,
> >
> > On Wed, Sep 18, 2019 at 1:29 PM Stefano Babic  wrote:
> >>
> >> Hi Jagan, Breno,
> >>
> >> On 17/09/19 09:13, Jagan Teki wrote:
> >>> Hi Breno,
> >>>
> >>> On Thu, Jul 18, 2019 at 6:06 PM Breno Matheus Lima  
> >>> wrote:
> 
>  In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to
>  avoid a possible HAB failure event:
> 
>  - HAB Event 1 -
>  event data:
>  0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00
>  0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00
>  0x00 0x01 0x10 0x00
>  STS = HAB_FAILURE (0x33)
>  RSN = HAB_INV_ADDRESS (0x22)
>  CTX = HAB_CTX_TARGET (0x33)
>  ENG = HAB_ENG_ANY (0x00)
> 
>  As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT
>  default") the i.MX6 SPL size limit is 68KB.
> 
>  The ROM code is copying the image size defined in boot data to its
>  respective load address, in case we exceed the OCRAM free region a
>  HAB invalid address failure event is generated.
> 
>  The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size
>  limit based on this configuration.
> 
>  Signed-off-by: Breno Lima 
>  ---
>   tools/spl_size_limit.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
>  diff --git a/tools/spl_size_limit.c b/tools/spl_size_limit.c
>  index 98ff491867..8902e30129 100644
>  --- a/tools/spl_size_limit.c
>  +++ b/tools/spl_size_limit.c
>  @@ -14,6 +14,9 @@ int main(int argc, char *argv[])
> 
>   #ifdef CONFIG_SPL_SIZE_LIMIT
>  spl_size_limit = CONFIG_SPL_SIZE_LIMIT;
>  +#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_CSF_SIZE)
>  +   spl_size_limit -= CONFIG_CSF_SIZE;
>  +#endif
> >>>
> >>> But, if the target enable HAB on SPL the size would be part of SPL
> >>> limit, isn't ?
> >>
> >> Indeed - it is not clear to me, too, if it is correct, even if CSF is
> >> added later by the NXP signing tools. The patch reduces significantly
> >> the available space for SPL, I just wondering why just mamoj is
> >> affected. Jagan, does it work without this patch applied ?
> >
> > mamoj is affected since the board enables SPL_DM, SPL_OF_CONTROL. Yes,
> > the build look fine without this patch.
>
> Anyway, SPL size does not seem to much. But dropping 0x4000 to the
> available size is really a lot, and I hope we can reduce this.
>

Thanks for submitting a fix for mamoj board.

We should also reduce CSF_SIZE in default_image.c and image.c to avoid
a U-Boot proper authentication failure in HAB closed devices. The
current U-Boot tools code is hardcoding CSF_SIZE as 0x2000 and
mx6ul_14x14_defconfig target is failing to boot with error below:

Authenticate image from DDR location 0x877fffc0...
bad magic magic=0x32 length=0x6131 version=0x38
bad length magic=0x32 length=0x6131 version=0x38
bad version magic=0x32 length=0x6131 version=0x38
spl: ERROR:  image authentication fail

The intent of "habv4: tools: Avoid hardcoded CSF size for SPL targets"
is to avoid such issue. I tried to apply this patch back but I'm
seeing gunzip related errors as reported by Igor Opaniuk. We may need
to understand better this dependency.

I have just submitted a patch reducing default CSF_SIZE to 0x2060,
this patch is also modifying default_image.c and image.c but CSF_SIZE
still hardcoded in U-Boot tools code.

Thanks,
Breno Lima
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 3/4] imx6: spl: Reduce SPL limit size in case CONFIG_SECURE_BOOT is enabled

2019-07-18 Thread Fabio Estevam
On Thu, Jul 18, 2019 at 9:34 AM Breno Matheus Lima  wrote:
>
> In case CONFIG_SECURE_BOOT is enabled we need to limit the SPL size to
> avoid a possible HAB failure event:
>
> - HAB Event 1 -
> event data:
> 0xdb 0x00 0x14 0x42 0x33 0x22 0x33 0x00
> 0x00 0x00 0x00 0x0f 0x00 0x90 0x70 0x00
> 0x00 0x01 0x10 0x00
> STS = HAB_FAILURE (0x33)
> RSN = HAB_INV_ADDRESS (0x22)
> CTX = HAB_CTX_TARGET (0x33)
> ENG = HAB_ENG_ANY (0x00)
>
> As explained in Commit 23612534fe0f ("spl: imx6: Provide a SPL_SIZE_LIMIT
> default") the i.MX6 SPL size limit is 68KB.
>
> The ROM code is copying the image size defined in boot data to its
> respective load address, in case we exceed the OCRAM free region a
> HAB invalid address failure event is generated.
>
> The maximum CSF size is defined in CONFIG_CSF_SIZE, reduce SPL size
> limit based on this configuration.
>
> Signed-off-by: Breno Lima 

Reviewed-by: Fabio Estevam 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot