Hi Andrew,
On 14 November 2016 at 12:49, Andrew F. Davis <a...@ti.com> wrote:
> To help automate the loading of a TEE image during the boot we add a new
> FIT section type 'tee', when we see this type while loading the loadable
> sections we automatically call the platforms TEE processing function on
> this image section.
>
> Signed-off-by: Andrew F. Davis <a...@ti.com>
> ---
> Kconfig | 10 ++++++++++
> common/image.c | 18 ++++++++++++++++++
> include/image.h | 15 +++++++++++++++
> 3 files changed, 43 insertions(+)
>
> diff --git a/Kconfig b/Kconfig
> index 1263d0b..97cf7c8 100644
> --- a/Kconfig
> +++ b/Kconfig
> @@ -291,6 +291,16 @@ config FIT_IMAGE_POST_PROCESS
> injected into the FIT creation (i.e. the blobs would have been pre-
> processed before being added to the FIT image).
>
> +config FIT_IMAGE_TEE_PROCESS
> + bool "Enable processing of TEE images during FIT loading by U-Boot"
> + depends on FIT && TI_SECURE_DEVICE
This is a generic option so I don't think it should depend on TI.
> + help
> + Allows platforms to perform processing, such as authentication and
> + installation, on TEE images extracted from FIT images in a platform
> + or board specific way. In order to use this feature a platform or
> + board-specific implementation of board_tee_image_process() must be
> + provided.
> +
> config SPL_DFU_SUPPORT
> bool "Enable SPL with DFU to load binaries to memory device"
> depends on USB
> diff --git a/common/image.c b/common/image.c
> index 7604494..4552ca5 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -165,6 +165,7 @@ static const table_entry_t uimage_type[] = {
> { IH_TYPE_ZYNQIMAGE, "zynqimage", "Xilinx Zynq Boot Image" },
> { IH_TYPE_ZYNQMPIMAGE, "zynqmpimage", "Xilinx ZynqMP Boot
> Image" },
> { IH_TYPE_FPGA, "fpga", "FPGA Image" },
> + { IH_TYPE_TEE, "tee", "TEE OS Image",},
Perhaps write out TEE in full? It's a bit cryptic.
> { -1, "", "", },
> };
>
> @@ -1408,6 +1409,8 @@ int boot_get_loadable(int argc, char * const argv[],
> bootm_headers_t *images,
> int fit_img_result;
> const char *uname;
>
> + uint8_t img_type;
> +
> /* Check to see if the images struct has a FIT configuration */
> if (!genimg_has_config(images)) {
> debug("## FIT configuration was not specified\n");
> @@ -1447,6 +1450,21 @@ int boot_get_loadable(int argc, char * const argv[],
> bootm_headers_t *images,
> /* Something went wrong! */
> return fit_img_result;
> }
> +
> + fit_img_result = fit_image_get_node(buf, uname);
> + if (fit_img_result < 0) {
> + /* Something went wrong! */
> + return fit_img_result;
> + }
> + fit_img_result = fit_image_get_type(buf,
> fit_img_result, &img_type);
> + if (fit_img_result < 0) {
> + /* Something went wrong! */
> + return fit_img_result;
> + }
> +#if defined(CONFIG_FIT_IMAGE_TEE_PROCESS)
> + if (img_type == IH_TYPE_TEE)
> + board_tee_image_process(img_data, img_len);
> +#endif
Instead of putting this here, I think it would be better for
boot_get_loadable() to return the correct values for ld_start and
ld_len. Perhaps you need to pass it the loadable index to load, so it
is called multiple times? The only caller is bootm_find_images().
It is too ugly, I think, to check the image type in the 'load'
function, and do special things.
> }
> break;
> default:
> diff --git a/include/image.h b/include/image.h
> index 2b1296c..57084c8 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -279,6 +279,7 @@ enum {
> IH_TYPE_ZYNQMPIMAGE, /* Xilinx ZynqMP Boot Image */
> IH_TYPE_FPGA, /* FPGA Image */
> IH_TYPE_VYBRIDIMAGE, /* VYBRID .vyb Image */
> + IH_TYPE_TEE, /* Trusted Execution Environment OS Image */
>
> IH_TYPE_COUNT, /* Number of image types */
> };
> @@ -1263,4 +1264,18 @@ int board_fit_config_name_match(const char *name);
> void board_fit_image_post_process(void **p_image, size_t *p_size);
> #endif /* CONFIG_SPL_FIT_IMAGE_POST_PROCESS */
>
> +#ifdef CONFIG_FIT_IMAGE_TEE_PROCESS
I don't think you should have this #ifdef in the header file.
> +/**
> + * board_fit_tee_process() - Do any needed processing on a loaded TEE image
> + *
> + * This is used to verify, decrypt, and/or install a TEE in a platform or
> + * board specific way.
nit: board-specific
> + *
> + * @tee_image: pointer to the image
What format is the image?
> + * @tee_size: the image size
> + * @return no return value (failure should be handled internally)
> + */
> +void board_tee_image_process(void *tee_image, size_t tee_size);
I think it's a good idea to return an error code here, since the
function may fail.
> +#endif /* CONFIG_FIT_IMAGE_TEE_PROCESS */
> +
> #endif /* __IMAGE_H__ */
> --
> 2.10.1
>
Regards,
SImon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
