On Mon, Sep 22, 2025 at 11:29:07AM +0200, Quentin Schulz wrote: > Hi Tom, > > On 9/19/25 6:10 PM, Tom Rini wrote: > > On Fri, Sep 19, 2025 at 05:50:21PM +0200, Quentin Schulz wrote: > > > > > From: Quentin Schulz <[email protected]> > > > > > > It is currently possible to build an image with an OP-TEE OS (via the > > > TEE environment variable) without OPTEE_LIB. U-Boot will happily load > > > the TEE OS and the next OS (e.g. the Linux kernel). > > > > > > This is an issue because on FDT-enabled devices, OP-TEE OS adds nodes to > > > the reserved-memory FDT node for the memory regions it just reserved for > > > itself. This updated is then passed to U-Boot proper which should know > > > better not to use memory from there. The actual issue is that without > > > OPTEE_LIB and OF_LIBFDT enabled, U-Boot proper will not copy those nodes > > > over to the next OS's FDT before starting it. This results in the next > > > OS's (e.g. Linux kernel) to not be aware of reserved memory, incurring > > > random crashes or device reboots when it tries to access secure reserved > > > memory area. > > > > > > Signed-off-by: Quentin Schulz <[email protected]> > > > --- > > > I've just lost a day trying to figure out why my board suddenly boot > > > loops when running a specific program in Linux userspace after updating > > > U-Boot. I actually inadvertently had the TEE environment variable set > > > for a device which doesn't actually need to run any TEE OS (so had > > > OPTEE_LIB disabled). > > > > > > This device is Rockchip PX30-based and the image is built with binman > > > like all Rockchip-based devices. > > > > > > The issue is that I built a U-Boot FIT with an OP-TEE OS that reserves > > > some memory regions for itself (via the TEE environment variable) but > > > U-Boot proper "forgot" to propagate those to the Linux kernel's FDT > > > (because of the missing OPTEE_LIB). > > > > > > Unfortunately, binman doesn't seem to have access to Kconfig symbols > > > (grep CONFIG_ doesn't return anything meaningful and binman is either > > > configured through FDT nodes or via CLI arguments, c.f. cmd_binman in > > > the root Makefile) so I cannot try to be smart and guide the user to > > > the correct Kconfig option to select if TEE is set. I could add a > > > property based on the presence of OPTEE_LIB in rockchip-u-boot.dtsi for > > > example and have a custom message based on that, the issue is that I > > > assume all FDT-based platforms do actually need to do this dance, and > > > not only Rockchip. > > > Another option could be to add a CLI argument to binman through which > > > we would pass the state of OPTEE_LIB and error out the build in that > > > case, but that feels like opening the door to dirty hacks (though this > > > patch is admittedly another dirty hack :) ). > > > > > > Another option is to propagate the TEE environment variable to the > > > preprocessor of the FDT (via dtc_cpp_flags) and then I can do > > > > > > #if defined(TEE) && !IS_ENABLED(CONFIG_OPTEE_LIB) > > > #error "CONFIG_OPTEE_LIB must be enabled!" > > > #endif > > > > > > but we have the same issue as above, it is then Rockchip specific and > > > also feels bad. > > > > > > Another option is to remove the @tee-SEQ node from the binman FIT > > > description when OPTEE_LIB isn't set but then I would lose the nice > > > message when no TEE is provided: > > > > > > Image 'simple-bin' is missing optional external blobs but is still > > > functional: tee-os > > > > > > and even worse, build without any TEE OS even though I could provide one > > > with the TEE environment variable. > > > > > > Finally, another option could be to move this hack under > > > arch/arm/mach-rockchip/Kconfig to make it Rockchip specific or add a > > > depends on ARCH_ROCKCHIP > > > > > > OP-TEE OS on Aarch32 Rockchip boards doesn't actually need any of that > > > if SPL_OPTEE_IMAGE is set because arch/arm/mach-rockchip/sdram.c then > > > marks some hardcoded memory regions in RAM as holes in DRAM, which has > > > the same effect as reserved memory regions I guess. I assume other > > > platforms may use something different, so it may be casting too wide of > > > a net. > > > > > > Does anyone have something more delicate than this sledgehammer of a > > > solution? > > > > It looks like on other SoCs there's an ARCH_x wide imply/select OPTEE > > (which in turn gets OPTEE_LIB), so is there a case for ARCH_ROCKCHIP && > > !ARM64 && !OPTEE ? I see you explained that 32bit has a workaround for > > this already. > > > > The issue is that on Rockchip you don't need OPTEE to be selected for the > OP-TEE OS to be integrated in the U-Boot FIT image. You simply set the TEE > environment variable to the path to the bl32 binary and it's going to > include it in the U-Boot FIT. > > I think this is actually a good thing to be allowed to have the OP-TEE OS > image **without** the OPTEE driver being compiled: if I want OP-TEE OS to be > loaded in the system, this will be done by the SPL by loading the binary at > a specific location in RAM, but I may not want to increase the attack > surface by enabling the OP-TEE driver in U-Boot if I don't need to interact > with OP-TEE OS from U-Boot at all. > > The Aarch64 Rockchip SoCs also do not require OP-TEE OS to work (but I > **think** Aarch32 SoCs do?). "imply" in that case would not be enough > because we would still be able to include the OP-TEE OS image in the U-Boot > FIT without OPTEE driver compiled in (and thus OPTEE_LIB). "select" would be > too strong, because we really don't need OP-TEE OS (or rather its driver, > but the latter kind of implies the former) to be in the image in order for > the kernel to work just fine. > > It all boils down to the ability to add OP-TEE OS to U-Boot FIT via the TEE > environment variable without changing the defconfig (i.e. it's optional). We > could make the @tee-SEQ node in arch/arm/dts/rockchip-u-boot.dtsi presence > depend on OPTEE_LIB for example, but then we wouldn't be able to tell the > user that the TEE OS image is missing. Also, this would limit TEE OS to > OP-TEE OS (which I assume is probably what everybody is using on Rockchip > but we could be using something else technically?). That may be an > acceptable limitation though! > > As for forcing OPTEE_LIB on Aarch32 Rockchip SoCs on, this OPTEE_LIB thing > relies on OP-TEE OS patching the DTB passed to U-Boot proper so that the > reserved memory nodes for OP-TEE are made explicit. I somehow doubt this is > necessarily handled by OP-TEE blobs from Rockchip? Which means that I'm not > sure that switching the hardcoded implementation to use the logic in > OPTEE_LIB will be enough. > This also reminds me that TF-A blobs from Rockchip (used to?) crash for some > DTBs, which was worked around by simply just no passing the DTB to TF-A (see > SPL_ATF_NO_PLATFORM_PARAM), which I assume means OP-TEE OS doesn't have > access to a DTB so cannot patch it. I'm not sure how (if!) it is handled by > U-Boot in that case.
Getting back to this, thanks for detailing the explanation. I could not think of a more clever way to deal with the problem you're describing, so whatever you think looks sufficiently clean, please non-RFC and this is a case where a long commit message is likely good for future reference in case someone wants to clean this up. Thanks! -- Tom
signature.asc
Description: PGP signature
