[U-Boot] [PATCH v4 1/2] powerpc/mpc85xx: SECURE BOOT- Enable chain of trust in SPL
Sumit Garg
sumit.garg at nxp.com
Fri Jun 10 06:21:24 CEST 2016
Hi Andreas, Simon,
> -----Original Message-----
> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
> Sent: Friday, June 10, 2016 6:05 AM
> To: Andreas Dannenberg <dannenberg at ti.com>
> Cc: Sumit Garg <sumit.garg at nxp.com>; U-Boot Mailing List <u-
> boot at lists.denx.de>; york sun <york.sun at nxp.com>; Ruchika Gupta
> <ruchika.gupta at nxp.com>; Prabhakar Kushwaha
> <prabhakar.kushwaha at nxp.com>; Teddy Reed V <teddy.reed at gmail.com>;
> Aneesh Bansal <aneesh.bansal at nxp.com>
> Subject: Re: [PATCH v4 1/2] powerpc/mpc85xx: SECURE BOOT- Enable chain of
> trust in SPL
>
> Hi,
>
> On 9 June 2016 at 09:05, Andreas Dannenberg <dannenberg at ti.com> wrote:
> > Hi Sumit,
> >
> > On Thu, Jun 02, 2016 at 08:16:37AM -0400, Sumit Garg wrote:
> >> As part of Chain of Trust for Secure boot, the SPL U-Boot will
> >> validate the next level U-boot image. Add a new function
> >> spl_validate_uboot to perform the validation.
> >
> > I noticed your patch series just now and I certainly don't want to
> > derail what you are trying to do here but I wanted to see what you
> > think about something that's pretty much related.
> >
> > In order to be more flexible in how we go from SPL to U-Boot (we have
> > a need to inject ROM-API calls for decryption in addition to
> > authentication) I've been experimenting with a method that basically
> > injects a generic post-process function call into
> > spl_load_simple_fit() that the platform-specific layer can
> > provide/override. This function will basically get invoked on the
> > U-Boot firmware itself as well as for the selected DTB after it gets
> > loaded, allowing to do things like custom authentication and/or
> > decryption through plugging in for example our ROM API calls. But this
> > should also be a starting point for someone to plug in the U-Boot RSA
> > libraries directly in a way that doesn't require using CONFIG_SPL_DM (for
> memory-constrained devices).
It’s a good idea to plug ROM API calls for decryption and authentication in
U-boot firmware. I think your main concern is DM framework in SPL for
memory constrained devices. But for our platforms memory constraint is not a
barrier in this case, so for time being we could use CONFIG_SPL_DM. So if you
come up with framework to add U-Boot RSA libraries without using DM in SPL,
we could enable it on our platforms too.
> >
> > So mostly I wanted to see if this is something that could be useful
> > for you as well, and any comments/concerns you may have on that.
> >
> > Thanks and Regards,
> >
> > --
> > Andreas Dannenberg
> > Texas Instruments Inc
> >
> >
> >> Enable hardware crypto operations in SPL using SEC block.
> >> In case of Secure Boot, PAMU is not bypassed. For allowing SEC block
> >> access to CPC configured as SRAM, configure PAMU.
> >>
> >> Reviewed-by: Ruchika Gupta <ruchika.gupta at nxp.com>
> >> Signed-off-by: Aneesh Bansal <aneesh.bansal at nxp.com>
> >> Signed-off-by: Sumit Garg <sumit.garg at nxp.com>
> >> ---
> >> Changes in v2:
> >> Patches rebased
> >>
> >> Changes in v3:
> >> Patches rebased
> >>
> >> Changes in v4:
> >> Generic changes in lib, drivers, common Makefiles removed from this
> >> patchset. Rebased this patchset on top of patch [1], so this patchset
> >> is dependent on patch [1].
> >>
> >> [1]https://patchwork.ozlabs.org/patch/627664/
> >>
> >> arch/powerpc/cpu/mpc8xxx/fsl_pamu.c | 8 +++++
> >> arch/powerpc/cpu/mpc8xxx/pamu_table.c | 8 +++++
> >> arch/powerpc/include/asm/fsl_secure_boot.h | 28 ++++++++++++++++
> >> board/freescale/common/fsl_chain_of_trust.c | 50
> +++++++++++++++++++++++++++++
> >> drivers/crypto/fsl/jr.c | 16 +++++++++
> >> drivers/mtd/nand/fsl_ifc_spl.c | 24 ++++++++++++++
> >> include/fsl_validate.h | 1 +
> >> 7 files changed, 135 insertions(+)
> >>
> >> diff --git a/arch/powerpc/cpu/mpc8xxx/fsl_pamu.c
> >> b/arch/powerpc/cpu/mpc8xxx/fsl_pamu.c
> >> index 9421f1e..ede8e66 100644
> >> --- a/arch/powerpc/cpu/mpc8xxx/fsl_pamu.c
> >> +++ b/arch/powerpc/cpu/mpc8xxx/fsl_pamu.c
> >> @@ -239,15 +239,23 @@ int pamu_init(void)
> >> spaact_size = sizeof(struct paace) * NUM_SPAACT_ENTRIES;
> >>
> >> /* Allocate space for Primary PAACT Table */
> >> +#if (defined(CONFIG_SPL_BUILD) &&
> defined(CONFIG_SPL_PPAACT_ADDR))
> >> + ppaact = (void *)CONFIG_SPL_PPAACT_ADDR; #else
> >> ppaact = memalign(PAMU_TABLE_ALIGNMENT, ppaact_size);
> >> if (!ppaact)
> >> return -1;
> >> +#endif
> >> memset(ppaact, 0, ppaact_size);
> >>
> >> /* Allocate space for Secondary PAACT Table */
> >> +#if (defined(CONFIG_SPL_BUILD) &&
> defined(CONFIG_SPL_SPAACT_ADDR))
> >> + sec = (void *)CONFIG_SPL_SPAACT_ADDR; #else
> >> sec = memalign(PAMU_TABLE_ALIGNMENT, spaact_size);
> >> if (!sec)
> >> return -1;
> >> +#endif
> >> memset(sec, 0, spaact_size);
> >>
> >> ppaact_phys = virt_to_phys((void *)ppaact); diff --git
> >> a/arch/powerpc/cpu/mpc8xxx/pamu_table.c
> >> b/arch/powerpc/cpu/mpc8xxx/pamu_table.c
> >> index 26c5ea4..a8e6f51 100644
> >> --- a/arch/powerpc/cpu/mpc8xxx/pamu_table.c
> >> +++ b/arch/powerpc/cpu/mpc8xxx/pamu_table.c
> >> @@ -28,6 +28,14 @@ void construct_pamu_addr_table(struct
> >> pamu_addr_tbl *tbl, int *num_entries)
> >>
> >> i++;
> >> #endif
> >> +#if (defined(CONFIG_SPL_BUILD) && (CONFIG_SYS_INIT_L3_VADDR))
> >> + tbl->start_addr[i] =
> >> + (uint64_t)virt_to_phys((void *)CONFIG_SYS_INIT_L3_VADDR);
> >> + tbl->size[i] = 256 * 1024; /* 256K CPC flash */
> >> + tbl->end_addr[i] = tbl->start_addr[i] + tbl->size[i] - 1;
> >> +
> >> + i++;
> >> +#endif
> >> debug("PAMU address\t\t\tsize\n");
> >> for (j = 0; j < i ; j++)
> >> debug("%llx \t\t\t%llx\n", tbl->start_addr[j],
> >> tbl->size[j]); diff --git
> >> a/arch/powerpc/include/asm/fsl_secure_boot.h
> >> b/arch/powerpc/include/asm/fsl_secure_boot.h
> >> index 826f9c9..99eec7f 100644
> >> --- a/arch/powerpc/include/asm/fsl_secure_boot.h
> >> +++ b/arch/powerpc/include/asm/fsl_secure_boot.h
> >> @@ -72,6 +72,32 @@
> >>
> >> #ifdef CONFIG_CHAIN_OF_TRUST
> >>
> >> +#ifdef CONFIG_SPL_BUILD
> >> +#define CONFIG_SPL_DM 1
> >> +#define CONFIG_SPL_CRYPTO_SUPPORT
> >> +#define CONFIG_SPL_HASH_SUPPORT
> >> +#define CONFIG_SPL_RSA
> >> +#define CONFIG_SPL_DRIVERS_MISC_SUPPORT
> >> +/*
> >> + * PPAACT and SPAACT table for PAMU must be placed on DDR after DDR
> >> +init
> >> + * due to space crunch on CPC and thus malloc will not work.
> >> + */
> >> +#define CONFIG_SPL_PPAACT_ADDR 0x2e000000
> >> +#define CONFIG_SPL_SPAACT_ADDR 0x2f000000
> >> +#define CONFIG_SPL_JR0_LIODN_S 454
> >> +#define CONFIG_SPL_JR0_LIODN_NS 458
> >> +/*
> >> + * Define the key hash for U-Boot here if public/private key pair
> >> +used to
> >> + * sign U-boot are different from the SRK hash put in the fuse
> >> + * Example of defining KEY_HASH is
> >> + * #define CONFIG_SPL_UBOOT_KEY_HASH \
> >> + *
> "41066b564c6ffcef40ccbc1e0a5d0d519604000c785d97bbefd25e4d288d1c8b"
> >> + * else leave it defined as NULL
> >> + */
> >> +
> >> +#define CONFIG_SPL_UBOOT_KEY_HASH NULL
> >> +#endif /* ifdef CONFIG_SPL_BUILD */
> >> +
> >> #define CONFIG_CMD_ESBC_VALIDATE
> >> #define CONFIG_CMD_BLOB
> >> #define CONFIG_FSL_SEC_MON
> >> @@ -82,6 +108,7 @@
> >> #define CONFIG_FSL_CAAM
> >> #endif
> >>
> >> +#ifndef CONFIG_SPL_BUILD
> >> /* fsl_setenv_chain_of_trust() must be called from
> >> * board_late_init()
> >> */
> >> @@ -119,5 +146,6 @@
> >> #endif /* #ifdef CONFIG_BOOTSCRIPT_COPY_RAM */
> >>
> >> #include <config_fsl_chain_trust.h>
> >> +#endif /* #ifndef CONFIG_SPL_BUILD */
> >> #endif /* #ifdef CONFIG_CHAIN_OF_TRUST */ #endif diff --git
> >> a/board/freescale/common/fsl_chain_of_trust.c
> >> b/board/freescale/common/fsl_chain_of_trust.c
> >> index ecfcc82..992babf 100644
> >> --- a/board/freescale/common/fsl_chain_of_trust.c
> >> +++ b/board/freescale/common/fsl_chain_of_trust.c
> >> @@ -6,7 +6,17 @@
> >>
> >> #include <common.h>
> >> #include <fsl_validate.h>
> >> +#include <fsl_secboot_err.h>
> >> #include <fsl_sfp.h>
> >> +#include <dm/root.h>
> >> +
> >> +#ifdef CONFIG_ADDR_MAP
> >> +#include <asm/mmu.h>
> >> +#endif
> >> +
> >> +#ifdef CONFIG_FSL_CORENET
> >> +#include <asm/fsl_pamu.h>
> >> +#endif
> >>
> >> #ifdef CONFIG_LS102XA
> >> #include <asm/arch/immap_ls102xa.h>
> >> @@ -52,6 +62,7 @@ int fsl_check_boot_mode_secure(void)
> >> return 0;
> >> }
> >>
> >> +#ifndef CONFIG_SPL_BUILD
> >> int fsl_setenv_chain_of_trust(void)
> >> {
> >> /* Check Boot Mode
> >> @@ -68,3 +79,42 @@ int fsl_setenv_chain_of_trust(void)
> >> setenv("bootcmd", CONFIG_CHAIN_BOOT_CMD);
> >> return 0;
> >> }
> >> +#endif
> >> +
> >> +#ifdef CONFIG_SPL_BUILD
> >> +void spl_validate_uboot(uint32_t hdr_addr, uintptr_t img_addr) {
> >> + int res;
> >> +
> >> + /* Check Boot Mode
> >> + * If Boot Mode is Non-Secure, skip validation
> >> + */
> >> + if (fsl_check_boot_mode_secure() == 0)
> >> + return;
> >> +
> >> + printf("SPL: Validating U-Boot image\n");
> >> +
> >> +#ifdef CONFIG_ADDR_MAP
> >> + init_addr_map();
> >> +#endif
> >> +
> >> +#ifdef CONFIG_FSL_CORENET
> >> + if (pamu_init() < 0)
> >> + fsl_secboot_handle_error(ERROR_ESBC_PAMU_INIT);
> >> +#endif
> >> +
> >> +#ifdef CONFIG_FSL_CAAM
> >> + if (sec_init() < 0)
> >> + fsl_secboot_handle_error(ERROR_ESBC_SEC_INIT);
> >> +#endif
> >> +
> >> +#if defined(CONFIG_DM)
> >> + dm_init_and_scan(false);
>
> Eek, this is duplicating code and adding a board-specific hack. I agree that it
> would be better to use SPL FIT to handle this.
Currently our powerpc based platforms do not use generic SPL framework but
uses board specific SPL framework (/board/freescale/<platform>/spl.c) and do not
use DM in SPL either. So to include RSA library I called it here once rather than
calling in every board specific file.
Yeah you are correct I should use CONFIG_SPL_DM here instead of CONFIG_DM.
>
> >> +#endif
> >> + res = fsl_secboot_validate(hdr_addr, CONFIG_SPL_UBOOT_KEY_HASH,
> >> + &img_addr);
> >> +
> >> + if (res == 0)
> >> + printf("SPL: Validation of U-boot successful\n"); }
> >> +#endif
> >> diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c index
> >> 510fa4e..1d4dd32 100644
> >> --- a/drivers/crypto/fsl/jr.c
> >> +++ b/drivers/crypto/fsl/jr.c
> >> @@ -599,10 +599,26 @@ int sec_init_idx(uint8_t sec_idx)
> >> sec_out32(&sec->mcfgr, mcr);
> >>
> >> #ifdef CONFIG_FSL_CORENET
> >> +#ifdef CONFIG_SPL_BUILD
> >> + /* For SPL Build, Set the Liodns in SEC JR0 for
> >> + * creating PAMU entries corresponding to these.
> >> + * For normal build, these are set in set_liodns().
> >> + */
> >> + liodn_ns = CONFIG_SPL_JR0_LIODN_NS & JRNSLIODN_MASK;
> >> + liodn_s = CONFIG_SPL_JR0_LIODN_S & JRSLIODN_MASK;
> >> +
> >> + liodnr = sec_in32(&sec->jrliodnr[0].ls) &
> >> + ~(JRNSLIODN_MASK | JRSLIODN_MASK);
> >> + liodnr = liodnr |
> >> + (liodn_ns << JRNSLIODN_SHIFT) |
> >> + (liodn_s << JRSLIODN_SHIFT);
> >> + sec_out32(&sec->jrliodnr[0].ls, liodnr); #else
> >> liodnr = sec_in32(&sec->jrliodnr[0].ls);
> >> liodn_ns = (liodnr & JRNSLIODN_MASK) >> JRNSLIODN_SHIFT;
> >> liodn_s = (liodnr & JRSLIODN_MASK) >> JRSLIODN_SHIFT; #endif
> >> +#endif
> >>
> >> ret = jr_init(sec_idx);
> >> if (ret < 0) {
> >> diff --git a/drivers/mtd/nand/fsl_ifc_spl.c b/drivers/mtd/nand/fsl_ifc_spl.c
> >> index cbeb74a..30aa966 100644
> >> --- a/drivers/mtd/nand/fsl_ifc_spl.c
> >> +++ b/drivers/mtd/nand/fsl_ifc_spl.c
> >> @@ -11,6 +11,9 @@
> >> #include <asm/io.h>
> >> #include <fsl_ifc.h>
> >> #include <linux/mtd/nand.h>
> >> +#ifdef CONFIG_CHAIN_OF_TRUST
> >> +#include <fsl_validate.h>
> >> +#endif
> >>
> >> static inline int is_blank(uchar *addr, int page_size)
> >> {
> >> @@ -268,6 +271,27 @@ void nand_boot(void)
> >> */
> >> flush_cache(CONFIG_SYS_NAND_U_BOOT_DST,
> CONFIG_SYS_NAND_U_BOOT_SIZE);
> >> #endif
> >> +
> >> +#ifdef CONFIG_CHAIN_OF_TRUST
> >> + /*
> >> + * As U-Boot header is appended at end of U-boot image, so
> >> + * calculate U-boot header address using U-boot header size.
> >> + */
> >> +#define CONFIG_U_BOOT_HDR_ADDR \
> >> + ((CONFIG_SYS_NAND_U_BOOT_START + \
> >> + CONFIG_SYS_NAND_U_BOOT_SIZE) - \
> >> + CONFIG_U_BOOT_HDR_SIZE)
> >> + spl_validate_uboot(CONFIG_U_BOOT_HDR_ADDR,
> >> + CONFIG_SYS_NAND_U_BOOT_START);
> >> + /*
> >> + * In case of failure in validation, spl_validate_uboot would
> >> + * not return back in case of Production environment with ITS=1.
> >> + * Thus U-Boot will not start.
> >> + * In Development environment (ITS=0 and SB_EN=1), the function
> >> + * may return back in case of non-fatal failures.
> >> + */
> >> +#endif
> >> +
> >> uboot = (void *)CONFIG_SYS_NAND_U_BOOT_START;
> >> uboot();
> >> }
> >> diff --git a/include/fsl_validate.h b/include/fsl_validate.h
> >> index a71e1ce..7695b30 100644
> >> --- a/include/fsl_validate.h
> >> +++ b/include/fsl_validate.h
> >> @@ -254,4 +254,5 @@ int fsl_secboot_blob_decap(cmd_tbl_t *cmdtp, int
> flag, int argc,
> >>
> >> int fsl_check_boot_mode_secure(void);
> >> int fsl_setenv_chain_of_trust(void);
> >> +void spl_validate_uboot(uint32_t hdr_addr, uintptr_t img_addr);
> >> #endif
> >> --
> >> 1.8.1.4
> >>
>
> Regards,
> Simon
Regards,
Sumit
More information about the U-Boot
mailing list