[U-Boot] [PATCHv6 2/6] ARMv8: add the secure monitor firmware framework

[york sun]

On 06/21/2016 08:42 PM, Zhiqiang Hou wrote:
> From: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
>
> This framework is introduced for ARMv8 secure monitor mode firmware.
> The main functions of the framework are, on EL3, verify the firmware,
> load it to the secure memory and jump into it, and while it returned
> to U-Boot, do some necessary setups at the 'target exception level'
> that is determined by the respective secure firmware.
>
> So far, the framework support only FIT format image, and need to define
> the name of which config node should be used in 'configurations' and
> the name of property for the raw secure firmware image in that config.
> The FIT image should be stored in Byte accessing memory, such as NOR
> Flash, or else it should be copied to main memory to use this framework.
>
> Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou at nxp.com>
> ---
> V6:
>   - Abstracted more code from PPA to this framework.
>   - Introduced gd->sec_firmware to hold the load address.
>   - Refactor the func sec_firmware_support_psci_version().

A lot of change in this version.

>
> V5:
>   - Added c file sec_firmware.c.
>   - Added declaration of sec_firmware_init().
>   - Renamed the func sec_firmware_validate().
>
> V4:
>   - Reordered this patch.
>   - Removed the FSL PPA related items.
>
>   arch/arm/cpu/armv8/Makefile               |   1 +
>   arch/arm/cpu/armv8/sec_firmware.c         | 262 ++++++++++++++++++++++++++++++
>   arch/arm/cpu/armv8/sec_firmware_asm.S     |  53 ++++++
>   arch/arm/include/asm/armv8/sec_firmware.h |  18 ++
>   include/asm-generic/global_data.h         |  11 ++
>   5 files changed, 346 insertions(+)
>   create mode 100644 arch/arm/cpu/armv8/sec_firmware.c
>   create mode 100644 arch/arm/cpu/armv8/sec_firmware_asm.S
>   create mode 100644 arch/arm/include/asm/armv8/sec_firmware.h
>
> diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
> index bf8644c..ee9e009 100644
> --- a/arch/arm/cpu/armv8/Makefile
> +++ b/arch/arm/cpu/armv8/Makefile
> @@ -15,6 +15,7 @@ obj-y	+= cache.o
>   obj-y	+= tlb.o
>   obj-y	+= transition.o
>   obj-y	+= fwcall.o
> +obj-$(CONFIG_ARMV8_SEC_FIRMWARE_SUPPORT) += sec_firmware.o sec_firmware_asm.o
>
>   obj-$(CONFIG_FSL_LAYERSCAPE) += fsl-layerscape/
>   obj-$(CONFIG_S32V234) += s32v234/
> diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
> new file mode 100644
> index 0000000..986df48
> --- /dev/null
> +++ b/arch/arm/cpu/armv8/sec_firmware.c
> @@ -0,0 +1,266 @@
> +/*
> + * Copyright 2016 NXP Semiconductor, Inc.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <errno.h>
> +#include <linux/kernel.h>
> +#include <asm/io.h>
> +#include <asm/system.h>
> +#include <asm/types.h>
> +#include <asm/macro.h>
> +#include <asm/armv8/sec_firmware.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +extern void c_runtime_cpu_setup(void);
> +
> +static int sec_firmware_get_data(void *sec_firmware_img,
> +				const void **data, size_t *size)

Throughout this patch, sec_firmware_img is used as read-only. How about 
add "const" to it?

> +{
> +	void *fit_hdr;

Variable fit_hdr doesn't serve more purpose. You can use 
sec_firmware_img directly.


> +	int conf_node_off, fw_node_off;
> +	char *conf_node_name = NULL;
> +	char *desc;
> +	int ret;
> +
> +	fit_hdr = sec_firmware_img;
> +	conf_node_name = SEC_FIRMEWARE_FIT_CNF_NAME;
> +
> +	conf_node_off = fit_conf_get_node(fit_hdr, conf_node_name);
> +	if (conf_node_off < 0) {
> +		printf("SEC Firmware: %s: no such config\n", conf_node_name);
> +		return -ENOENT;
> +	}
> +
> +	fw_node_off = fit_conf_get_prop_node(fit_hdr, conf_node_off,
> +			SEC_FIRMWARE_FIT_IMAGE);
> +	if (fw_node_off < 0) {
> +		printf("SEC Firmware: No '%s' in config\n",
> +				SEC_FIRMWARE_FIT_IMAGE);

You have many of this alignment issues throughout this patch.

<snip>

York