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

[Marek Vasut]

On 10/27/24 7:38 PM, Rasmus Villemoes wrote:
> On Fri, Oct 25 2024, Marek Vasut <marex at denx.de> wrote:
> 
>> On 10/25/24 9:10 AM, Rasmus Villemoes wrote:
>>> On Thu, Oct 24 2024, Marek Vasut <marex at 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 at 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.
Oh, I see, all right then. Thanks for clarifying.