On Fri, Oct 25 2024, Marek Vasut <[email protected]> wrote: > On 10/25/24 9:10 AM, Rasmus Villemoes wrote: >> On Thu, Oct 24 2024, Marek Vasut <[email protected]> wrote: >> >>> On 10/24/24 2:27 PM, Rasmus Villemoes wrote: >>>> Loading flash.bin using uuu fails when flash.bin does not have the >>>> right size. >>>> When flash.bin is loaded from some storage medium (sd card/emmc), >>>> SPL >>>> just loads some random garbage bytes from beyond what has been >>>> populated when flash.bin was written, but when loaded via uuu, SPL >>>> hangs waiting for the host to send the expected number of bytes. Which >>>> is (size of FIT image aligned to 0x1000)+CONFIG_CSF_SIZE. The >>>> alignment to 0x1000 is already done and is necessary in all cases >>>> because that's the exact expected location of the 32 byte IVT >>>> header. But the IVT+CSF blob tacked onto the end must be a total of >>>> CONFIG_CSF_SIZE. >>>> This is exactly the same fix as 89f19f45d650, except that this time >>>> around I don't know how to cleanly get CONFIG_CSF_SIZE. >>>> Fixes: bc6beae7c55f (binman: Add nxp_imx8mcst etype for i.MX8M >>>> flash.bin signing) >>>> Signed-off-by: Rasmus Villemoes <[email protected]> >>>> --- >>>> Heiko, can you check if this works for you? >>>> And if somebody wants to pick this up and knows how to get at >>>> CONFIG_ >>>> values, feel free to fix up and take authorship. But perhaps it's not >>>> really configurable at all; imx8mimage.c has the value 0x2000 >>>> hard-coded, so I don't think anything good could ever come from >>>> modifying CONFIG_CSF_SIZE. If so, the right fix is probably just to >>>> make that knob non-settable. >>>> tools/binman/etype/nxp_imx8mcst.py | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> diff --git a/tools/binman/etype/nxp_imx8mcst.py >>>> b/tools/binman/etype/nxp_imx8mcst.py >>>> index 8221517b0c4..9a1974cc522 100644 >>>> --- a/tools/binman/etype/nxp_imx8mcst.py >>>> +++ b/tools/binman/etype/nxp_imx8mcst.py >>>> @@ -137,6 +137,8 @@ class Entry_nxp_imx8mcst(Entry_mkimage): >>>> args = ['-i', cfg_fname, '-o', output_fname] >>>> if self.cst.run_cmd(*args) is not None: >>>> outdata = tools.read_file(output_fname) >>>> + # fixme: 0x2000 should be CONFIG_CSF_SIZE >>>> + outdata += tools.get_bytes(0, 0x2000 - 0x20 - len(outdata)) >>>> return data + outdata >>>> else: >>>> # Bintool is missing; just use the input data as the output >>> >>> I have to admit, I never really figured out this binman stuff, but >>> shouldn't the fix be also in tools/binman/etype/nxp_imx8mimage.py ? >> No, why? That logic is all about generating the imx-specific header >> in >> front of SPL.bin, there's no CSF being generated. > > This patch is modifying tools/binman/etype/nxp_imx8mcst.py which is > the Code-Signing-Tool binman etype, the thing used for generating > SIGNED images during HAB use. > > Shouldn't this patch modify tools/binman/etype/nxp_imx8mimage.py which > is the etype used when generating flash.bin itself ?
No, because this is not about flash.bin per se, but about the (u-boot FIT+IVT+CSF) blob that forms part of flash.bin, in the IMX_HAB case. > My understanding was that the problem Heiko reported was alignment of > flash.bin , independently of it being signed or not ? Read the thread again. Heiko's problem was very much related to IMX_HAB being enabled. I've already explained what the problem actually is and why this fixes it. Not only once, but as I said, this is equivalent to a fix I sent for the shell script implementation, so to answer a question you asked elsewhere: Yes, this is missing in the binman implementation. Extending the size of flash.bin in the !IMX_HAB case will have the opposite problem (as will adding the wrong amount of padding bytes in the IMX_HAB case), namely that the board will boot, but uuu on the host will "hang" waiting for the target to consume the remaining bytes. Rasmus
