+Tom for license comment Hi,
On 6 January 2017 at 00:55, Michal Simek <michal.si...@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.y...@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 to >>>> + load 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.y...@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. > > 0. 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. > > 1. If this ARM license is compatible with u-boot Licensing model > 2. 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: 1. Add to Licenses directory with a suitable identifier (see for example commit 0f4d2f8e) 2. Submit a new license registration request Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
