Re: [PATCH 1/2] lib: optee: remove the duplicate CONFIG_OPTEE

["Alex G." <mr.nuke.me@gmail.com>]

On 9/6/21 11:53 AM, Patrick DELAUNAY wrote:
>>
>>> In fact, the SPL boot path for OP-TEE doesn't use this function. That's
>>> intentional.
>>>
>>> Here's what I suggest:
>>>     - Remove OPTEE_TZDRAM_BASE and _SIZE
>> There is some legacy here, board/warp7and board/technexion/pico-imx7d.
> 
> 
> it is not possible, it is used for U-Boot proper on other platforms
> 
> board/warp7/warp7.c:38:        gd->ram_size -= CONFIG_OPTEE_TZDRAM_SIZE;
> board/warp7/warp7.c:122:    optee_start = optee_end - CONFIG_OPTEE_TZDRAM_SIZE;
> board/technexion/pico-imx7d/pico-imx7d.c:56: gd->ram_size -= CONFIG_OPTEE_TZDRAM_SIZE;
> include/configs/mx7_common.h:52:#if (CONFIG_OPTEE_TZDRAM_SIZE != 0)

I have an idea how to work around that.


> And for me this configuration (size of memory used by OPTEE) is more a 
> system configuration
> depending of the OP-TEE firmware used than a Device Tree configuration 
> at SPL level
> 
> PS: for the TF-A case it is done in a secure FW configuration file => in 
> the FIP
>        this information is no hardcoded information in BL2
>      in SPL, the load address / entry point it is already provided by 
> FIT for OPTEE image
> 
>       (=> optee_image_get_load_addr / optee_image_get_entry_point)
>       no need to have this information in DT (optee base address)
> 
> tools/default_image.c:119
> 
>      if (params->os == IH_OS_TEE) {
>          addr = optee_image_get_load_addr(hdr);
>          ep = optee_image_get_entry_point(hdr);
> 
>      }

The OPTEE entry point is available:
1) in both FIT and uImage files.
2) As the optee reserved-memory node in DT
3) Via CONFIG_OPTEE_TZDRAM_BASE

On the one hand, (1) and (2) together could hint that the OPTEE image is 
incompatible with the board, so they are not completely redundant.
On the other hand, there is no point in (3) given that the information 
could be obtained in at least two other ways.


> 
>      for CONFIG_OPTEE_TZDRAM_SIZE, I think that can be also found by 
> parsing the OP-TEE header
> 
> => see : init_mem_usage
> 
>      the OPTEE should be access to this memory .....
>      and it can change the firewall configuration is it is necessary
>      for the shared memory for example
> 
> 
> => no need to update first stage boot loader = SPL (with the risk to 
> brick the device)
>       when only OP-TEE firmware change

I see your point. It's a packaging issue, which we could solve with FIT, 
but not with uImage. Though, how often does an OP-TEE update change the 
TZDRAM location?


>>>     - Remove optee_verify_bootm_image()
> 
> but it is used in
> 
> common/bootm_os.c:491:    ret = 
> optee_verify_boot_image(images->os.image_start,

Yes. It only checks if the OP-TEE image fits within some hardcoded, and 
potentially wrong, boundaries. Which is contrary to your arguments from 
a few paragraphs ago. Just don't call optee_verify_boot_image in bootm_os.c.

Alex