[U-Boot] [RFC PATCH 3/3] spl: add support to booting with ATF
Simon Glass
sjg at chromium.org
Thu Jan 26 15:23:40 CET 2017
+Tom for license comment
Hi,
On 6 January 2017 at 00:55, Michal Simek <michal.simek at 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 at 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.yang at 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
More information about the U-Boot
mailing list