[U-Boot] [PATCH v2 0/6] pico-imx7d: Add support for BL33 case

Add configuration to boot U-boot as BL33 case. The boot flow is ATF -> OPTEE -> U-boot.
Changes vs V1: - Remove DCD file. - Add a patch to fix uart clock root ID. - Change file name from pico-pi-imx7d_bl33_defconfig to pico-imx7d_bl33_defconfig
Jun Nie (6): mx7_common: Share configs to skip low level init imx: mx7: Add empty arch_cpu_init if skipped pico-imx7d: Reserve region of memory to OPTEE pico-imx7d: Add boot option for verified boot pico-imx7d: Add bl33 config pico-imx7d: Correct uart clock root
arch/arm/include/asm/arch-mx7/clock.h | 5 +++ arch/arm/mach-imx/mx7/clock.c | 2 +- arch/arm/mach-imx/mx7/soc.c | 4 +++ board/technexion/pico-imx7d/pico-imx7d.c | 5 +++ configs/pico-imx7d_bl33_defconfig | 61 ++++++++++++++++++++++++++++++++ include/configs/mx7_common.h | 11 ++++++ include/configs/pico-imx7d.h | 39 ++++++++++++++++++-- include/configs/warp7.h | 11 ------ 8 files changed, 123 insertions(+), 15 deletions(-) create mode 100644 configs/pico-imx7d_bl33_defconfig

Share configs in mx7 to skip low level init if we are in the case where OPTEE is loaded already (maybe by ARM Trusted Firmware) and that most of the low level initialization is already done and that we may/should skip it doing them here.
Fix the definition detection with size detection to decide whether to skip it.
Signed-off-by: Jun Nie jun.nie@linaro.org --- include/configs/mx7_common.h | 11 +++++++++++ include/configs/warp7.h | 11 ----------- 2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/include/configs/mx7_common.h b/include/configs/mx7_common.h index a895c93..912fb06 100644 --- a/include/configs/mx7_common.h +++ b/include/configs/mx7_common.h @@ -57,4 +57,15 @@ #endif #endif
+/* + * If we have defined the OPTEE ram size and not OPTEE it means that we were + * launched by OPTEE, because of that we shall skip all the low level + * initialization since it was already done by ATF or OPTEE + */ +#if (CONFIG_OPTEE_TZDRAM_SIZE != 0) +#ifndef CONFIG_OPTEE +#define CONFIG_SKIP_LOWLEVEL_INIT +#endif +#endif + #endif diff --git a/include/configs/warp7.h b/include/configs/warp7.h index 043f286..80ddd72 100644 --- a/include/configs/warp7.h +++ b/include/configs/warp7.h @@ -13,17 +13,6 @@
#define PHYS_SDRAM_SIZE SZ_512M
-/* - * If we have defined the OPTEE ram size and not OPTEE it means that we were - * launched by OPTEE, because of that we shall skip all the low level - * initialization since it was already done by ATF or OPTEE - */ -#ifdef CONFIG_OPTEE_TZDRAM_SIZE -#ifndef CONFIG_OPTEE -#define CONFIG_SKIP_LOWLEVEL_INIT -#endif -#endif - #define CONFIG_MXC_UART_BASE UART1_IPS_BASE_ADDR
/* Size of malloc() pool */

Add empty arch_cpu_init if low level init is skipped. So that it does not break spl compile though spl is not needed in the skipped case actually.
Signed-off-by: Jun Nie jun.nie@linaro.org --- arch/arm/mach-imx/mx7/soc.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/arch/arm/mach-imx/mx7/soc.c b/arch/arm/mach-imx/mx7/soc.c index 7cfdff0..9b04013 100644 --- a/arch/arm/mach-imx/mx7/soc.c +++ b/arch/arm/mach-imx/mx7/soc.c @@ -286,6 +286,11 @@ int arch_cpu_init(void)
return 0; } +#else +int arch_cpu_init(void) +{ + return 0; +} #endif
#ifdef CONFIG_ARCH_MISC_INIT

