On Thu, Jan 26, 2017 at 07:23:40AM -0700, Simon Glass wrote:
+Tom for license comment
Hi,
On 6 January 2017 at 00:55, Michal Simek michal.simek@xilinx.com wrote:
Hi,
On 6.1.2017 08:09, Kever Yang wrote:
Hi Michal,
Thanks for your comments.On 01/02/2017 11:05 PM, Michal Simek wrote:
On 29.12.2016 11:25, Kever Yang wrote:
ATF(ARM Trust Firmware) is used by ARM arch64 SoCs, find more infomation about ATF at:
SPL is consider as BL2 in ATF, it needs to load other part of ATF binary
SPL replaces BL2 in ATF
OK, will follow your comment in next patch.
like BL31, BL32, SCP-BL30, and BL33(U-Boot). And needs to prepare the parameter for BL31 which including entry and image information for all other images. Then the SPL handle PC to BL31 with the parameter, the BL31 will do the rest of work and at last get into U-Boot(BL33).
But the main question for this is how do load that images and in which format. It means I would think that you will introduce fit format which contain BL33(U-Boot), BL32(secure os) and BL31(ATF) and SPL will be able to load all of them.
Yes, I use FIT format to contain BL33 and BL32 and SPL load all of them.
Do you have some logs? I didn't check the latest code but IIRC it was possible to handle one image and dt not several images which has to be supported. There is also loadables section in fit which can help with this.
If you look at zynqmp I did a small trick where I consider case that with ATF it is OS boot where kernel is ATF and dtb is full u-boot to get it boot.
This is a good idea, and it look fine for support ATF in SPL in local source code, but it will be better if we have an official support for ATF, right?
Definitely having support just for ATF is much better solution than what I use in ZynqMP.
If you adopt fit format then I expect SPL will be able to remember which part is where and based on that fill structure for ATF. Then SPL_ATF_TEXT_BASE address is not needed because it will be read from fit format.
Yes, you are right, SPL_ATF_TEXT_BASE is not a must, we gen get it from fit.
ok.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
common/spl/Kconfig | 14 +++ common/spl/Makefile | 1 + common/spl/spl.c | 4 + common/spl/spl_atf.c | 91 ++++++++++++++++ include/atf_common.h | 295 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/spl.h | 1 + 6 files changed, 406 insertions(+) create mode 100644 common/spl/spl_atf.c create mode 100644 include/atf_common.h
diff --git a/common/spl/Kconfig b/common/spl/Kconfig index cba51f5..1bb4360 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -577,6 +577,20 @@ config SPL_YMODEM_SUPPORT means of transmitting U-Boot over a serial line for using in SPL, with a checksum to ensure correctness. +config SPL_ATF_SUPPORT
- bool "Support ARM trust firmware"
- depends on SPL
- help
ATF(ARM Trust Firmware) is component for ARM arch64 which need toload by SPL(consider as BL2 in ATF).More detail at:https://github.com/ARM-software/arm-trusted-firmware
+config SPL_ATF_TEXT_BASE
- depends on SPL_ATF_SUPPORT
- hex "ATF TEXT BASE addr"
- help
This is the base address in memory for ATF text and entry point.- config TPL_ENV_SUPPORT bool "Support an environment" depends on TPL
diff --git a/common/spl/Makefile b/common/spl/Makefile index ed02635..620ae90 100644 --- a/common/spl/Makefile +++ b/common/spl/Makefile @@ -20,6 +20,7 @@ endif obj-$(CONFIG_SPL_UBI) += spl_ubi.o obj-$(CONFIG_SPL_NET_SUPPORT) += spl_net.o obj-$(CONFIG_SPL_MMC_SUPPORT) += spl_mmc.o +obj-$(CONFIG_SPL_ATF_SUPPORT) += spl_atf.o obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o obj-$(CONFIG_SPL_FAT_SUPPORT) += spl_fat.o obj-$(CONFIG_SPL_EXT_SUPPORT) += spl_ext.o diff --git a/common/spl/spl.c b/common/spl/spl.c index 1729034..7daf7bd 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -390,6 +390,10 @@ void board_init_r(gd_t *dummy1, ulong dummy2) gd->malloc_ptr / 1024); #endif +#ifdef CONFIG_SPL_ATF_SUPPORT
- bl31_entry();
+#endif
debug("loaded - jumping to U-Boot..."); spl_board_prepare_for_boot(); jump_to_image_no_args(&spl_image);diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c new file mode 100644 index 0000000..cf23b7a --- /dev/null +++ b/common/spl/spl_atf.c @@ -0,0 +1,91 @@ +/*
- Copyright (C) 2016 Rockchip Electronic Co.,Ltd
- Written by Kever Yang kever.yang@rock-chips.com
- origin from arm-trust-firmware
- plat/arm/common/arm_bl2_setup.c
- SPDX-License-Identifier: GPL-2.0+
this is not based on gpl file that's why license should be different.
Sorry, I do not get what your mean, I'm not good at license policy, ARM ATF use its own license, what should I do for license when I use code from ATF?
I am not that guy too. But if you took parts of code which is not GPL you can't label it as a GPL.
That's right. Perhaps rewrite and rename bl2_plat_get_bl31_params()? Then you can say that it is inspired by that file, and perhaps avoid the license problem.
The macros are horrible anyway.
- */
+#include <common.h> +#include <errno.h> +#include <spl.h> +#include <atf_common.h>
+static struct bl2_to_bl31_params_mem_t bl31_params_mem; +static struct bl31_params_t *bl2_to_bl31_params;
+/*******************************************************************************
- This function assigns a pointer to the memory that the platform
has kept
- aside to pass platform specific and trusted firmware related
information
- to BL31. This memory is allocated by allocating memory to
- bl2_to_bl31_params_mem_t structure which is a superset of all the
- structure whose information is passed to BL31
- NOTE: This function should be called only once and should be done
- before generating params to BL31
******************************************************************************/
+struct bl31_params_t *bl2_plat_get_bl31_params(void) +{
- struct entry_point_info_t *bl33_ep_info;
- /*
* Initialise the memory for all the arguments that needs to* be passed to BL31*/- memset(&bl31_params_mem, 0, sizeof(struct
bl2_to_bl31_params_mem_t));
- /* Assign memory for TF related information */
- bl2_to_bl31_params = &bl31_params_mem.bl31_params;
- SET_PARAM_HEAD(bl2_to_bl31_params, PARAM_BL31, VERSION_1, 0);
- /* Fill BL31 related information */
- SET_PARAM_HEAD(bl2_to_bl31_params->bl31_image_info,
PARAM_IMAGE_BINARY,
VERSION_1, 0);- /* Fill BL32 related information if it exists */
+#ifdef BL32_BASE
- bl2_to_bl31_params->bl32_ep_info = &bl31_params_mem.bl32_ep_info;
- SET_PARAM_HEAD(bl2_to_bl31_params->bl32_ep_info, PARAM_EP,
VERSION_1, 0);- bl2_to_bl31_params->bl32_image_info =
&bl31_params_mem.bl32_image_info;
- SET_PARAM_HEAD(bl2_to_bl31_params->bl32_image_info,
PARAM_IMAGE_BINARY,
VERSION_1, 0);+#endif /* BL32_BASE */
Is this used?
Not use for me now, but it may useful later, because we need to fill info about bl32 if there is.
I think that make sense to look at fit format and try to integrate all these stuff together.
- /* Fill BL33 related information */
- bl2_to_bl31_params->bl33_ep_info = &bl31_params_mem.bl33_ep_info;
- bl33_ep_info = &bl31_params_mem.bl33_ep_info;
- SET_PARAM_HEAD(bl33_ep_info, PARAM_EP, VERSION_1, EP_NON_SECURE);
- /* BL33 expects to receive the primary CPU MPID (through x0) */
- bl33_ep_info->args.arg0 = 0xffff & read_mpidr();
- bl33_ep_info->pc = CONFIG_SYS_TEXT_BASE;
- bl33_ep_info->spsr = SPSR_64(MODE_EL2, MODE_SP_ELX,
DISABLE_ALL_EXECPTIONS);- bl2_to_bl31_params->bl33_image_info =
&bl31_params_mem.bl33_image_info;
- SET_PARAM_HEAD(bl2_to_bl31_params->bl33_image_info,
PARAM_IMAGE_BINARY,
VERSION_1, 0);double lines.
Will fix in next patch, confuse why checkpatch did not find this.
- return bl2_to_bl31_params;
+}
+void raw_write_daif(uint32_t daif) +{
- __asm__ __volatile__("msr DAIF, %0\n\t" : : "r" (daif) : "memory");
+}
+void bl31_entry(void) +{
- struct bl31_params_t *bl31_params;
- void (*entry)(struct bl31_params_t *params, void *plat_params) =
NULL;
- bl31_params = bl2_plat_get_bl31_params();
- entry = (void *)CONFIG_SPL_ATF_TEXT_BASE;
- raw_write_daif(SPSR_EXCEPTION_MASK);
- dcache_disable();
- entry(bl31_params, NULL);
+} diff --git a/include/atf_common.h b/include/atf_common.h new file mode 100644 index 0000000..8351302 --- /dev/null +++ b/include/atf_common.h @@ -0,0 +1,295 @@ +/*
- Copyright (c) 2013-2016, ARM Limited and Contributors. All rights
reserved.
- Redistribution and use in source and binary forms, with or without
- modification, are permitted provided that the following
conditions are met:
- Redistributions of source code must retain the above copyright
notice, this
- list of conditions and the following disclaimer.
- Redistributions in binary form must reproduce the above copyright
notice,
- this list of conditions and the following disclaimer in the
documentation
- and/or other materials provided with the distribution.
- Neither the name of ARM nor the names of its contributors may be
used
- to endorse or promote products derived from this software without
specific
- prior written permission.
- THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
CONTRIBUTORS "AS IS"
- AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
TO, THE
- IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
PARTICULAR PURPOSE
- ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
CONTRIBUTORS BE
- LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
BUSINESS
- INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
WHETHER IN
- CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
OTHERWISE)
- ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
ADVISED OF THE
- POSSIBILITY OF SUCH DAMAGE.
This should be probably in SPDX format.
Like previous reply, I don't know how to deal with this license, do you mean I can use SPDX license without any information about the origin License?
Tom, Simon: Please correct me if I am wrong. There are 2 things here.
- Identify license and especially this part. The rest looks like BSD
license
- Neither the name of ARM nor the names of its contributors may be used
- to endorse or promote products derived from this software without specific
- prior written permission.
- If this ARM license is compatible with u-boot Licensing model
- We have Licenses folder where License can go but not sure if only
SPDX licenses can got there and if ARM published this license there to have official SPDX tag. Please look at Licenses/README
Perhaps we need to get this licence registered with SPDX?
https://spdx.org/spdx-license-list/request-new-license
But it would be better to get that piece removed somehow. It makes a non-standard license. Failing that, add a new licence:
- Add to Licenses directory with a suitable identifier (see for
example commit 0f4d2f8e) 2. Submit a new license registration request
For both license questions, we must be correct. The copy of ATF that I just looked at has a 3 clause BSD license, with the 3rd clause worded perhaps oddly, but, that looks right for 3-clause BSD. This is compatible with U-Boot. If on the other hand ATF has some data structures/etc that non-ATF code really needs to know and it is not BSD-3-Clause, we should perhaps try and contact someone with the authority to re-license it, using include/android_image.h as an example of where U-Boot hosts, officially, this kind of information in a more permissible license. Thanks!