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