Subtracts CONFIG_OPTEE_TZDRAM_SIZE from the available DRAM size so that the OPTEE memory is not override during u-boot relocation.
Note the OPTEE boot process will itself subtract the DRAM region it lives in from the memory map passed to Linux.
Signed-off-by: Jun Nie jun.nie@linaro.org --- board/technexion/pico-imx7d/pico-imx7d.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/board/technexion/pico-imx7d/pico-imx7d.c b/board/technexion/pico-imx7d/pico-imx7d.c index 53e1469..7c9e145 100644 --- a/board/technexion/pico-imx7d/pico-imx7d.c +++ b/board/technexion/pico-imx7d/pico-imx7d.c @@ -60,6 +60,11 @@ int dram_init(void) { gd->ram_size = imx_ddr_size();
+ /* Subtract the defined OPTEE runtime firmware length */ +#ifdef CONFIG_OPTEE_TZDRAM_SIZE + gd->ram_size -= CONFIG_OPTEE_TZDRAM_SIZE; +#endif + return 0; }

Add boot option to boot from fitimage to support verified boot. The boot script plain text file should be packed into fit blob as image with name of bootscr.
Signed-off-by: Jun Nie jun.nie@linaro.org --- include/configs/pico-imx7d.h | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-)
diff --git a/include/configs/pico-imx7d.h b/include/configs/pico-imx7d.h index 1884c58..8eb9064 100644 --- a/include/configs/pico-imx7d.h +++ b/include/configs/pico-imx7d.h @@ -52,11 +52,29 @@ "/boot/imx7d-pico-pi.dtb ext4 0 1;" \ "rootfs part 0 1\0" \
-#define BOOTMENU_ENV \ +/* When booting with FIT specify the node entry containing boot.scr */ +#if defined(CONFIG_FIT) +#define PICO_BOOT_ENV \ + "bootscr_fitimage_name=bootscr\0" \ + "bootscriptaddr=0x83200000\0" \ + "fdtovaddr=0x83100000\0" \ + "mmcdev=" __stringify(CONFIG_SYS_MMC_ENV_DEV)"\0" \ + "mmcpart=" __stringify(CONFIG_SYS_MMC_IMG_LOAD_PART) "\0" \ + "mmcargs=setenv bootargs console=${console},${baudrate} " \ + "rootwait rw;\0" \ + "loadbootscript=" \ + "load mmc ${mmcdev}:${mmcpart} ${bootscriptaddr} ${script};\0" \ + "bootscript=echo Running bootscript from mmc ...; " \ + "source ${bootscriptaddr}:${bootscr_fitimage_name}\0" +#else +#define PICO_BOOT_ENV \ "bootmenu_0=Boot using PICO-Hobbit baseboard=" \ "setenv fdtfile imx7d-pico-hobbit.dtb\0" \ "bootmenu_1=Boot using PICO-Pi baseboard=" \ "setenv fdtfile imx7d-pico-pi.dtb\0" \ + BOOTENV +#endif +
#define CONFIG_SUPPORT_EMMC_BOOT /* eMMC specific */ #define CONFIG_SYS_MMC_IMG_LOAD_PART 1 @@ -68,7 +86,6 @@ "fdt_high=0xffffffff\0" \ "initrd_high=0xffffffff\0" \ "fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \ - BOOTMENU_ENV \ "fdt_addr=0x83000000\0" \ "fdt_addr_r=0x83000000\0" \ "kernel_addr_r=" __stringify(CONFIG_LOADADDR) "\0" \ @@ -88,7 +105,22 @@ "name=rootfs,size=0,uuid=${uuid_gpt_rootfs}\0" \ "fastboot_partition_alias_system=rootfs\0" \ "setup_emmc=mmc dev 0; gpt write mmc 0 $partitions; reset;\0" \ - BOOTENV + PICO_BOOT_ENV + +#if defined(CONFIG_FIT) +#define CONFIG_BOOTCOMMAND \ + "mmc dev ${mmcdev};" \ + "mmc dev ${mmcdev}; if mmc rescan; then " \ + "if run loadbootscript; then " \ + "iminfo ${bootscriptaddr};" \ + "if test $? -eq 1; then hab_failsafe; fi;" \ + "run bootscript; " \ + "else " \ + "echo Fail to load fitImage with boot script;" \ + "hab_failsafe;" \ + "fi; " \ + "fi" +#endif
#define BOOT_TARGET_DEVICES(func) \ func(MMC, mmc, 0) \

