From mboxrd@z Thu Jan  1 00:00:00 1970
From: york sun <york.sun@nxp.com>
Date: Fri, 31 Mar 2017 19:25:41 +0000
Subject: [U-Boot] [PATCH 3/3][v3] [RESEND] arm: ls1046ardb: Add SD
 secure boot target
References: <1490815584-30551-1-git-send-email-ruchika.gupta@nxp.com>
Message-ID: <VI1PR0401MB2687CCC6DC8F766E9E221E8C9A370@VI1PR0401MB2687.eurprd04.prod.outlook.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 03/29/2017 07:21 AM, Ruchika Gupta wrote:
> From: Vinitha Pillai-B57223 <vinitha.pillai@nxp.com>
>
> - Add SD secure boot target for ls1046ardb.
> - Implement board specific spl_board_init() to setup CAAM stream ID and
>   corresponding stream ID in SMMU.
> - Change the u-boot size defined by a macro for copying the main U-Boot by SPL
>   to also include the u-boot Secure Boot header size as header is appended to
>   u-boot image. So header will also be copied from SD to DDR.
> - CONFIG_MAX_SPL_SIZE is limited to 90K.SPL is copied to OCRAM (128K) where 32K
>   are reserved for use by boot ROM and 6K for the header
> - Reduce the size of CAAM driver for SPL. Since the size of spl image
>   was about 94K, Blobification functions and descriptors, that are not required
>   at the time of SPL are disabled. Further error code conversion to strings
>   is disabled for SPL build. This reduces the spl image size to 92K.
>
> Signed-off-by: Vinitha Pillai <vinitha.pillai@nxp.com>
> Signed-off-by: Sumit Garg <sumit.garg@nxp.com>
> Signed-off-by: Ruchika Gupta <ruchika.gupta@nxp.com>
> ---
> Changes from v1:
> - Rebased patches to latest dependent patch set
> - With the dependent path set , spl imag size increased to 94K. So
> - additionally  reduce the spl image size by removing the functions from
> - CAAM driver that are not required in SPL flow
>
>

<snip>

>
>  arch/arm/include/asm/fsl_secure_boot.h          |  2 +-
>  board/freescale/ls1046ardb/ls1046ardb.c         | 19 +++++++++++
>  configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig | 45 +++++++++++++++++++++++++
>  drivers/crypto/fsl/jobdesc.c                    |  4 +--
>  drivers/crypto/fsl/jr.c                         | 19 ++++++-----
>  include/configs/ls1046a_common.h                | 17 ++++++++--
>  6 files changed, 91 insertions(+), 15 deletions(-)
>  create mode 100644 configs/ls1046ardb_sdcard_SECURE_BOOT_defconfig
>

<snip>

> diff --git a/drivers/crypto/fsl/jobdesc.c b/drivers/crypto/fsl/jobdesc.c
> index 6125bbb..375ff9d 100644
> --- a/drivers/crypto/fsl/jobdesc.c
> +++ b/drivers/crypto/fsl/jobdesc.c
> @@ -204,7 +204,7 @@ void inline_cnstr_jobdesc_hash(uint32_t *desc,
>  	append_store(desc, dma_addr_out, storelen,
>  		     LDST_CLASS_2_CCB | LDST_SRCDST_BYTE_CONTEXT);
>  }
> -
> +#ifndef CONFIG_SPL_BUILD
>  void inline_cnstr_jobdesc_blob_encap(uint32_t *desc, uint8_t *key_idnfr,
>  				     uint8_t *plain_txt, uint8_t *enc_blob,
>  				     uint32_t in_sz)
> @@ -252,7 +252,7 @@ void inline_cnstr_jobdesc_blob_decap(uint32_t *desc, uint8_t *key_idnfr,
>
>  	append_operation(desc, OP_TYPE_DECAP_PROTOCOL | OP_PCLID_BLOB);
>  }
> -
> +#endif

Why do you have this change in _this_ patch?

>  /*
>   * Descriptor to instantiate RNG State Handle 0 in normal mode and
>   * load the JDKEK, TDKEK and TDSK registers
> diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c
> index 1b88229..163e729 100644
> --- a/drivers/crypto/fsl/jr.c
> +++ b/drivers/crypto/fsl/jr.c
> @@ -342,7 +342,9 @@ static void desc_done(uint32_t status, void *arg)
>  {
>  	struct result *x = arg;
>  	x->status = status;
> +#ifndef CONFIG_SPL_BUILD
>  	caam_jr_strstatus(status);
> +#endif

Same question here.

>  	x->done = 1;
>  }
>
> @@ -436,7 +438,11 @@ static inline int sec_reset_idx(uint8_t sec_idx)
>
>  	return 0;
>  }
> -
> +int sec_reset(void)
> +{
> +	return sec_reset_idx(0);
> +}
> +#ifndef CONFIG_SPL_BUILD
>  static int instantiate_rng(uint8_t sec_idx)
>  {
>  	struct result op;
> @@ -472,11 +478,6 @@ static int instantiate_rng(uint8_t sec_idx)
>  	return ret;
>  }
>
> -int sec_reset(void)
> -{
> -	return sec_reset_idx(0);
> -}
> -
>  static u8 get_rng_vid(uint8_t sec_idx)
>  {
>  	ccsr_sec_t *sec = (void *)SEC_ADDR(sec_idx);
> @@ -561,7 +562,7 @@ static int rng_init(uint8_t sec_idx)
>
>  	return ret;
>  }
> -
> +#endif
>  int sec_init_idx(uint8_t sec_idx)
>  {
>  	ccsr_sec_t *sec = (void *)SEC_ADDR(sec_idx);
> @@ -634,7 +635,7 @@ int sec_init_idx(uint8_t sec_idx)
>
>  	pamu_enable();
>  #endif
> -
> +#ifndef CONFIG_SPL_BUILD
>  	if (get_rng_vid(sec_idx) >= 4) {
>  		if (rng_init(sec_idx) < 0) {
>  			printf("SEC%u: RNG instantiation failed\n", sec_idx);
> @@ -642,7 +643,7 @@ int sec_init_idx(uint8_t sec_idx)
>  		}
>  		printf("SEC%u: RNG instantiated\n", sec_idx);
>  	}
> -
> +#endif
>  	return ret;
>  }
>

You seem to have some non-LS1046ARDB specific change in this patch. I 
can understand if you need to make some change to support SPL 
validation. But don't you need these changes to get LS1043ARDB work as 
your patch #1? Maybe you can reorganize your change set to have one 
patch before your #1 patch (including the change you made to 
fsl_validate.c) in the first patch). The bottom of line is every single 
commit should serve one purpose and it should work after each commit.

York