Re: [PATCH v1 0/2] imx8m: fix secure boot

From: Heiko Schocher <hs@denx.de>
To: "Peng Fan (OSS)" <peng.fan@oss.nxp.com>,
	"u-boot@lists.denx.de" <u-boot@lists.denx.de>,
	Stefano Babic <sbabic@denx.de>
Cc: "tharvey@gateworks.com" <tharvey@gateworks.com>,
	"Alexandru Gagniuc" <mr.nuke.me@gmail.com>,
	"Fabio Estevam" <festevam@gmail.com>,
	"Faiz Abbas" <faiz_abbas@ti.com>,
	"Jaehoon Chung" <jh80.chung@samsung.com>,
	"Marek Vasut" <marex@denx.de>, "Michael Walle" <michael@walle.cc>,
	"Michal Simek" <michal.simek@xilinx.com>,
	dl-uboot-imx <uboot-imx@nxp.com>, "Pali Rohár" <pali@kernel.org>,
	"Philippe Reynes" <philippe.reynes@softathome.com>,
	"Simon Glass" <sjg@chromium.org>, "Stefan Roese" <sr@denx.de>,
	"t.remmet@phytec.de" <T.remmet@phytec.de>
Subject: Re: [PATCH v1 0/2] imx8m: fix secure boot
Date: Fri, 6 Aug 2021 08:39:36 +0200	[thread overview]
Message-ID: <30be0d4f-adad-c5b0-935a-49b2e604f615@denx.de> (raw)
In-Reply-To: <3a0116de-b73e-274f-5d40-def37bfdea17@oss.nxp.com>

Hello Peng,

On 06.08.21 07:56, Peng Fan (OSS) wrote:
> 
> 
> On 2021/8/6 12:44, Heiko Schocher wrote:
>>
>> This series fixes secure boot on imx8m based boards. Tim
>> also detected this issue and the patches fixed on his hardware
>> also the problem, see discussion here:
>>
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.denx.de%2Fpipermail%2Fu-boot%2F2021-July%2F454351.html&amp;data=04%7C01%7Cpeng.fan%40nxp.com%7C60053de52160416b6d9608d95894ead0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637638218882598843%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=ZF%2FCsb%2FdBiLKHa5o6wRmAsQOIduZHhFI5MC%2BuIB0Vz4%3D&amp;reserved=0
>>
>>
>> Problem is that the IVT header gets loaded to a memallocated
>> buffer, but it needs to sit on memaddress coded in IVT header
>> itself. This patchseries adds a weak function spl_load_simple_fit()
>> in common spl code, which does not change current code behaviour.
>>
>> Second patch than implements this weak function for imx based
>> boards (if no IVT header is found on address which is passed
>> to it, it does nothing).
>>
>> I am not sure if this is the best solution, but it fixes a real
>> bug, and may could be made clearer, if possible.
> 
> NXP downstream dropped malloc, with
> buf = board_spl_fit_buffer_addr(size, sectors, info->bl_len);
> 
> And this will use previous fixed address.

Ah, okay, you mean:

https://source.codeaurora.org/external/imx/uboot-imx/tree/arch/arm/mach-imx/spl.c?h=lf_v2021.04#n334

https://source.codeaurora.org/external/imx/uboot-imx/tree/common/spl/spl_fit.c?h=lf_v2021.04#n541

and

https://source.codeaurora.org/external/imx/uboot-imx/tree/common/spl/spl_fit.c?h=lf_v2021.04#n581

correct?

But I do not see, where ivt->self is used... or is per definiton
ivt->self equal to:
https://source.codeaurora.org/external/imx/uboot-imx/tree/arch/arm/mach-imx/spl.c?h=lf_v2021.04#n345

?

bye,
Heiko

> 
> Regards,
> Peng.
> 
>>
>>
>> Heiko Schocher (2):
>>    spl_fit. add hook to make fixes after fit header is loaded
>>    imx: spl: implement spl_load_simple_fit_fix_load
>>
>>   arch/arm/mach-imx/spl.c | 33 +++++++++++++++++++++++++++++++++
>>   common/spl/spl_fit.c    | 11 +++++++++++
>>   include/spl.h           |  8 ++++++++
>>   3 files changed, 52 insertions(+)
>>

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