Add default configuration to run u-boot as BL33 in the boot flow case of ATF(ARM Trusted Firmware) -> OPTEE -> U-boot.
Signed-off-by: Jun Nie jun.nie@linaro.org --- configs/pico-imx7d_bl33_defconfig | 61 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 configs/pico-imx7d_bl33_defconfig
diff --git a/configs/pico-imx7d_bl33_defconfig b/configs/pico-imx7d_bl33_defconfig new file mode 100644 index 0000000..77e46d5 --- /dev/null +++ b/configs/pico-imx7d_bl33_defconfig @@ -0,0 +1,61 @@ +CONFIG_ARM=y +CONFIG_ARCH_MX7=y +CONFIG_SYS_TEXT_BASE=0x87800000 +CONFIG_SPL_GPIO_SUPPORT=y +CONFIG_SPL_LIBCOMMON_SUPPORT=y +CONFIG_SPL_LIBGENERIC_SUPPORT=y +CONFIG_SECURE_BOOT=y +CONFIG_TARGET_PICO_IMX7D=y +CONFIG_SPL_MMC_SUPPORT=y +CONFIG_SPL_SERIAL_SUPPORT=y +CONFIG_SPL=y +CONFIG_ARMV7_BOOT_SEC_DEFAULT=y +CONFIG_FIT=y +CONFIG_FIT_SIGNATURE=y +CONFIG_FIT_VERBOSE=y +CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=arch/arm/mach-imx/spl_sd.cfg" +CONFIG_DEFAULT_FDT_FILE="imx7d-pico-pi.dtb" +CONFIG_BOUNCE_BUFFER=y +CONFIG_SPL_I2C_SUPPORT=y +CONFIG_SPL_USB_HOST_SUPPORT=y +CONFIG_SPL_USB_GADGET=y +CONFIG_SPL_USB_SDP_SUPPORT=y +CONFIG_HUSH_PARSER=y +# CONFIG_CMD_BOOTD is not set +CONFIG_CMD_BOOTZ=y +CONFIG_CMD_SPL=y +CONFIG_CMD_SPL_WRITE_SIZE=0x20000 +CONFIG_CMD_MEMTEST=y +CONFIG_CMD_DFU=y +CONFIG_CMD_GPIO=y +CONFIG_CMD_GPT=y +CONFIG_CMD_I2C=y +CONFIG_CMD_MMC=y +CONFIG_CMD_USB=y +CONFIG_CMD_USB_SDP=y +CONFIG_CMD_USB_MASS_STORAGE=y +CONFIG_CMD_DHCP=y +CONFIG_CMD_PXE=y +CONFIG_CMD_CACHE=y +CONFIG_CMD_FAT=y +CONFIG_CMD_FS_GENERIC=y +CONFIG_NET_RANDOM_ETHADDR=y +CONFIG_DFU_MMC=y +CONFIG_FSL_ESDHC=y +CONFIG_PHYLIB=y +CONFIG_MII=y +CONFIG_USB=y +CONFIG_USB_EHCI_HCD=y +CONFIG_MXC_USB_OTG_HACTIVE=y +CONFIG_USB_GADGET=y +CONFIG_USB_GADGET_MANUFACTURER="FSL" +CONFIG_USB_GADGET_VENDOR_NUM=0x0525 +CONFIG_USB_GADGET_PRODUCT_NUM=0xa4a5 +CONFIG_CI_UDC=y +CONFIG_USB_GADGET_DOWNLOAD=y +CONFIG_USB_ETHER=y +CONFIG_USB_ETH_CDC=y +CONFIG_USBNET_HOST_ADDR="de:ad:be:af:00:00" +CONFIG_OF_LIBFDT=y +CONFIG_OF_LIBFDT_OVERLAY=y +CONFIG_OPTEE_TZDRAM_SIZE=0x2000000

Correct uart clock root ID. Incorrect ID may result the clock is gated because rate value 0 is returned in imx_get_uartclk()
Signed-off-by: Jun Nie jun.nie@linaro.org --- arch/arm/include/asm/arch-mx7/clock.h | 5 +++++ arch/arm/mach-imx/mx7/clock.c | 2 +- include/configs/pico-imx7d.h | 1 + 3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/arch-mx7/clock.h b/arch/arm/include/asm/arch-mx7/clock.h index f56564e..b6a1768 100644 --- a/arch/arm/include/asm/arch-mx7/clock.h +++ b/arch/arm/include/asm/arch-mx7/clock.h @@ -175,6 +175,11 @@ enum clk_root_index { CLK_ROOT_MAX, };
+/* Use UART1_CLK_ROOT as default root clk if platform does define it */ +#ifndef UART_CLK_ROOT +#define UART_CLK_ROOT UART1_CLK_ROOT +#endif + struct clk_root_setting { enum clk_root_index root; u32 setting; diff --git a/arch/arm/mach-imx/mx7/clock.c b/arch/arm/mach-imx/mx7/clock.c index 8cda71c..e364b16 100644 --- a/arch/arm/mach-imx/mx7/clock.c +++ b/arch/arm/mach-imx/mx7/clock.c @@ -53,7 +53,7 @@ static u32 get_ipg_clk(void)
u32 imx_get_uartclk(void) { - return get_root_clk(UART1_CLK_ROOT); + return get_root_clk(UART_CLK_ROOT); }
u32 imx_get_fecclk(void) diff --git a/include/configs/pico-imx7d.h b/include/configs/pico-imx7d.h index 8eb9064..9806348 100644 --- a/include/configs/pico-imx7d.h +++ b/include/configs/pico-imx7d.h @@ -28,6 +28,7 @@ #define CONFIG_SYS_MALLOC_LEN (32 * SZ_1M)
#define CONFIG_MXC_UART_BASE UART5_IPS_BASE_ADDR +#define UART_CLK_ROOT UART5_CLK_ROOT
/* Network */ #define CONFIG_FEC_MXC

