Hi Stefano, On 09/12/2014 03:46 AM, Stefano Babic wrote: > Hi Nitin, > > On 04/09/2014 03:18, Nitin Garg wrote: >> When CONFIG_SECURE_BOOT is enabled, the signed images >> like kernel and dtb can be authenticated using iMX6 CAAM. >> The added command hab_auth_img can be used for HAB >> authentication of images. The command takes the image >> DDR location, IVT (Image Vector Table) offset inside >> image as parameters. Detailed info about signing images >> can be found in Freescale AppNote AN4581. >> >> Signed-off-by: Nitin Garg <nitin.g...@freescale.com> >> >> --- >> >> Changes in v3: >> - Remove typecast of get_cpu_rev since its not required >> >> Changes in v2: >> - Cleaned up clock code as per review comments >> - Removed dead code as per review comments >> - Re-written commit log as per review comments >> >> arch/arm/cpu/armv7/mx6/clock.c | 32 ++++++- >> arch/arm/cpu/armv7/mx6/hab.c | 165 >> ++++++++++++++++++++++++++++++++- >> arch/arm/cpu/armv7/mx6/soc.c | 15 +++ >> arch/arm/include/asm/arch-mx6/clock.h | 4 + >> 4 files changed, 214 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c >> index 820b8d5..db6a8fc 100644 >> --- a/arch/arm/cpu/armv7/mx6/clock.c >> +++ b/arch/arm/cpu/armv7/mx6/clock.c >> @@ -1,5 +1,5 @@ >> /* >> - * Copyright (C) 2010-2011 Freescale Semiconductor, Inc. >> + * Copyright (C) 2010-2014 Freescale Semiconductor, Inc. >> * >> * SPDX-License-Identifier: GPL-2.0+ >> */ >> @@ -543,6 +543,36 @@ int enable_pcie_clock(void) >> BM_ANADIG_PLL_ENET_ENABLE_PCIE); >> } >> >> +#ifdef CONFIG_SECURE_BOOT >> +void hab_caam_clock_enable(void) >> +{ >> + struct mxc_ccm_reg *const imx_ccm = >> + (struct mxc_ccm_reg *)CCM_BASE_ADDR; >> + >> + /*CG4 ~ CG6, enable CAAM clocks*/ >> + setbits_le32(&imx_ccm->CCGR0, MXC_CCM_CCGR0_CAAM_WRAPPER_IPG_MASK | >> + MXC_CCM_CCGR0_CAAM_WRAPPER_ACLK_MASK | >> + MXC_CCM_CCGR0_CAAM_SECURE_MEM_MASK); >> + >> + /* Enable EMI slow clk */ >> + setbits_le32(&imx_ccm->CCGR6, MXC_CCM_CCGR6_EMI_SLOW_MASK); >> +} >> + >> +void hab_caam_clock_disable(void) >> +{ >> + struct mxc_ccm_reg *const imx_ccm = >> + (struct mxc_ccm_reg *)CCM_BASE_ADDR; >> + >> + /*CG4 ~ CG6, disable CAAM clocks*/ >> + clrbits_le32(&imx_ccm->CCGR0, MXC_CCM_CCGR0_CAAM_WRAPPER_IPG_MASK | >> + MXC_CCM_CCGR0_CAAM_WRAPPER_ACLK_MASK | >> + MXC_CCM_CCGR0_CAAM_SECURE_MEM_MASK); >> + >> + /* Disable EMI slow clk */ >> + clrbits_le32(&imx_ccm->CCGR6, MXC_CCM_CCGR6_EMI_SLOW_MASK); >> +} >> +#endif > > > Generally, we have in clock.c one function per clock, getting as > enable_uart_clkparameter a boolean for enabling/disabling (i.e. > enable_ocotp_clk(), enable_uart_clk(),...) > > Please stick with the same rule. Accepted. Rework in v4.
> >> + >> unsigned int mxc_get_clock(enum mxc_clock clk) >> { >> switch (clk) { >> diff --git a/arch/arm/cpu/armv7/mx6/hab.c b/arch/arm/cpu/armv7/mx6/hab.c >> index f6810a6..61a94a1 100644 >> --- a/arch/arm/cpu/armv7/mx6/hab.c >> +++ b/arch/arm/cpu/armv7/mx6/hab.c >> @@ -1,5 +1,5 @@ >> /* >> - * Copyright (C) 2010-2013 Freescale Semiconductor, Inc. >> + * Copyright (C) 2010-2014 Freescale Semiconductor, Inc. >> * >> * SPDX-License-Identifier: GPL-2.0+ >> */ >> @@ -7,8 +7,12 @@ >> #include <common.h> >> #include <asm/io.h> >> #include <asm/arch/hab.h> >> +#include <asm/arch/clock.h> >> #include <asm/arch/sys_proto.h> >> >> +/* HAB (High Assurance Boot) debug */ >> +#undef DEBUG_AUTHENTICATE_IMAGE > > This is never defined, you do not need to undefine it. Accepted. Rework in v4. > >> + >> /* -------- start of HAB API updates ------------*/ >> >> #define hab_rvt_report_event_p \ >> @@ -71,6 +75,41 @@ >> ((hab_rvt_exit_t *)HAB_RVT_EXIT) \ >> ) >> >> +#define IVT_SIZE 0x20 >> +#define ALIGN_SIZE 0x1000 >> +#define CSF_PAD_SIZE 0x2000 >> + >> +/* >> + * +------------+ 0x0 (DDR_UIMAGE_START) - >> + * | Header | | >> + * +------------+ 0x40 | >> + * | | | >> + * | | | >> + * | | | >> + * | | | >> + * | Image Data | | >> + * . | | >> + * . | > Stuff to be authenticated >> ----+ >> + * . | | >> | >> + * | | | >> | >> + * | | | >> | >> + * +------------+ | >> | >> + * | | | >> | >> + * | Fill Data | | >> | >> + * | | | >> | >> + * +------------+ Align to ALIGN_SIZE | >> | >> + * | IVT | | >> | >> + * +------------+ + IVT_SIZE - >> | >> + * | | >> | >> + * | CSF DATA | >> <---------------------------------------------------------+ >> + * | | >> + * +------------+ >> + * | | >> + * | Fill Data | >> + * | | >> + * +------------+ + CSF_PAD_SIZE >> + */ >> + >> bool is_hab_enabled(void) >> { >> struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR; >> @@ -144,6 +183,105 @@ int get_hab_status(void) >> return 0; >> } >> >> +uint32_t authenticate_image(uint32_t ddr_start, uint32_t image_size) >> +{ >> + uint32_t load_addr = 0; >> + size_t bytes; >> + ptrdiff_t ivt_offset = 0; >> + int result = 0; >> + ulong start; >> + hab_rvt_authenticate_image_t *hab_rvt_authenticate_image; >> + hab_rvt_entry_t *hab_rvt_entry; >> + hab_rvt_exit_t *hab_rvt_exit; >> + >> + hab_rvt_authenticate_image = hab_rvt_authenticate_image_p; >> + hab_rvt_entry = hab_rvt_entry_p; >> + hab_rvt_exit = hab_rvt_exit_p; >> + >> + if (is_hab_enabled()) { >> + printf("\nAuthenticate image from DDR location 0x%x...\n", >> + ddr_start); >> + >> + hab_caam_clock_enable(); >> + >> + if (hab_rvt_entry() == HAB_SUCCESS) { >> + /* If not already aligned, Align to ALIGN_SIZE */ >> + ivt_offset = (image_size + ALIGN_SIZE - 1) & >> + ~(ALIGN_SIZE - 1); >> + >> + start = ddr_start; >> + bytes = ivt_offset + IVT_SIZE + CSF_PAD_SIZE; >> + >> +#ifdef DEBUG_AUTHENTICATE_IMAGE > > > We have already a way for adding debugging. Use debug() instead of > printf(), and you can simply use #ifdef DEBUG for conditional branches. > > We decided some times ago to avoid adding any flavour of specific DEBUG_ > switches. Accepted. Rework in v4. > > >> + printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n", >> + ivt_offset, ddr_start + ivt_offset); >> + printf("Dumping IVT\n"); >> + print_buffer(ddr_start + ivt_offset, >> + (void *)(ddr_start + ivt_offset), >> + 4, 0x8, 0); >> + >> + printf("Dumping CSF Header\n"); >> + print_buffer(ddr_start + ivt_offset+IVT_SIZE, >> + (void *)(ddr_start + ivt_offset+IVT_SIZE), >> + 4, 0x10, 0); >> + >> + get_hab_status(); >> + >> + printf("\nCalling authenticate_image in ROM\n"); >> + printf("\tivt_offset = 0x%x\n", ivt_offset); >> + printf("\tstart = 0x%08lx\n", start); >> + printf("\tbytes = 0x%x\n", bytes); >> +#endif >> + /* >> + * If the MMU is enabled, we have to notify the ROM >> + * code, or it won't flush the caches when needed. >> + * This is done, by setting the "pu_irom_mmu_enabled" >> + * word to 1. You can find its address by looking in >> + * the ROM map. This is critical for >> + * authenticate_image(). If MMU is enabled, without >> + * setting this but, authentication will fail and may >> + * crash. >> + */ >> + if (is_cpu_type(MXC_CPU_MX6Q) || >> + is_cpu_type(MXC_CPU_MX6D)) { >> + /* >> + * This won't work on Rev 1.0.0 of i.MX6Q/D, >> + * since their ROM doesn't do cache flushes. >> + * I don't think any exist, so we ignore them. >> + */ >> + writel(1, 0x009024a8); > > Can you add defines or (better) structures for this ? Writing in this > way into the hardware is generally not allowed in u-boot. Accepted. Rework in v4. This was due to ROM bug so pls consider it. > > In the comments you say that it must be checked if MMU is on (generally > on when cache is enabled), but there is no check afterward, only a > different behavior depending on CPU. Does it mean that > pu_irom_mmu_enabled is set independently from MMU status ? Accepted. Rework in v4. > >> + } else if (is_cpu_type(MXC_CPU_MX6DL) || >> + is_cpu_type(MXC_CPU_MX6SOLO)) { >> + writel(1, 0x00901dd0); >> + } else if (is_cpu_type(MXC_CPU_MX6SL)) { >> + writel(1, 0x00900a18); >> + } >> + >> + load_addr = (uint32_t)hab_rvt_authenticate_image( >> + HAB_CID_UBOOT, >> + ivt_offset, (void **)&start, >> + (size_t *)&bytes, NULL); >> + if (hab_rvt_exit() != HAB_SUCCESS) { >> + printf("hab exit function fail\n"); >> + load_addr = 0; >> + } >> + } else { >> + printf("hab entry function fail\n"); > > Use puts() instead of printf() when you want to output a constant string. Accepted. Rework in v4. > >> + } >> + >> + hab_caam_clock_disable(); >> + >> + get_hab_status(); >> + } else { >> + printf("hab fuse not enabled\n"); >> + } >> + >> + if ((!is_hab_enabled()) || (load_addr != 0)) >> + result = 1; >> + >> + return result; >> +} >> + >> int do_hab_status(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >> { >> if ((argc != 1)) { >> @@ -156,8 +294,33 @@ int do_hab_status(cmd_tbl_t *cmdtp, int flag, int argc, >> char * const argv[]) >> return 0; >> } >> >> +static int do_authenticate_image(cmd_tbl_t *cmdtp, int flag, int argc, >> + char * const argv[]) >> +{ >> + ulong addr, ivt_offset; >> + int rcode = 0; >> + >> + if (argc < 3) >> + return CMD_RET_USAGE; >> + >> + addr = simple_strtoul(argv[1], NULL, 16); >> + ivt_offset = simple_strtoul(argv[2], NULL, 16); >> + >> + rcode = authenticate_image(addr, ivt_offset); >> + >> + return rcode; >> +} >> + >> U_BOOT_CMD( >> hab_status, CONFIG_SYS_MAXARGS, 1, do_hab_status, >> "display HAB status", >> "" >> ); >> + >> +U_BOOT_CMD( >> + hab_auth_img, 3, 1, do_authenticate_image, >> + "authenticate image via HAB", >> + "addr ivt_offset\n" >> + "addr - image hex address\n" >> + "ivt_offset - hex offset of IVT in the image" >> + ); >> diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c >> index b0c1306..9842efb 100644 >> --- a/arch/arm/cpu/armv7/mx6/soc.c >> +++ b/arch/arm/cpu/armv7/mx6/soc.c >> @@ -409,10 +409,25 @@ int board_postclk_init(void) >> #ifndef CONFIG_SYS_DCACHE_OFF >> void enable_caches(void) >> { >> +#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH) >> + enum dcache_option option = DCACHE_WRITETHROUGH; >> +#else >> + enum dcache_option option = DCACHE_WRITEBACK; >> +#endif >> + >> /* Avoid random hang when download by usb */ >> invalidate_dcache_all(); >> + >> /* Enable D-cache. I-cache is already enabled in start.S */ >> dcache_enable(); >> + >> + /* Enable caching on OCRAM and ROM */ >> + mmu_set_region_dcache_behaviour(ROMCP_ARB_BASE_ADDR, >> + ROMCP_ARB_END_ADDR, >> + option); >> + mmu_set_region_dcache_behaviour(IRAM_BASE_ADDR, >> + IRAM_SIZE, >> + option); >> } >> #endif >> >> diff --git a/arch/arm/include/asm/arch-mx6/clock.h >> b/arch/arm/include/asm/arch-mx6/clock.h >> index 339c789..2482e1a 100644 >> --- a/arch/arm/include/asm/arch-mx6/clock.h >> +++ b/arch/arm/include/asm/arch-mx6/clock.h >> @@ -2,6 +2,8 @@ >> * (C) Copyright 2009 >> * Stefano Babic, DENX Software Engineering, sba...@denx.de. >> * >> + * (C) Copyright 2014 Freescale Semiconductor, Inc. >> + * >> * SPDX-License-Identifier: GPL-2.0+ >> */ >> >> @@ -60,4 +62,6 @@ int enable_i2c_clk(unsigned char enable, unsigned i2c_num); >> int enable_spi_clk(unsigned char enable, unsigned spi_num); >> void enable_ipu_clock(void); >> int enable_fec_anatop_clock(enum enet_freq freq); >> +void hab_caam_clock_enable(void); >> +void hab_caam_clock_disable(void); >> #endif /* __ASM_ARCH_CLOCK_H */ >> > > I have not found enough documentation to verify this: is this code > suitable for MX53, too ? I can see in MX53 manual that a "CAAM" is > available, but nothing more as that. No. This works on MX6 only. > > Best regards, > Stefano Babic > Pls review v4 version. Regards, Nitin Garg _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot