[U-Boot] [RFC PATCH 3/3] spl: add support to booting with ATF
Tom Rini
trini at konsulko.com
Sat Jan 28 22:55:52 CET 2017
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 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
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170128/b4b9927a/attachment.sig>
More information about the U-Boot
mailing list