On Thu, Mar 28, 2019 at 5:34 AM Jun Nie jun.nie@linaro.org wrote:
Correct uart clock root ID. Incorrect ID may result the clock is gated because rate value 0 is returned in imx_get_uartclk()
Signed-off-by: Jun Nie jun.nie@linaro.org
How this could not be defined? couldn't this to be fixed on kconfig to use it as default?

Otavio Salvador otavio.salvador@ossystems.com.br 于2019年3月28日周四 下午8:58写道:
On Thu, Mar 28, 2019 at 5:34 AM Jun Nie jun.nie@linaro.org wrote:
Correct uart clock root ID. Incorrect ID may result the clock is gated because rate value 0 is returned in imx_get_uartclk()
Signed-off-by: Jun Nie jun.nie@linaro.org
How this could not be defined? couldn't this to be fixed on kconfig to use it as default?
Sure, it can be fixed to a default value or string. I had tried to add an IMX_UART_PORT config option in Kconfig to dynamic configure the CLK_ROOT ID as below, the expansion result is UARTCONFIG_IMX_UART_PORT_CLK_ROOT instead of UART1_CLK_ROOT. It seems macro expansion happens before the inclusion of defconfig.
#define IMX_UART_CLK_ROOT UART##CONFIG_IMX_CONS_ID##_CLK_ROOT
Do you see any better idea other than many definition if #if #elif as below? Below code is not beautiful, only usable.
+++ b/arch/arm/include/asm/arch-mx7/clock.h @@ -175,6 +175,24 @@ enum clk_root_index { CLK_ROOT_MAX, };
+#if (CONFIG_IMX_CONS_ID == 1) +#define UART_CLK_ROOT UART1_CLK_ROOT +#elif (CONFIG_IMX_CONS_ID == 2) +#define UART_CLK_ROOT UART2_CLK_ROOT +#elif (CONFIG_IMX_CONS_ID == 3) +#define UART_CLK_ROOT UART3_CLK_ROOT +#elif (CONFIG_IMX_CONS_ID == 4) +#define UART_CLK_ROOT UART4_CLK_ROOT +#elif (CONFIG_IMX_CONS_ID == 5) +#define UART_CLK_ROOT UART5_CLK_ROOT +#elif (CONFIG_IMX_CONS_ID == 6) +#define UART_CLK_ROOT UART6_CLK_ROOT +#elif (CONFIG_IMX_CONS_ID == 7) +#define UART_CLK_ROOT UART7_CLK_ROOT +#else +#error "Invalid IMX console ID is defined" +#endif
Best Regards. Jun
-- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://code.ossystems.com.br Mobile: +55 (53) 9 9981-7854 Mobile: +1 (347) 903-9750

On Thu, Mar 28, 2019 at 5:30 AM Jun Nie jun.nie@linaro.org wrote:
Add configuration to boot U-boot as BL33 case. The boot flow is ATF -> OPTEE -> U-boot.
Could you add a README explaining how to use / test it?

