[U-Boot] [RFC PATCH 3/3] spl: add support to booting with ATF

Michal Simek michal.simek at xilinx.com
Fri Jan 6 08:55:52 CET 2017


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.


> 
>>
>>
>>> + */
>>> +
>>> +#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

Thanks,
Michal





More information about the U-Boot mailing list