Re: [PATCH] imx: hab: fix size of IVT+CSF blob tacked on to u-boot.itb

[Rasmus Villemoes <ravi@prevas.dk>]

On Fri, Oct 25 2024, Marek Vasut <marex@denx.de> wrote:

> On 10/25/24 9:10 AM, Rasmus Villemoes wrote:
>> On Thu, Oct 24 2024, Marek Vasut <marex@denx.de> 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 <ravi@prevas.dk>
>>>> ---
>>>> 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