Otavio Salvador otavio.salvador@ossystems.com.br 于2019年3月28日周四 下午8:57写道:
On Thu, Mar 28, 2019 at 5:30 AM Jun Nie jun.nie@linaro.org wrote:
Add configuration to boot U-boot as BL33 case. The boot flow is ATF -> OPTEE -> U-boot.
Could you add a README explaining how to use / test it?
Hi Otavio,
Below is draft of README. some of command may not be accurate because I am on travel and cannot access latest code in my PC in home. If you are OK with idea in it, I will polish it later. The SPL is built but not used because the BL33 is loaded by ATF while ATF exist in SRAM. ATF cannot load SPL into SRAM to override itself.
README.pico-imx7d_BL33 This document describes the instruction to build ATF/OPTEE and flash eMMC on pico-imx7d board. U-Boot is loaded by ATF/OPTEE in this setup.
- Build u-boot # set environment variable of CROSS_COMPILE for your toolchain and ARCH=arm make pico-pi-imx7d-mbl_defconfig make all
- Download and build OPTEE git clone https://git.linaro.org/landing-teams/working/mbl/optee_os.git -b linaro-pico make PLATFORM=imx PLATFORM_FLAVOR=mx7dpico_mbl ARCH=arm
- Download and build ATF, create FIP image git clone https://git.linaro.org/landing-teams/working/mbl/arm-trusted-firmware.git -b linaro-imx7 make DEBUG=1 PLAT=picopi ARCH=aarch32 ARM_ARCH_MAJOR=7 CROSS_COMPILE=arm-linux-gnueabihf- LOG_LEVEL=50 V=1 CRASH_REPORTING=1 AARCH32_SP=optee all wget http://git.linaro.org/landing-teams/working/mbl/u-boot.git/tree/board/techne... mkimage -n pico-imx7d.cfg -T imximage -e 0x87800000 -d -d build/picopi/debug/bl2.bin bl2.imx # create a fiptool_images/ folder under atf folder, copy u-boot.bin in u-boot folder and tee*.bin in optee core/tee/ folder to fiptool_images. Run below command to generate FIP image. tools/fiptool/fiptool create --tos-fw fiptool_images/tee-header_v2.bin \ --tos-fw-extra1 fiptool_images/tee-pager_v2.bin \ --tos-fw-extra2 fiptool_images/tee-pageable_v2.bin \ --nt-fw fiptool_images/u-boot.bin \ fip.bin
- Burn the images to eMMC for test. # Run below command in atf folder: dd if=build/picopi/debug/bl2.bin.imx of=/dev/disk/by-id/usb-<your device> bs=1024 seek=1;sync dd if=fip.bin of=/dev/disk/by-id/usb-<your device> bs=1024 seek=1;sync
- Test Just boot up your board and wait for u-boot start up after ATF's log. For booting Linux in FIT image, please reference the FIT files in u-boot doc/uImage.FIT/ folder.
-- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://code.ossystems.com.br Mobile: +55 (53) 9 9981-7854 Mobile: +1 (347) 903-9750

Hi Otavio,
And for the file name pico-imx7d_bl33_defconfig, pico-pi-imx7d_bl33_defconfig should be better in my later thought. Because a concrete device tree source file should be specified in defconfig file, so that public key can be stored in u-boot FDT blob for kernel/initramfs FIT signature verification. What's your comments?
Thanks! Jun

On Wed, Apr 3, 2019 at 9:00 AM Jun Nie jun.nie@linaro.org wrote:
And for the file name pico-imx7d_bl33_defconfig, pico-pi-imx7d_bl33_defconfig should be better in my later thought. Because a concrete device tree source file should be specified in defconfig file, so that public key can be stored in u-boot FDT blob for kernel/initramfs FIT signature verification. What's your comments?
The FIT image might have them all, no? It would allow a single binary to work on all baseboards, giving a very nice user experience.

Otavio Salvador otavio.salvador@ossystems.com.br 于2019年4月4日周四 下午8:40写道:
On Wed, Apr 3, 2019 at 9:00 AM Jun Nie jun.nie@linaro.org wrote:
And for the file name pico-imx7d_bl33_defconfig, pico-pi-imx7d_bl33_defconfig should be better in my later thought. Because a concrete device tree source file should be specified in defconfig file, so that public key can be stored in u-boot FDT blob for kernel/initramfs FIT signature verification. What's your comments?
The FIT image might have them all, no? It would allow a single binary to work on all baseboards, giving a very nice user experience.
The FIT image have all device trees. I mean the device tree for u-boot should be specified. As device trees are copied from kernel, while there is no dts file for pico-imx7d. It is a bit peculiar if you specify device tree as pico-pi-imx7d in pico-imx7d_bl33_defconfig.
-- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://code.ossystems.com.br Mobile: +55 (53) 9 9981-7854 Mobile: +1 (347) 903-9750
participants (2)
-
Jun Nie
-
Otavio Salvador