[U-Boot] [PATCH v3] spl: add support to booting with ATF
Kever Yang
kever.yang at rock-chips.com
Fri May 5 03:33:18 UTC 2017
Hi
On 04/06/2017 12:40 AM, Masahiro Yamada wrote:
> Hi.
>
>
> 2017-04-05 14:05 GMT+09:00 Kever Yang <kever.yang at rock-chips.com>:
>
>> --- a/common/spl/Kconfig
>> +++ b/common/spl/Kconfig
>> @@ -670,6 +670,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 Trusted Firmware"
>> + depends on SPL && ARM64
>> + help
>> + ATF(ARM Trusted Firmware) is a component for ARM arch64 which which
>> + is loaded by SPL(which is considered as BL2 in ATF terminology).
>> + More detail at: https://github.com/ARM-software/arm-trusted-firmware
> This statement may not be precise.
> ATF supports Aarch32 as well as Aarch64.
>
>
> I thik this is just U-Boot side matter.
> As far as I understood from the U-Boot directory structure
> (I only see Aarch64 code in arch/arm/cpu/armv8),
> U-Boot does not expect 32bit ARM code in ARMv8 generation
> (such as Cortex-A32, Coretex-A35, ...).
> As for U-Boot, ARM64 == ARMV8
> unless I am missing something.
I would like to leave it as is now, people can remove this if there is
aarch32 want to
support ATF, at least I didn't see any of them now.
>
>
>> +config SPL_ATF_TEXT_BASE
>> + depends on SPL_ATF_SUPPORT
>> + hex "ATF TEXT BASE addr"
> hex "ATF BL31 base address"
Will fix, thanks.
>
>
>
>> + help
>> + This is the base address in memory for ATF text and entry point.
> I'd like you to mention "BL31" explicitly
> because there are many images in ATF.
Will fix, thanks.
>
>
>
>> config TPL_ENV_SUPPORT
>> bool "Support an environment"
>> depends on TPL
>> diff --git a/common/spl/Makefile b/common/spl/Makefile
>> index 1933cbd..b3b34d6 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 26bc9ef..7041e1b 100644
>> --- a/common/spl/spl.c
>> +++ b/common/spl/spl.c
>> @@ -374,6 +374,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>> gd->malloc_ptr / 1024);
>> #endif
>>
>> + if (IS_ENABLED(CONFIG_SPL_ATF_SUPPORT))
>> + bl31_entry();
>> +
>> debug("loaded - jumping to U-Boot...\n");
>> spl_board_prepare_for_boot();
>> jump_to_image_no_args(&spl_image);
>
>
>
> switch (spl_image.os) {
> case IH_OS_U_BOOT:
> debug("Jumping to U-Boot\n");
> break;
> #ifdef CONFIG_SPL_OS_BOOT
> case IH_OS_LINUX:
> debug("Jumping to Linux\n");
> spl_board_prepare_for_linux();
> jump_to_image_linux(&spl_image,
> (void *)CONFIG_SYS_SPL_ARGS_ADDR);
> #endif
> default:
> debug("Unsupported OS image.. Jumping nevertheless..\n");
> }
>
>
> Which debug message is printed
> before SPL jumps to BL31?
>
> Jumping to U-Boot or Linux or Unsupported OS?
It's Jumping to U-Boot now, how about add one message before jump to
BL31 like this:
debug("loaded - jumping to U-Boot via ATF BL31.\n");
>
>
>
>
>
>
>
>> +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);
>> +}
> I guess SPL is supposed to load at least BL31 and BL33(=U-Boot full)
> because BL31 does not have a facility to load images from non-volatile devices.
>
> I can not see how to do it from this patch.
>
> Relying on Andre's work for multi images in FIT?
> http://patchwork.ozlabs.org/patch/745827/
Yes, I use Andre's patch to load multi image in FIT, this can handle
flexible number of
ATF target.
>
>
> I am not tracking how they are related, though.
>
>
>
>
>> +/*******************************************************************************
>> + * Structure used for telling the next BL how much of a particular type of
>> + * memory is available for its use and how much is already used.
>> + ******************************************************************************/
>> +struct aapcs64_params_t {
>
> Please do not add _t suffix for structure.
>
>
>> + unsigned long arg0;
>> + unsigned long arg1;
>> + unsigned long arg2;
>> + unsigned long arg3;
>> + unsigned long arg4;
>> + unsigned long arg5;
>> + unsigned long arg6;
>> + unsigned long arg7;
>> +};
>
>
> In ATF, every structure has a shorthand
> typedef with _t suffix, like this:
>
> typedef struct aapcs64_params {
> u_register_t arg0;
> u_register_t arg1;
> u_register_t arg2;
> u_register_t arg3;
> u_register_t arg4;
> u_register_t arg5;
> u_register_t arg6;
> u_register_t arg7;
> } aapcs64_params_t;
>
>
> This is bad habit anyway, so
> no new typedef:s, no _t suffix in U-boot.
>
>
>
>
>
>> +/***************************************************************************
>> + * This structure provides version information and the size of the
>> + * structure, attributes for the structure it represents
>> + ***************************************************************************/
>> +struct param_header_t {
> Please rename to
> struct param_header
>
> Likewise for the others.
OK, will do it.
>
>
>
>> +/*******************************************************************************
>> + * This structure represents the superset of information that is passed to
>> + * BL31, e.g. while passing control to it from BL2, bl31_params
>> + * and other platform specific params
>> + ******************************************************************************/
>> +struct bl2_to_bl31_params_mem_t {
>> + struct bl31_params_t bl31_params;
>> + struct atf_image_info_t bl31_image_info;
>> + struct atf_image_info_t bl32_image_info;
>> + struct atf_image_info_t bl33_image_info;
>> + struct entry_point_info_t bl33_ep_info;
>> + struct entry_point_info_t bl32_ep_info;
>> + struct entry_point_info_t bl31_ep_info;
>> +};
>> +
> This is old interface. (LOAD_IMAGE_V1).
>
>
> As Dan explained, this was superseded by struct bl_params and bl_params_node.
>
>
> I'd recommend you to implement LOAD_IMAGE_V2.
Yes, there is new IMAGE_V2 available now, but I would like to support V1
first,
because there already have bl31 binary release from Rockchip which is V1
format.
Thanks,
- Kever
>
>
More information about the U-Boot
mailing list