[U-Boot] [PATCH v3] imx: Support i.MX6 High Assurance Boot authentication
Nitin Garg
nitin.garg at freescale.com
Tue Sep 16 20:37:18 CEST 2014
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.garg at 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, sbabic at 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
More information about the U-Boot
mailing list