Re: [PATCH] arm64: dts: imx8m: Reinstate TEE ifdeffery

[Marek Vasut <marex@nabladev.com>]

On 7/3/26 8:16 AM, Yannic Moog wrote:

Hello Yannic,

[...]

>>> is not accurate. You were able to package OP-TEE without needing to enable support in U-Boot.
>>> And
>>> afaik this is a perfectly valid use-case.
>>
>> When you include optee-os in the U-Boot fitImage, what do you do with
>> that optee-os which U-Boot does not support ? Details please ?
> 
> Sure, sorry. OS can/will use it. There is no requirement, that I know, of any bootloader
> interfacing with OP-TEE for proper functionality and its purpose is to work with Linux kernel.

At minimum, if CONFIG_OPTEE is not enabled, U-Boot won't execute 
optee_copy_fdt_nodes() and won't transfer any optee-os firmware and 
reserved-memory DT nodes to the Linux DT when starting the kernel, so 
Linux DT will be buggy and Linux may possibly misbehave.

> I guess the only job bootloader needs to do is load it into memory. I don't know details about
> specific uses within the security context and if there are any that require bootloader support.

If in your use case, U-Boot is supposed to only load optee-os into 
memory , why not load it from U-Boot command line or as part of fitImage 
when booting Linux kernel ?

It seems in your use case, optee-os does not have to be part of the 
bootloader image at all ?

>>> With this change you can no longer do that, so this change is a regression.
>>>
>>> About the warning message. I understand that you find it confusing; binman is designed so that
>>> it
>>> emits a warning when an optional blob is not found.
>>
>> Please see above.
>>
>>> I prefer that approach to no message at all. You may want to package OP-TEE (e.g. you have an
>>> tf-a
>>> blob which jumps to OP-TEE addr) and being alerted to a possible mistake is important; Let's
>>> say
>>> tee path was incorrect during build.
>>
>> Is this some sort of a board-specific configuration ?
> 
> kind of. We have tee path set in <soc>-u-boot.dtsi: filename = "tee.bin";
> 
> Could happen that TEE=teee.bin is set or file is named tee-raw.bin and then it helps to get a
> warning that binman was not able to package the blob.
> 
>>
>> Why does U-Boot not enable OPTEE-OS support in this case ?
> 
> No need to. Maybe to conserve space, or by principle only enable the configs that are actually
> needed? I believe PHYTEC boards used to do that before KASLR support.

This does not seem correct, please see above.

At this point, I would suggest that if you want to retain this kind of 
configuration for your hardware, it should be moved into board specific 
DT, it should not be part of the common DTs.

>>> Also, keeping optional when using if defs is not a good idea. Binman will still build you an
>>> image
>>> when no OP-TEE blob is present, but should not since you enabled OPTEE configs. If you enable
>>> OPTEE, you should have an OP-TEE blob packaged.
>>> We could add ifndef OPTEE around the optional property.
>>
>> The optional keyword is actually kept in this case, it is not a full revert.
>>
>> You did however miss one vital point -- the tee node is part of the
>> U-boot fitImage, even if the tee.bin binary is not present and OPTEE
>> support in U-Boot is disabled. Binman prints a confusing message, and
>> generates a bogus empty tee {} node in the fitImage. That is incorrect.
> 
> Yes, forgot to address that. I don't think it is confusing, at least content wise. Visually, I can
> see how that is confusing. I want to neither agree nor disagree on the correctness part because I
> don't really know the fitImage spec. Having the tee node be part of the fitImage is a design
> choice, all optional blobs behave that way from my understanding.

It is a broken node introduced by commit b57ed147939c ("dts: 
imx8m{m,n,p,q}: Make optee packaging optional") , it is not supposed to 
be part of the fitImage unless optee-os is bundled into the fitImage.

> I guess it means that there is a bug in binman if you say having empty tee node is incorrect.
> And so it should be fixed in binman itself imo, not here for imx8.
No, this is not a bug in binman, please see above.