* [U-Boot] [PATCH] i.MX6 High Assurance Boot (HAB) support @ 2014-08-31 20:16 nitin.garg at freescale.com 2014-08-31 20:16 ` [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images nitin.garg at freescale.com 0 siblings, 1 reply; 13+ messages in thread From: nitin.garg at freescale.com @ 2014-08-31 20:16 UTC (permalink / raw) To: u-boot From: Nitin Garg <nitin.garg@freescale.com> Add hab_auth_img u-boot command which can be used for HAB authentication of images. Nitin Garg (1): Support i.MX6 High Assurance Boot (HAB) authentication of images arch/arm/cpu/armv7/mx6/clock.c | 40 ++++++- arch/arm/cpu/armv7/mx6/hab.c | 180 ++++++++++++++++++++++++++++- arch/arm/cpu/armv7/mx6/soc.c | 15 +++ arch/arm/include/asm/arch-mx6/clock.h | 6 + arch/arm/include/asm/arch-mx6/sys_proto.h | 4 +- 5 files changed, 242 insertions(+), 3 deletions(-) -- 1.7.9.5 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images 2014-08-31 20:16 [U-Boot] [PATCH] i.MX6 High Assurance Boot (HAB) support nitin.garg at freescale.com @ 2014-08-31 20:16 ` nitin.garg at freescale.com 2014-09-01 1:09 ` Otavio Salvador 2014-09-01 5:29 ` Wolfgang Denk 0 siblings, 2 replies; 13+ messages in thread From: nitin.garg at freescale.com @ 2014-08-31 20:16 UTC (permalink / raw) To: u-boot From: Nitin Garg <nitin.garg@freescale.com> Add hab_auth_img u-boot command which can be used for HAB authentication of images. Signed-off-by: Nitin Garg <nitin.garg@freescale.com> --- arch/arm/cpu/armv7/mx6/clock.c | 40 ++++++- arch/arm/cpu/armv7/mx6/hab.c | 180 ++++++++++++++++++++++++++++- arch/arm/cpu/armv7/mx6/soc.c | 15 +++ arch/arm/include/asm/arch-mx6/clock.h | 6 + arch/arm/include/asm/arch-mx6/sys_proto.h | 4 +- 5 files changed, 242 insertions(+), 3 deletions(-) diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c index 820b8d5..62de8c9 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,44 @@ 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 *ccm_regs = (struct mxc_ccm_reg *)CCM_BASE_ADDR; + u32 reg = 0; + + /*CG4 ~ CG6, enable CAAM clocks*/ + reg = readl(&ccm_regs->CCGR0); + reg |= (MXC_CCM_CCGR0_CAAM_WRAPPER_IPG_MASK | + MXC_CCM_CCGR0_CAAM_WRAPPER_ACLK_MASK | + MXC_CCM_CCGR0_CAAM_SECURE_MEM_MASK); + writel(reg, &ccm_regs->CCGR0); + + /* Enable EMI slow clk */ + reg = readl(&ccm_regs->CCGR6); + reg |= MXC_CCM_CCGR6_EMI_SLOW_MASK; + writel(reg, &ccm_regs->CCGR6); +} + +void hab_caam_clock_disable(void) +{ + struct mxc_ccm_reg *ccm_regs = (struct mxc_ccm_reg *)CCM_BASE_ADDR; + u32 reg = 0; + + /*CG4 ~ CG6, disable CAAM clocks*/ + reg = readl(&ccm_regs->CCGR0); + reg &= ~(MXC_CCM_CCGR0_CAAM_WRAPPER_IPG_MASK | + MXC_CCM_CCGR0_CAAM_WRAPPER_ACLK_MASK | + MXC_CCM_CCGR0_CAAM_SECURE_MEM_MASK); + writel(reg, &ccm_regs->CCGR0); + + /* Disable EMI slow clk */ + reg = readl(&ccm_regs->CCGR6); + reg &= ~MXC_CCM_CCGR6_EMI_SLOW_MASK; + writel(reg, &ccm_regs->CCGR6); +} +#endif + 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..55780b8 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> +/* enable HAB (High Assurance Boot) debug */ +/* #define DEBUG_AUTHENTICATE_IMAGE */ + /* -------- 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,120 @@ int get_hab_status(void) return 0; } +#ifdef DEBUG_AUTHENTICATE_IMAGE +void dump_mem(uint32_t addr, int size) +{ + int i; + + for (i = 0; i < size; i += 4) { + if (i != 0) { + if (i % 16 == 0) + printf("\n"); + else + printf(" "); + } + + printf("0x%08x", *(uint32_t *)addr); + addr += 4; + } + + printf("\n"); + + return; +} +#endif + +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 uImage 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 + printf("\nivt_offset = 0x%x, ivt addr = 0x%x\n", + ivt_offset, ddr_start + ivt_offset); + printf("Dumping IVT\n"); + dump_mem(ddr_start + ivt_offset, 0x20); + + printf("Dumping CSF Header\n"); + dump_mem(ddr_start + ivt_offset + 0x20, 0x40); + + get_hab_status(); + + printf("\nCalling authenticate_image in ROM\n"); + printf("\tivt_offset = 0x%x\n\tstart = 0x%08x" + "\n\tbytes = 0x%x\n", ivt_offset, start, 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); + } 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"); + + hab_caam_clock_disable(); + + get_hab_status(); + } + + 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 +309,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 8aacdaf..4cb46a8 100644 --- a/arch/arm/cpu/armv7/mx6/soc.c +++ b/arch/arm/cpu/armv7/mx6/soc.c @@ -408,10 +408,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..b7c36e5 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,8 @@ 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); +#ifdef CONFIG_SECURE_BOOT +void hab_caam_clock_enable(void); +void hab_caam_clock_disable(void); +#endif #endif /* __ASM_ARCH_CLOCK_H */ diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h b/arch/arm/include/asm/arch-mx6/sys_proto.h index 306d699..2bbb86e 100644 --- a/arch/arm/include/asm/arch-mx6/sys_proto.h +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h @@ -2,6 +2,8 @@ * (C) Copyright 2009 * Stefano Babic, DENX Software Engineering, sbabic@denx.de. * + * (C) Copyright 2014 Freescale Semiconductor, Inc. + * * SPDX-License-Identifier: GPL-2.0+ */ @@ -11,7 +13,7 @@ #include <asm/imx-common/regs-common.h> #include "../arch-imx/cpu.h" -#define soc_rev() (get_cpu_rev() & 0xFF) +#define soc_rev() ((int)(get_cpu_rev() & 0xFF)) #define is_soc_rev(rev) (soc_rev() - rev) u32 get_nr_cpus(void); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images 2014-08-31 20:16 ` [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images nitin.garg at freescale.com @ 2014-09-01 1:09 ` Otavio Salvador 2014-09-03 1:36 ` Nitin Garg 2014-09-01 5:29 ` Wolfgang Denk 1 sibling, 1 reply; 13+ messages in thread From: Otavio Salvador @ 2014-09-01 1:09 UTC (permalink / raw) To: u-boot Hello Nitin, On Sun, Aug 31, 2014 at 5:16 PM, <nitin.garg@freescale.com> wrote: > From: Nitin Garg <nitin.garg@freescale.com> > > Add hab_auth_img u-boot command which can be used for HAB authentication > of images. > > Signed-off-by: Nitin Garg <nitin.garg@freescale.com> As the other patch I commented, this commit log also needs some rework to comply to the guidelines. I would also welcome a more detailed description about what this adds on top of previous HAB code. ... > diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h b/arch/arm/include/asm/arch-mx6/sys_proto.h > index 306d699..2bbb86e 100644 > --- a/arch/arm/include/asm/arch-mx6/sys_proto.h > +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h ... > @@ -11,7 +13,7 @@ > #include <asm/imx-common/regs-common.h> > #include "../arch-imx/cpu.h" > > -#define soc_rev() (get_cpu_rev() & 0xFF) > +#define soc_rev() ((int)(get_cpu_rev() & 0xFF)) This seems unrelated change, isn't it? -- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://code.ossystems.com.br Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images 2014-09-01 1:09 ` Otavio Salvador @ 2014-09-03 1:36 ` Nitin Garg 2014-09-03 1:41 ` Otavio Salvador 0 siblings, 1 reply; 13+ messages in thread From: Nitin Garg @ 2014-09-03 1:36 UTC (permalink / raw) To: u-boot On 08/31/2014 08:09 PM, Otavio Salvador wrote: > Hello Nitin, > > On Sun, Aug 31, 2014 at 5:16 PM, <nitin.garg@freescale.com> wrote: >> From: Nitin Garg <nitin.garg@freescale.com> >> >> Add hab_auth_img u-boot command which can be used for HAB authentication >> of images. >> >> Signed-off-by: Nitin Garg <nitin.garg@freescale.com> > > As the other patch I commented, this commit log also needs some rework > to comply to the guidelines. I would also welcome a more detailed > description about what this adds on top of previous HAB code. > I will improve the commit log and add detailed description. > ... >> diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h b/arch/arm/include/asm/arch-mx6/sys_proto.h >> index 306d699..2bbb86e 100644 >> --- a/arch/arm/include/asm/arch-mx6/sys_proto.h >> +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h > ... >> @@ -11,7 +13,7 @@ >> #include <asm/imx-common/regs-common.h> >> #include "../arch-imx/cpu.h" >> >> -#define soc_rev() (get_cpu_rev() & 0xFF) >> +#define soc_rev() ((int)(get_cpu_rev() & 0xFF)) > > This seems unrelated change, isn't it? > Since get_cpu_rev returns unsigned int, this was causing a mix of unsigned int and int across binary operators. e.g: if(soc_rev() >= CHIP_REV_1_5) ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images 2014-09-03 1:36 ` Nitin Garg @ 2014-09-03 1:41 ` Otavio Salvador 2014-09-03 1:47 ` Nitin Garg 0 siblings, 1 reply; 13+ messages in thread From: Otavio Salvador @ 2014-09-03 1:41 UTC (permalink / raw) To: u-boot Hello Nitin, On Tue, Sep 2, 2014 at 10:36 PM, Nitin Garg <nitin.garg@freescale.com> wrote: > On 08/31/2014 08:09 PM, Otavio Salvador wrote: >>> diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h b/arch/arm/include/asm/arch-mx6/sys_proto.h >>> index 306d699..2bbb86e 100644 >>> --- a/arch/arm/include/asm/arch-mx6/sys_proto.h >>> +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h >> ... >>> @@ -11,7 +13,7 @@ >>> #include <asm/imx-common/regs-common.h> >>> #include "../arch-imx/cpu.h" >>> >>> -#define soc_rev() (get_cpu_rev() & 0xFF) >>> +#define soc_rev() ((int)(get_cpu_rev() & 0xFF)) >> >> This seems unrelated change, isn't it? >> > Since get_cpu_rev returns unsigned int, this was causing > a mix of unsigned int and int across binary operators. > > e.g: > if(soc_rev() >= CHIP_REV_1_5) In this case, please split this change. Shouldn't this to be fixed in the get_cpu_rev? Cheers, -- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://code.ossystems.com.br Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images 2014-09-03 1:41 ` Otavio Salvador @ 2014-09-03 1:47 ` Nitin Garg 2014-09-03 1:49 ` Otavio Salvador 2014-09-03 1:52 ` Fabio Estevam 0 siblings, 2 replies; 13+ messages in thread From: Nitin Garg @ 2014-09-03 1:47 UTC (permalink / raw) To: u-boot On 09/02/2014 08:41 PM, Otavio Salvador wrote: > Hello Nitin, > > On Tue, Sep 2, 2014 at 10:36 PM, Nitin Garg <nitin.garg@freescale.com> wrote: >> On 08/31/2014 08:09 PM, Otavio Salvador wrote: >>>> diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h b/arch/arm/include/asm/arch-mx6/sys_proto.h >>>> index 306d699..2bbb86e 100644 >>>> --- a/arch/arm/include/asm/arch-mx6/sys_proto.h >>>> +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h >>> ... >>>> @@ -11,7 +13,7 @@ >>>> #include <asm/imx-common/regs-common.h> >>>> #include "../arch-imx/cpu.h" >>>> >>>> -#define soc_rev() (get_cpu_rev() & 0xFF) >>>> +#define soc_rev() ((int)(get_cpu_rev() & 0xFF)) >>> >>> This seems unrelated change, isn't it? >>> >> Since get_cpu_rev returns unsigned int, this was causing >> a mix of unsigned int and int across binary operators. >> >> e.g: >> if(soc_rev() >= CHIP_REV_1_5) > > In this case, please split this change. > > Shouldn't this to be fixed in the get_cpu_rev? > > Cheers, > But get_cpu_rev is correct, it returns unsigned int. The problem happens in hab code where there are comparisons between int and unsigned int, hence I think it should not be split. Pls advice. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images 2014-09-03 1:47 ` Nitin Garg @ 2014-09-03 1:49 ` Otavio Salvador 2014-09-03 1:52 ` Fabio Estevam 1 sibling, 0 replies; 13+ messages in thread From: Otavio Salvador @ 2014-09-03 1:49 UTC (permalink / raw) To: u-boot On Tue, Sep 2, 2014 at 10:47 PM, Nitin Garg <nitin.garg@freescale.com> wrote: > On 09/02/2014 08:41 PM, Otavio Salvador wrote: >> Hello Nitin, >> >> On Tue, Sep 2, 2014 at 10:36 PM, Nitin Garg <nitin.garg@freescale.com> wrote: >>> On 08/31/2014 08:09 PM, Otavio Salvador wrote: >>>>> diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h b/arch/arm/include/asm/arch-mx6/sys_proto.h >>>>> index 306d699..2bbb86e 100644 >>>>> --- a/arch/arm/include/asm/arch-mx6/sys_proto.h >>>>> +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h >>>> ... >>>>> @@ -11,7 +13,7 @@ >>>>> #include <asm/imx-common/regs-common.h> >>>>> #include "../arch-imx/cpu.h" >>>>> >>>>> -#define soc_rev() (get_cpu_rev() & 0xFF) >>>>> +#define soc_rev() ((int)(get_cpu_rev() & 0xFF)) >>>> >>>> This seems unrelated change, isn't it? >>>> >>> Since get_cpu_rev returns unsigned int, this was causing >>> a mix of unsigned int and int across binary operators. >>> >>> e.g: >>> if(soc_rev() >= CHIP_REV_1_5) >> >> In this case, please split this change. >> >> Shouldn't this to be fixed in the get_cpu_rev? >> >> Cheers, >> > But get_cpu_rev is correct, it returns unsigned int. > The problem happens in hab code where there are > comparisons between int and unsigned int, hence > I think it should not be split. Pls advice. Well, this is not up to me however in this case wouldn't be better to fix the HAB code? -- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://code.ossystems.com.br Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images 2014-09-03 1:47 ` Nitin Garg 2014-09-03 1:49 ` Otavio Salvador @ 2014-09-03 1:52 ` Fabio Estevam 2014-09-03 1:55 ` Nitin Garg 1 sibling, 1 reply; 13+ messages in thread From: Fabio Estevam @ 2014-09-03 1:52 UTC (permalink / raw) To: u-boot On Tue, Sep 2, 2014 at 10:47 PM, Nitin Garg <nitin.garg@freescale.com> wrote: > But get_cpu_rev is correct, it returns unsigned int. > The problem happens in hab code where there are > comparisons between int and unsigned int, hence Where exactly in the hab code does the problem happen? ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images 2014-09-03 1:52 ` Fabio Estevam @ 2014-09-03 1:55 ` Nitin Garg 2014-09-03 2:54 ` Otavio Salvador 2014-09-04 0:10 ` Fabio Estevam 0 siblings, 2 replies; 13+ messages in thread From: Nitin Garg @ 2014-09-03 1:55 UTC (permalink / raw) To: u-boot On 09/02/2014 08:52 PM, Fabio Estevam wrote: > On Tue, Sep 2, 2014 at 10:47 PM, Nitin Garg <nitin.garg@freescale.com> wrote: > >> But get_cpu_rev is correct, it returns unsigned int. >> The problem happens in hab code where there are >> comparisons between int and unsigned int, hence > > Where exactly in the hab code does the problem happen? > In the macros of HAB, like: hab_rvt_report_event_p The compiler generates bhi instead of bgt. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images 2014-09-03 1:55 ` Nitin Garg @ 2014-09-03 2:54 ` Otavio Salvador 2014-09-04 0:10 ` Fabio Estevam 1 sibling, 0 replies; 13+ messages in thread From: Otavio Salvador @ 2014-09-03 2:54 UTC (permalink / raw) To: u-boot On Tue, Sep 2, 2014 at 10:55 PM, Nitin Garg <nitin.garg@freescale.com> wrote: > On 09/02/2014 08:52 PM, Fabio Estevam wrote: >> On Tue, Sep 2, 2014 at 10:47 PM, Nitin Garg <nitin.garg@freescale.com> wrote: >> >>> But get_cpu_rev is correct, it returns unsigned int. >>> The problem happens in hab code where there are >>> comparisons between int and unsigned int, hence >> >> Where exactly in the hab code does the problem happen? >> > In the macros of HAB, like: > hab_rvt_report_event_p > > The compiler generates bhi instead of bgt. However looking at the code I didn't see any negative CHIP_REV: #define CHIP_REV_1_0 0x10 #define CHIP_REV_1_2 0x12 #define CHIP_REV_1_5 0x15 So I am not sure we have a failure case in the U-Boot code now. I think I am missing something though ... -- Otavio Salvador O.S. Systems http://www.ossystems.com.br http://code.ossystems.com.br Mobile: +55 (53) 9981-7854 Mobile: +1 (347) 903-9750 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images 2014-09-03 1:55 ` Nitin Garg 2014-09-03 2:54 ` Otavio Salvador @ 2014-09-04 0:10 ` Fabio Estevam 2014-09-04 1:10 ` Nitin Garg 1 sibling, 1 reply; 13+ messages in thread From: Fabio Estevam @ 2014-09-04 0:10 UTC (permalink / raw) To: u-boot On Tue, Sep 2, 2014 at 10:55 PM, Nitin Garg <nitin.garg@freescale.com> wrote: > In the macros of HAB, like: > hab_rvt_report_event_p > > The compiler generates bhi instead of bgt. hab_rvt_report_event_p exists prior to this patch. Is the issue present in current code then? If so, it should be handled on a separate patch. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images 2014-09-04 0:10 ` Fabio Estevam @ 2014-09-04 1:10 ` Nitin Garg 0 siblings, 0 replies; 13+ messages in thread From: Nitin Garg @ 2014-09-04 1:10 UTC (permalink / raw) To: u-boot On 09/03/2014 07:10 PM, Fabio Estevam wrote: > On Tue, Sep 2, 2014 at 10:55 PM, Nitin Garg <nitin.garg@freescale.com> wrote: > >> In the macros of HAB, like: >> hab_rvt_report_event_p >> >> The compiler generates bhi instead of bgt. > > hab_rvt_report_event_p exists prior to this patch. Is the issue > present in current code then? > > If so, it should be handled on a separate patch. > I will remove the typecast as its not essential (I stepped through the code today and its fine). ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images 2014-08-31 20:16 ` [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images nitin.garg at freescale.com 2014-09-01 1:09 ` Otavio Salvador @ 2014-09-01 5:29 ` Wolfgang Denk 1 sibling, 0 replies; 13+ messages in thread From: Wolfgang Denk @ 2014-09-01 5:29 UTC (permalink / raw) To: u-boot Dear nitin.garg at freescale.com, In message <1409516179-32553-2-git-send-email-nitin.garg@freescale.com> you wrote: > > + reg = readl(&ccm_regs->CCGR0); > + reg |= (MXC_CCM_CCGR0_CAAM_WRAPPER_IPG_MASK | > + MXC_CCM_CCGR0_CAAM_WRAPPER_ACLK_MASK | > + MXC_CCM_CCGR0_CAAM_SECURE_MEM_MASK); > + writel(reg, &ccm_regs->CCGR0); Is there any specific reason for not using setbits_le32() instead? > + /* Enable EMI slow clk */ > + reg = readl(&ccm_regs->CCGR6); > + reg |= MXC_CCM_CCGR6_EMI_SLOW_MASK; > + writel(reg, &ccm_regs->CCGR6); Ditto? > + reg = readl(&ccm_regs->CCGR0); > + reg &= ~(MXC_CCM_CCGR0_CAAM_WRAPPER_IPG_MASK | > + MXC_CCM_CCGR0_CAAM_WRAPPER_ACLK_MASK | > + MXC_CCM_CCGR0_CAAM_SECURE_MEM_MASK); > + writel(reg, &ccm_regs->CCGR0); Why not using clrbits_le32() instead? > + /* Disable EMI slow clk */ > + reg = readl(&ccm_regs->CCGR6); > + reg &= ~MXC_CCM_CCGR6_EMI_SLOW_MASK; > + writel(reg, &ccm_regs->CCGR6); Ditto? > +/* enable HAB (High Assurance Boot) debug */ > +/* #define DEBUG_AUTHENTICATE_IMAGE */ > + Please do not add dead code. > +#ifdef DEBUG_AUTHENTICATE_IMAGE > +void dump_mem(uint32_t addr, int size) > +{ > + int i; > + > + for (i = 0; i < size; i += 4) { > + if (i != 0) { > + if (i % 16 == 0) > + printf("\n"); > + else > + printf(" "); > + } > + > + printf("0x%08x", *(uint32_t *)addr); > + addr += 4; > + } > + > + printf("\n"); > + > + return; > +} > +#endif Is this really needed? Do we not already have such code, like print_buffer() ? > @@ -60,4 +62,8 @@ 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); > +#ifdef CONFIG_SECURE_BOOT > +void hab_caam_clock_enable(void); > +void hab_caam_clock_disable(void); > +#endif You can drop the #ifdef for prototypes. > --- a/arch/arm/include/asm/arch-mx6/sys_proto.h > +++ b/arch/arm/include/asm/arch-mx6/sys_proto.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+ > */ > > @@ -11,7 +13,7 @@ > #include <asm/imx-common/regs-common.h> > #include "../arch-imx/cpu.h" > > -#define soc_rev() (get_cpu_rev() & 0xFF) > +#define soc_rev() ((int)(get_cpu_rev() & 0xFF)) Claiming copyright for adding a single cast is a bit excessive, isn't it? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Your csh still thinks true is false. Write to your vendor today and tell them that next year Configure ought to "rm /bin/csh" unless they fix their blasted shell. :-) - Larry Wall in Configure from the perl distribution ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-09-04 1:10 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-08-31 20:16 [U-Boot] [PATCH] i.MX6 High Assurance Boot (HAB) support nitin.garg at freescale.com 2014-08-31 20:16 ` [U-Boot] [PATCH] Support i.MX6 High Assurance Boot (HAB) authentication of images nitin.garg at freescale.com 2014-09-01 1:09 ` Otavio Salvador 2014-09-03 1:36 ` Nitin Garg 2014-09-03 1:41 ` Otavio Salvador 2014-09-03 1:47 ` Nitin Garg 2014-09-03 1:49 ` Otavio Salvador 2014-09-03 1:52 ` Fabio Estevam 2014-09-03 1:55 ` Nitin Garg 2014-09-03 2:54 ` Otavio Salvador 2014-09-04 0:10 ` Fabio Estevam 2014-09-04 1:10 ` Nitin Garg 2014-09-01 5:29 ` Wolfgang Denk
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox