[PATCH 0/3] imx8m: Automatically add the optee firmware node to the FDT
Hi everyone,
this short series is about letting u-boot automatically add the optee firmware node to the DTB on imx8m. The imx8 already does that using its own function and the optee lib has a very similar function which copy the DT node from u-boot DT.
So this series start by adding a new helper to the optee lib to add the optee firmware node with the provided parameters. This function is then used to add this functionality to the imx8m boards and to replace the custom code used on the imx8 boards.
Alban Bedel (3): optee: Add an helper to add the optee firmware node in the FDT imx8m: Automatically add the optee firmware node to the FDT imx8: Use the optee lib to add the optee firmware node to the FDT
arch/arm/mach-imx/imx8/fdt.c | 59 ++++--------------------- arch/arm/mach-imx/imx8m/soc.c | 9 ++++ configs/apalis-imx8_defconfig | 1 + configs/cgtqmx8_defconfig | 1 + configs/colibri-imx8x_defconfig | 1 + configs/deneb_defconfig | 1 + configs/giedi_defconfig | 1 + configs/imx8qm_mek_defconfig | 1 + configs/imx8qm_rom7720_a1_4G_defconfig | 1 + configs/imx8qxp_mek_defconfig | 1 + include/tee/optee.h | 11 +++++ lib/optee/optee.c | 60 +++++++++++++++++++------- 12 files changed, 80 insertions(+), 67 deletions(-)
Some platforms can detect if an optee firmware is running and then manually add the firmware node to the FDT. Provide a common helper to avoid code duplication in the board code.
Signed-off-by: Alban Bedel alban.bedel@aerq.com --- include/tee/optee.h | 11 +++++++++ lib/optee/optee.c | 60 +++++++++++++++++++++++++++++++++------------ 2 files changed, 55 insertions(+), 16 deletions(-)
diff --git a/include/tee/optee.h b/include/tee/optee.h index 5412bc7386ec..d9a316150522 100644 --- a/include/tee/optee.h +++ b/include/tee/optee.h @@ -58,11 +58,22 @@ static inline int optee_verify_bootm_image(unsigned long image_addr,
#if defined(CONFIG_OPTEE_LIB) && defined(CONFIG_OF_LIBFDT) int optee_copy_fdt_nodes(void *new_blob); + +int optee_add_firmware_node(void *fdt_blob, + const char *compatible, + const char *method); #else static inline int optee_copy_fdt_nodes(void *new_blob) { return 0; } + +static inline int optee_add_firmware_node(void *fdt_blob, + const char *compatible, + const char *method) +{ + return 0; +} #endif
#endif /* _OPTEE_H */ diff --git a/lib/optee/optee.c b/lib/optee/optee.c index b03622404469..8d4e110dfd04 100644 --- a/lib/optee/optee.c +++ b/lib/optee/optee.c @@ -65,10 +65,10 @@ error: #endif
#if defined(CONFIG_OF_LIBFDT) -static int optee_copy_firmware_node(ofnode node, void *fdt_blob) +static int optee_set_firmware_node(void *fdt_blob, const char *compatible, + const char *method) { - int offs, ret, len; - const void *prop; + int offs, ret;
offs = fdt_path_offset(fdt_blob, "/firmware"); if (offs < 0) { @@ -85,29 +85,32 @@ static int optee_copy_firmware_node(ofnode node, void *fdt_blob) if (offs < 0) return offs;
+ ret = fdt_setprop_string(fdt_blob, offs, "compatible", compatible); + if (ret < 0) + return ret; + + return fdt_setprop_string(fdt_blob, offs, "method", method); +} + +static int optee_copy_firmware_node(ofnode node, void *fdt_blob) +{ + const char *compatible, *method; + /* copy the compatible property */ - prop = ofnode_get_property(node, "compatible", &len); - if (!prop) { + compatible = ofnode_read_string(node, "compatible"); + if (!compatible) { debug("missing OP-TEE compatible property"); return -EINVAL; }
- ret = fdt_setprop(fdt_blob, offs, "compatible", prop, len); - if (ret < 0) - return ret; - /* copy the method property */ - prop = ofnode_get_property(node, "method", &len); - if (!prop) { + method = ofnode_read_string(node, "method"); + if (!method) { debug("missing OP-TEE method property"); return -EINVAL; }
- ret = fdt_setprop(fdt_blob, offs, "method", prop, len); - if (ret < 0) - return ret; - - return 0; + return optee_set_firmware_node(fdt_blob, compatible, method); }
int optee_copy_fdt_nodes(void *new_blob) @@ -190,4 +193,29 @@ int optee_copy_fdt_nodes(void *new_blob)
return 0; } + +int optee_add_firmware_node(void *fdt_blob, const char *compatible, + const char *method) +{ + int ret; + + /* + * Do not proceed if the target dt already has an OP-TEE node. + * In this case assume that the system knows better somehow, + * so do not interfere. + */ + if (fdt_check_header(fdt_blob)) + return -EINVAL; + + if (fdt_path_offset(fdt_blob, "/firmware/optee") >= 0) { + debug("OP-TEE Device Tree node already exists in target"); + return 0; + } + + ret = fdt_increase_size(fdt_blob, 512); + if (ret) + return ret; + + return optee_set_firmware_node(fdt_blob, compatible, method); +} #endif
If optee is running add the firmware node to the FDT to allow the kernel to use a more generic device tree.
Signed-off-by: Alban Bedel alban.bedel@aerq.com --- arch/arm/mach-imx/imx8m/soc.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 8e23e6da326f..2a78cb6a0952 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -30,6 +30,7 @@ #include <fsl_wdog.h> #include <imx_sip.h> #include <linux/bitops.h> +#include <tee/optee.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -1347,6 +1348,14 @@ usb_modify_speed: #endif
cleanup_nodes_for_efi(blob); + + if (rom_pointer[1]) { + int err = optee_add_firmware_node(blob, "linaro,optee-tz", + "smc"); + if (err) + return err; + } + return 0; } #endif
Hi Alban
On Mon, May 16, 2022 at 10:23 AM Alban Bedel alban.bedel@aerq.com wrote:
If optee is running add the firmware node to the FDT to allow the kernel to use a more generic device tree.
Signed-off-by: Alban Bedel alban.bedel@aerq.com
arch/arm/mach-imx/imx8m/soc.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 8e23e6da326f..2a78cb6a0952 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -30,6 +30,7 @@ #include <fsl_wdog.h> #include <imx_sip.h> #include <linux/bitops.h> +#include <tee/optee.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -1347,6 +1348,14 @@ usb_modify_speed: #endif
cleanup_nodes_for_efi(blob);
if (rom_pointer[1]) {int err = optee_add_firmware_node(blob, "linaro,optee-tz","smc");if (err)return err;}return 0;}
#endif
Adding the node is not sufficient. I think that we need reserved memory node on top of it. The nice scenario will be only to define the size of such area and use always the last part of the memory to reserve it. Do you know if we have some api to pass information to the atf and optee?
Michael
2.34.1
Hi,
On Mon, May 16, 2022 at 12:00 PM Michael Nazzareno Trimarchi michael@amarulasolutions.com wrote:
Hi Alban
On Mon, May 16, 2022 at 10:23 AM Alban Bedel alban.bedel@aerq.com wrote:
If optee is running add the firmware node to the FDT to allow the kernel to use a more generic device tree.
Signed-off-by: Alban Bedel alban.bedel@aerq.com
arch/arm/mach-imx/imx8m/soc.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 8e23e6da326f..2a78cb6a0952 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -30,6 +30,7 @@ #include <fsl_wdog.h> #include <imx_sip.h> #include <linux/bitops.h> +#include <tee/optee.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -1347,6 +1348,14 @@ usb_modify_speed: #endif
cleanup_nodes_for_efi(blob);
if (rom_pointer[1]) {int err = optee_add_firmware_node(blob, "linaro,optee-tz","smc");if (err)return err;}return 0;}
#endif
Adding the node is not sufficient. I think that we need reserved memory node on top of it. The nice scenario will be only to define the size of such area and use always the last part of the memory to reserve it. Do you know if we have some api to pass information to the atf and optee?
We have an ABI in OP-TEE for this, OPTEE_SMC_GET_SHM_CONFIG. This function wasn't intended for this, but I can't see that that should be a problem.
Cheers, Jens
Michael
2.34.1
-- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 michael@amarulasolutions.com __________________________________
Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 info@amarulasolutions.com www.amarulasolutions.com
Hi Jens
On Mon, May 16, 2022 at 3:49 PM Jens Wiklander jens.wiklander@linaro.org wrote:
Hi,
On Mon, May 16, 2022 at 12:00 PM Michael Nazzareno Trimarchi michael@amarulasolutions.com wrote:
Hi Alban
On Mon, May 16, 2022 at 10:23 AM Alban Bedel alban.bedel@aerq.com wrote:
If optee is running add the firmware node to the FDT to allow the kernel to use a more generic device tree.
Signed-off-by: Alban Bedel alban.bedel@aerq.com
arch/arm/mach-imx/imx8m/soc.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 8e23e6da326f..2a78cb6a0952 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -30,6 +30,7 @@ #include <fsl_wdog.h> #include <imx_sip.h> #include <linux/bitops.h> +#include <tee/optee.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -1347,6 +1348,14 @@ usb_modify_speed: #endif
cleanup_nodes_for_efi(blob);
if (rom_pointer[1]) {int err = optee_add_firmware_node(blob, "linaro,optee-tz","smc");if (err)return err;}return 0;}
#endif
Adding the node is not sufficient. I think that we need reserved memory node on top of it. The nice scenario will be only to define the size of such area and use always the last part of the memory to reserve it. Do you know if we have some api to pass information to the atf and optee?
We have an ABI in OP-TEE for this, OPTEE_SMC_GET_SHM_CONFIG. This function wasn't intended for this, but I can't see that that should be a problem.
Ok, the call resolves to know the size of SHM_CONFIG but does not give information about the memory to be reserved.
So SPL loads ATF that has hardcoded BL32_ADDR and BL32_SIZE. SPL is running in OCRAM as ATF, so then I can call the OPTEE_SMC_GET to know what reservation OPTEE needs. This avoid us to create dts entry for optee only
Michael
Cheers, Jens
Michael
2.34.1
-- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 michael@amarulasolutions.com __________________________________
Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 info@amarulasolutions.com www.amarulasolutions.com
-- Michael Nazzareno Trimarchi Co-Founder & Chief Executive Officer M. +39 347 913 2170 michael@amarulasolutions.com __________________________________
Amarula Solutions BV Joop Geesinkweg 125, 1114 AB, Amsterdam, NL T. +31 (0)85 111 9172 info@amarulasolutions.com www.amarulasolutions.com
On Mon, 2022-05-16 at 12:00 +0200, Michael Nazzareno Trimarchi wrote:
Hi Alban
On Mon, May 16, 2022 at 10:23 AM Alban Bedel alban.bedel@aerq.com wrote:
If optee is running add the firmware node to the FDT to allow the kernel to use a more generic device tree.
Signed-off-by: Alban Bedel alban.bedel@aerq.com
arch/arm/mach-imx/imx8m/soc.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach- imx/imx8m/soc.c index 8e23e6da326f..2a78cb6a0952 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -30,6 +30,7 @@ #include <fsl_wdog.h> #include <imx_sip.h> #include <linux/bitops.h> +#include <tee/optee.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -1347,6 +1348,14 @@ usb_modify_speed: #endif
cleanup_nodes_for_efi(blob);
+ if (rom_pointer[1]) { + int err = optee_add_firmware_node(blob, "linaro,optee-tz", + "smc"); + if (err) + return err; + }
return 0; } #endif --
Adding the node is not sufficient. I think that we need reserved memory node on top of it.
My board use an imx8m and there the SoC code already carve out the optee memory (arch/arm/mach-imx/imx8m/soc.c dram_init_banksize()). In that case it is enough to just add the firmware node as the memory node created in the DTB already exclude the optee area. I expect it was done like this to also prevent using this memory area in u-boot.
This is a first step we can later add another function to also create the reserved memory nodes when needed.
The nice scenario will be only to define the size of such area and use always the last part of the memory to reserve it.
The optee area might be anywhere, the size won't be enough.
Alban
Use the function provided by the optee lib to add the firmware node to the FDT. Enable the optee lib on all imx8 defconfig to keep the same behavior.
Signed-off-by: Alban Bedel alban.bedel@aerq.com --- arch/arm/mach-imx/imx8/fdt.c | 59 ++++---------------------- configs/apalis-imx8_defconfig | 1 + configs/cgtqmx8_defconfig | 1 + configs/colibri-imx8x_defconfig | 1 + configs/deneb_defconfig | 1 + configs/giedi_defconfig | 1 + configs/imx8qm_mek_defconfig | 1 + configs/imx8qm_rom7720_a1_4G_defconfig | 1 + configs/imx8qxp_mek_defconfig | 1 + 9 files changed, 16 insertions(+), 51 deletions(-)
diff --git a/arch/arm/mach-imx/imx8/fdt.c b/arch/arm/mach-imx/imx8/fdt.c index a132ce2e6a3a..60d6a6bb85b8 100644 --- a/arch/arm/mach-imx/imx8/fdt.c +++ b/arch/arm/mach-imx/imx8/fdt.c @@ -11,6 +11,7 @@ #include <dm/ofnode.h> #include <fdt_support.h> #include <linux/libfdt.h> +#include <tee/optee.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -229,56 +230,6 @@ static int config_smmu_fdt(void *blob) return 0; }
-static int ft_add_optee_node(void *fdt, struct bd_info *bd) -{ - const char *path, *subpath; - int offs; - - /* - * No TEE space allocated indicating no TEE running, so no - * need to add optee node in dts - */ - if (!boot_pointer[1]) - return 0; - - offs = fdt_increase_size(fdt, 512); - if (offs) { - printf("No Space for dtb\n"); - return 1; - } - - path = "/firmware"; - offs = fdt_path_offset(fdt, path); - if (offs < 0) { - path = "/"; - offs = fdt_path_offset(fdt, path); - - if (offs < 0) { - printf("Could not find root node.\n"); - return offs; - } - - subpath = "firmware"; - offs = fdt_add_subnode(fdt, offs, subpath); - if (offs < 0) { - printf("Could not create %s node.\n", subpath); - return offs; - } - } - - subpath = "optee"; - offs = fdt_add_subnode(fdt, offs, subpath); - if (offs < 0) { - printf("Could not create %s node.\n", subpath); - return offs; - } - - fdt_setprop_string(fdt, offs, "compatible", "linaro,optee-tz"); - fdt_setprop_string(fdt, offs, "method", "smc"); - - return 0; -} - int ft_system_setup(void *blob, struct bd_info *bd) { int ret; @@ -300,5 +251,11 @@ int ft_system_setup(void *blob, struct bd_info *bd) return ret; }
- return ft_add_optee_node(blob, bd); + if (boot_pointer[1]) { + ret = optee_add_firmware_node(blob, "linaro,optee-tz", "smc"); + if (ret) + return ret; + } + + return 0; } diff --git a/configs/apalis-imx8_defconfig b/configs/apalis-imx8_defconfig index beb20f6e1c01..f3dbb4bd9c19 100644 --- a/configs/apalis-imx8_defconfig +++ b/configs/apalis-imx8_defconfig @@ -72,3 +72,4 @@ CONFIG_FSL_LPUART=y CONFIG_DM_THERMAL=y CONFIG_IMX_SCU_THERMAL=y # CONFIG_EFI_LOADER is not set +CONFIG_OPTEE_LIB=y diff --git a/configs/cgtqmx8_defconfig b/configs/cgtqmx8_defconfig index 2cf882f826ab..ac251f37fed5 100644 --- a/configs/cgtqmx8_defconfig +++ b/configs/cgtqmx8_defconfig @@ -90,3 +90,4 @@ CONFIG_DM_SERIAL=y CONFIG_FSL_LPUART=y CONFIG_SPL_TINY_MEMSET=y # CONFIG_EFI_LOADER is not set +CONFIG_OPTEE_LIB=y diff --git a/configs/colibri-imx8x_defconfig b/configs/colibri-imx8x_defconfig index 0c9d6b64c1b6..f37da2411b04 100644 --- a/configs/colibri-imx8x_defconfig +++ b/configs/colibri-imx8x_defconfig @@ -70,3 +70,4 @@ CONFIG_FSL_LPUART=y CONFIG_DM_THERMAL=y CONFIG_IMX_SCU_THERMAL=y # CONFIG_EFI_LOADER is not set +CONFIG_OPTEE_LIB=y diff --git a/configs/deneb_defconfig b/configs/deneb_defconfig index 425fff6c70a6..e8f8cd18cacf 100644 --- a/configs/deneb_defconfig +++ b/configs/deneb_defconfig @@ -114,3 +114,4 @@ CONFIG_IMX_SCU_THERMAL=y # CONFIG_SPL_WDT is not set CONFIG_SPL_TINY_MEMSET=y # CONFIG_EFI_LOADER is not set +CONFIG_OPTEE_LIB=y diff --git a/configs/giedi_defconfig b/configs/giedi_defconfig index 4fbf7ebdcd95..7ee50e1f45c5 100644 --- a/configs/giedi_defconfig +++ b/configs/giedi_defconfig @@ -114,3 +114,4 @@ CONFIG_IMX_SCU_THERMAL=y # CONFIG_SPL_WDT is not set CONFIG_SPL_TINY_MEMSET=y # CONFIG_EFI_LOADER is not set +CONFIG_OPTEE_LIB=y diff --git a/configs/imx8qm_mek_defconfig b/configs/imx8qm_mek_defconfig index 2e42872f843d..10b61d62a821 100644 --- a/configs/imx8qm_mek_defconfig +++ b/configs/imx8qm_mek_defconfig @@ -91,3 +91,4 @@ CONFIG_DM_SERIAL=y CONFIG_FSL_LPUART=y CONFIG_SPL_TINY_MEMSET=y # CONFIG_EFI_LOADER is not set +CONFIG_OPTEE_LIB=y diff --git a/configs/imx8qm_rom7720_a1_4G_defconfig b/configs/imx8qm_rom7720_a1_4G_defconfig index d9997cfa8280..dc94fc67261c 100644 --- a/configs/imx8qm_rom7720_a1_4G_defconfig +++ b/configs/imx8qm_rom7720_a1_4G_defconfig @@ -86,3 +86,4 @@ CONFIG_DM_SERIAL=y CONFIG_FSL_LPUART=y CONFIG_SPL_TINY_MEMSET=y # CONFIG_EFI_LOADER is not set +CONFIG_OPTEE_LIB=y diff --git a/configs/imx8qxp_mek_defconfig b/configs/imx8qxp_mek_defconfig index 43f42f7a58af..250ce43758cf 100644 --- a/configs/imx8qxp_mek_defconfig +++ b/configs/imx8qxp_mek_defconfig @@ -94,3 +94,4 @@ CONFIG_DM_THERMAL=y CONFIG_IMX_SCU_THERMAL=y CONFIG_SPL_TINY_MEMSET=y # CONFIG_EFI_LOADER is not set +CONFIG_OPTEE_LIB=y
participants (4)
-
Alban Bedel -
Bedel, Alban
-
Jens Wiklander -
Michael Nazzareno Trimarchi