On Fri, 2021-08-06 at 08:39 +0200, Heiko Schocher wrote: > Caution: EXT Email > > 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%2 > > > Flists.denx.de%2Fpipermail%2Fu-boot%2F2021- > > > July%2F454351.html&data=04%7C01%7Cye.li%40nxp.com%7C4e50cef1a > > > 559457dc78c08d958a4f5d9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C > > > 0%7C637638287788477666%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMD > > > AiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata > > > =Rcmml9Sg0hSc%2FE68Dzjcn0wce1xYSpQNfJ0wfT4Jork%3D&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://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsou > rce.codeaurora.org%2Fexternal%2Fimx%2Fuboot- > imx%2Ftree%2Farch%2Farm%2Fmach- > imx%2Fspl.c%3Fh%3Dlf_v2021.04%23n334&data=04%7C01%7Cye.li%40nxp.c > om%7C4e50cef1a559457dc78c08d958a4f5d9%7C686ea1d3bc2b4c6fa92cd99c5c301 > 635%7C0%7C0%7C637638287788487624%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w > LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sda > ta=prhVBpPvqD1CDGWi7tWcN5%2BzChBeSQzeIK%2FvhedGcfE%3D&reserved=0 > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsou > rce.codeaurora.org%2Fexternal%2Fimx%2Fuboot- > imx%2Ftree%2Fcommon%2Fspl%2Fspl_fit.c%3Fh%3Dlf_v2021.04%23n541&da > ta=04%7C01%7Cye.li%40nxp.com%7C4e50cef1a559457dc78c08d958a4f5d9%7C686 > ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637638287788487624%7CUnknown% > 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJ > XVCI6Mn0%3D%7C1000&sdata=J%2FH%2FBBtiMMl9G744CjjPESEUVCxmO%2Bg7%2 > BHVJsM1yKc4%3D&reserved=0 > > and > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsou > rce.codeaurora.org%2Fexternal%2Fimx%2Fuboot- > imx%2Ftree%2Fcommon%2Fspl%2Fspl_fit.c%3Fh%3Dlf_v2021.04%23n581&da > ta=04%7C01%7Cye.li%40nxp.com%7C4e50cef1a559457dc78c08d958a4f5d9%7C686 > ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637638287788487624%7CUnknown% > 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJ > XVCI6Mn0%3D%7C1000&sdata=WVZ6Az8kazKu%2BWzysM7%2B3u5XHOb6gtggwiCK > rewnI2o%3D&reserved=0 > > correct?
Yes. correct. > > But I do not see, where ivt->self is used... or is per definiton > ivt->self equal to: > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsou > rce.codeaurora.org%2Fexternal%2Fimx%2Fuboot- > imx%2Ftree%2Farch%2Farm%2Fmach- > imx%2Fspl.c%3Fh%3Dlf_v2021.04%23n345&data=04%7C01%7Cye.li%40nxp.c > om%7C4e50cef1a559457dc78c08d958a4f5d9%7C686ea1d3bc2b4c6fa92cd99c5c301 > 635%7C0%7C0%7C637638287788487624%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w > LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sda > ta=Fo5efFghnqsyUvtuykwVm68NvDnk%2Fb1hCoiuQW1JkiA%3D&reserved=0 > > ? > The fit buffer was used in SPL is a fit size related offset to u-boot base. In mkimage, we generate IVT following the same calculation. So we don't use ivt->self, this address is aligned between SPL and IVT. Your patch depends on IVT. But actually IVT is not necessary for non- secure boot. The board_spl_fit_size_align in mach-imx/spl.c is only defined for HAB enabled. So for non-secure boot, it does not include size for IVT. This will be an issue. Best regards, Ye Li > 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: h...@denx.de
