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.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
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! -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
