[U-Boot] [PATCH 07/15] spl: atf: introduce spl_invoke_atf and make bl31_entry private

[Dr. Philipp Tomsich]

> On 17 Sep 2017, at 19:53, Simon Glass <sjg at chromium.org> wrote:
> 
> Hi Philipp,
> 
> On 13 September 2017 at 13:29, Philipp Tomsich
> <philipp.tomsich at theobroma-systems.com <mailto:philipp.tomsich at theobroma-systems.com>> wrote:
>> This adds a new interface spl_invoke_atf() that takes a spl_image_info
>> argument and then derives the necessary parameters for the ATF entry.
>> Based on the additional information recorded (into /fit-images) from
>> the FIT loadables, we can now easily locate the next boot stage.
>> 
>> We now pass a pointer to a FDT as the platform-specific parameter
>> pointer to ATF (so we don't run into the future headache of every
>> board/platform defining their own proprietary tag-structure), as
>> FDT access is already available in ATF.
>> 
>> With the necessary infrastructure in place, we can now update the
>> support for the ARM Trusted Firmware to dispatch into the
>> spl_invoke_atf function only if a IH_OS_ARM_TRUSTED_FIRMWARE image is
>> loaded.
>> 
>> Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
>> ---
>> 
>> common/spl/spl.c     | 11 +++----
>> common/spl/spl_atf.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++-----
>> 2 files changed, 82 insertions(+), 13 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg at chromium.org <mailto:sjg at chromium.org>>
> 
> Please see question below
> 
> [..]
> 
>> index 6e8f928..63557c0 100644
>> --- a/common/spl/spl_atf.c
>> +++ b/common/spl/spl_atf.c
>> @@ -5,6 +5,7 @@
>>  * reserved.
>>  * Copyright (C) 2016 Rockchip Electronic Co.,Ltd
>>  * Written by Kever Yang <kever.yang at rock-chips.com <mailto:kever.yang at rock-chips.com>>
>> + * Copyright (C) 2017 Theobroma Systems Design und Consulting GmbH
>>  *
>>  * SPDX-License-Identifier:     BSD-3-Clause
>>  */
>> @@ -30,7 +31,7 @@ static struct bl31_params *bl2_to_bl31_params;
>>  *
>>  * @return bl31 params structure pointer
>>  */
>> -struct bl31_params *bl2_plat_get_bl31_params(void)
>> +static struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl33_entry)
>> {
>>        struct entry_point_info *bl33_ep_info;
>> 
>> @@ -66,7 +67,7 @@ struct bl31_params *bl2_plat_get_bl31_params(void)
>> 
>>        /* 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->pc = bl33_entry;
>>        bl33_ep_info->spsr = SPSR_64(MODE_EL2, MODE_SP_ELX,
>>                                     DISABLE_ALL_EXECPTIONS);
>> 
>> @@ -77,21 +78,88 @@ struct bl31_params *bl2_plat_get_bl31_params(void)
>>        return bl2_to_bl31_params;
>> }
>> 
>> -void raw_write_daif(unsigned int daif)
>> +static inline void raw_write_daif(unsigned int daif)
>> {
>>        __asm__ __volatile__("msr DAIF, %0\n\t" : : "r" (daif) : "memory");
>> }
>> 
>> -void bl31_entry(void)
>> +typedef void (*atf_entry_t)(struct bl31_params *params, void *plat_params);
>> +
>> +static void bl31_entry(uintptr_t bl31_entry, uintptr_t bl33_entry,
>> +                      uintptr_t fdt_addr)
>> {
>>        struct bl31_params *bl31_params;
>> -       void (*entry)(struct bl31_params *params, void *plat_params) = NULL;
>> +       atf_entry_t  atf_entry = (atf_entry_t)bl31_entry;
>> 
>> -       bl31_params = bl2_plat_get_bl31_params();
>> -       entry = (void *)CONFIG_SPL_ATF_TEXT_BASE;
>> +       bl31_params = bl2_plat_get_bl31_params(bl33_entry);
>> 
>>        raw_write_daif(SPSR_EXCEPTION_MASK);
>>        dcache_disable();
>> 
>> -       entry(bl31_params, NULL);
>> +       atf_entry((void *)bl31_params, (void *)fdt_addr);
>> +}
>> +
>> +static int spl_fit_images_find_uboot(void *blob)
>> +{
>> +       int parent, node, ndepth;
>> +       const void *data;
>> +
>> +       if (!blob)
>> +               return -FDT_ERR_BADMAGIC;
>> +
>> +       parent = fdt_path_offset(blob, "/fit-images");
>> +       if (parent < 0)
>> +               return -FDT_ERR_NOTFOUND;
>> +
>> +       for (node = fdt_next_node(blob, parent, &ndepth);
>> +            (node >= 0) && (ndepth > 0);
>> +            node = fdt_next_node(blob, node, &ndepth)) {
>> +               if (ndepth != 1)
>> +                       continue;
>> +
>> +               data = fdt_getprop(blob, node, FIT_OS_PROP, NULL);
>> +               if (!data)
>> +                       continue;
>> +
>> +               if (genimg_get_os_id(data) == IH_OS_U_BOOT)
>> +                       return node;
> 
> How come this is in 'data' instead of the 'type' property?

In fact it’s in FIT_OS_PROP (which expands to “os”).
Naming this variable ‘data' may have not been the best choice, but ‘prop’ didn’t
seem much better…

Regards,
Philipp.