Hi Patrick
On 9/2/21 4:56 AM, Patrick Delaunay wrote:
The configuration CONFIG_OPTEE is defined 2 times: 1- in lib/optee/Kconfig for support of OPTEE images loaded by bootm command 2- in drivers/tee/optee/Kconfig for support of OP-TEE driver.
It is abnormal to have the same CONFIG define for 2 purpose; and it is difficult to managed correctly their dependencies.
Moreover CONFIG_SPL_OPTEE is defined in common/spl/Kconfig to manage OPTEE image load in SPL.
This definition causes an issue with the macro CONFIG_IS_ENABLED(OPTEE) to test the availability of the OP-TEE driver.
This patch cleans the configuration dependency with:
- CONFIG_OPTEE_IMAGE (renamed) => support of OP-TEE image in U-Boot
- CONFIG_SPL_OPTEE_IMAGE (renamed) => support of OP-TEE image in SPL
- CONFIG_OPTEE (same) => support of OP-TEE driver in U-Boot
- CONFIG_OPTEE_LIB (new) => support of OP-TEE library
After this patch, the macro have the correct behavior:
- CONFIG_IS_ENABLED(OPTEE_IMAGE) => Load of OP-TEE image is supported
- CONFIG_IS_ENABLED(OPTEE) => OP-TEE driver is supported
It seems a little odd to have both OPTEE_LIB and OPTEE_IMAGE, since they are both used together to support booting with OP-TEE. What also seems odd is that "OP-TEE driver in U-Boot" does not depend on "OP-TEE library".
Introducing OPTEE_LIB then, makes sense to me, provided that OPTEE depends on OPTEE_LIB, but I'm not sure about OPTEE_IMAGE.
diff --git a/lib/optee/optee.c b/lib/optee/optee.c index 672690dc53..5676785cb5 100644 --- a/lib/optee/optee.c +++ b/lib/optee/optee.c @@ -20,6 +20,7 @@ "\n\theader lo=0x%08x hi=0x%08x size=0x%08lx arch=0x%08x" \ "\n\tuimage params 0x%08lx-0x%08lx\n"
+#if defined(CONFIG_OPTEE_IMAGE) int optee_verify_image(struct optee_header *hdr, unsigned long tzdram_start, unsigned long tzdram_len, unsigned long image_len) { @@ -70,6 +71,7 @@ error:
return ret; } +#endif
One the idea of having CONFIGs is to include/exclude code via obj-$(CONFIG_FOO)+=code.c. This prevents the proliferation of #ifdefs. It's fairly counterintuitive to have a CONFIG_OPTEE_IMAGE in a file controlled by CONFIG_OPTEE_LIB.
Going to optee_verify_image() itself. It essentially checks against OPTEE_TZDRAM_(BASE/SIZE). But those are a derived from devicetree, not Kconfig. So it seems the motivation behing optee_verify_bootm_image() is flawed. Also the error message is not very helpful.
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 - Remove optee_verify_bootm_image() - No need for CONFIG_OPTEE_IMAGE
Alex