Add driver probing also for external device tree use

[vesajaaskelainen]

When secure external device tree is configured for use OP-TEE's drivers
should be loaded based on its definitions. Add support to probe drivers
also with secure external device tree.

This allows common system device tree to be used to define devices for
bootloaders and OP-TEE.

In any case if embedded device tree is defined this will take precedense.

Signed-off-by: Vesa Jääskeläinen vesa.jaaskelainen@vaisala.com

[vesajaaskelainen]

This is split from PR #5350 as we could not re-open that.

Also decided to split the commits to own PRs.

[xiaoxuZeng]

May I ask a question？

In some case, fdt will be "external_dt.blob"，but in old codes that fdt will be NULL;
For example: if get_embedded_dt() return NULL, get_dt() maybe return "external_dt.blob".

I don’t' know what the impact will be.

[vesajaaskelainen]

I believe this is addressed in @etienne-lms's proposal.

There is slight logical change still.

Previously if embedded device tree usage was enabled but it was not actually present then there would be assertion failure. Now this continues the boot (and may fail later on).

I would leave this to firmware integrator to make sure that they have configured things properly.

[etienne-lms]

For this change, I think we should have a helper function like this, to ensure the DTB used to configure OP-TEE is secure:

void *get_secure_dt(void)
{
	void *fdt = get_embedded_dt();

	if (!fdt && IS_ENABLED(CFG_MAP_EXT_DT_SECURE))
		fdt = get_external_dt();

	return fdt;
}

[vesajaaskelainen]

For this change, I think we should have a helper function like this, to ensure the DTB used to configure OP-TEE is secure:

void *get_secure_dt(void)
{
	void *fdt = get_embedded_dt();

	if (!fdt && IS_ENABLED(CFG_MAP_EXT_DT_SECURE))
		fdt = get_external_dt();

	return fdt;
}

I can give this approach a spin.

[vesajaaskelainen]

I have made updates based on comments. Please check it out and see if it matches your ideas.

[vesajaaskelainen]

Verified that one of my devices boots with CFG_MAP_EXT_DT_SECURE=y enabled.

[vesajaaskelainen]

Will update commit subjects "kernel: " -> "core: " on next update round.

[vesajaaskelainen]

I have also similar change for clocks:

commit e78479b3b2e9cf52382e6cd5c339276cf3ed3bc1
Author: Vesa Jääskeläinen <vesa.jaaskelainen@vaisala.com>
Date:   Tue Feb 8 17:41:34 2022 +0200

    drivers: clk_dt: Switch to use get_secure_dt()
    
    This adds support for both embedded and external secure device trees so
    that clock driver and system configuration information can be fetched from
    there.
    
    Signed-off-by: Vesa Jääskeläinen <vesa.jaaskelainen@vaisala.com>

diff --git a/core/drivers/clk/clk_dt.c b/core/drivers/clk/clk_dt.c
index e90abf638..ff91902c5 100644
--- a/core/drivers/clk/clk_dt.c
+++ b/core/drivers/clk/clk_dt.c
@@ -197,7 +197,7 @@ static void clk_probe_assigned(const void *fdt, int parent_node)
 
 static TEE_Result clk_dt_probe(void)
 {
-       const void *fdt = get_embedded_dt();
+       const void *fdt = get_secure_dt();
 
        DMSG("Probing clocks from devicetree");
        if (!fdt)

Should I include this here as additional commit?

[vesajaaskelainen]

I have also similar change for clocks:
...
Should I include this here as additional commit?

Added it here. Can drop if you think it is better with other clock changes for zynq7k.

[etienne-lms]

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> for the 3 patches.
I think dtb_get_sdp_region() could use get_secure_dt().

[vesajaaskelainen]

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> for the 3 patches. I think dtb_get_sdp_region() could use get_secure_dt().

Thanks for the review. Applied tags.

About other changes.

$ git grep get_embedded_dt
core/arch/arm/kernel/boot.c:    void *fdt = get_embedded_dt();
core/arch/arm/kernel/boot.c:void *get_embedded_dt(void)
core/arch/arm/kernel/boot.c:void *get_embedded_dt(void)
core/arch/arm/plat-ls/main.c:   void *fdt = get_embedded_dt();
core/arch/arm/plat-sam/freq.c:  const void *fdt = get_embedded_dt();
core/arch/arm/plat-stm32mp1/drivers/stm32mp1_pmic.c:    void *fdt = get_embedded_dt();
core/arch/arm/plat-stm32mp1/drivers/stm32mp1_pmic.c:    fdt = get_embedded_dt();
core/arch/arm/plat-stm32mp1/drivers/stm32mp1_pmic.c:    fdt = get_embedded_dt();
core/arch/arm/plat-stm32mp1/drivers/stm32mp1_pmic.c:    fdt = get_embedded_dt();
core/arch/arm/plat-stm32mp1/main.c:     fdt = get_embedded_dt();
core/arch/arm/plat-stm32mp1/shared_resources.c: void *fdt = get_embedded_dt();
core/drivers/atmel_saic.c:      const void *fdt = get_embedded_dt();
core/drivers/clk/clk_dt.c:      const void *fdt = get_embedded_dt();
core/drivers/crypto/se050/glue/i2c_stm32.c:     fdt = get_embedded_dt();
core/drivers/ls_gpio.c: fdt = get_embedded_dt();
core/drivers/ls_i2c.c:  fdt = get_embedded_dt();
core/drivers/ls_sec_mon.c:      fdt = get_embedded_dt();
core/drivers/stm32_bsec.c:      fdt = get_embedded_dt();
core/drivers/stm32_etzpc.c:     void *fdt = get_embedded_dt();
core/drivers/stm32_rng.c:       fdt = get_embedded_dt();
core/include/kernel/boot.h:void *get_embedded_dt(void);
core/include/kernel/boot.h:     return fdt && fdt == get_embedded_dt();
core/kernel/dt_driver.c:        fdt = get_embedded_dt();
core/kernel/dt_driver.c:        fdt = get_embedded_dt();
core/kernel/dt_driver.c:        fdt = get_embedded_dt();
core/mm/core_mmu.c:     fdt = get_embedded_dt();

$ git grep "get_dt()"
core/drivers/crypto/caam/hal/common/hal_cfg.c:  fdt = get_dt();
core/drivers/imx/dcp/dcp.c:     fdt = get_dt();
core/drivers/imx_i2c.c: void *fdt = get_dt();
core/drivers/imx_rngb.c:        void *fdt = get_dt();
core/drivers/imx_wdog.c:        fdt = get_dt();
core/drivers/ls_dspi.c: fdt = get_dt();
core/kernel/console.c:  fdt = get_dt();

There are plenty of places where get_embedded_dt() or get_dt() are used and probably could be replaced with get_secure_dt().

About core/mm/core_mmu.c usage of get_embedded_dt() is protected by CFG_SECURE_DATA_PATH. And I am not using that feature. Kinda feel better if someone else would modify that part -- this also matches to other parts of the system.

I can of course to dummy modifications for all wanted parts but I cannot test most of them.

[etienne-lms]

I'm fine with the 3 changes as is.

There are plenty of places where get_embedded_dt() or get_dt() are used and probably could be replaced with get_secure_dt().

I looked at the generic place. You covered them, but the SDP part.
All others are platform specific drivers so let their maintainer change the implementation if of intereset.

About core/mm/core_mmu.c usage of get_embedded_dt() is protected by CFG_SECURE_DATA_PATH. And I am not using that feature. Kinda feel better if someone else would modify that part -- this also matches to other parts of the system.

Fair enough, let's not addressed it here.