[U-Boot] [PATCH 1/3] fsl: PPA: add support PPA image validation from NAND and SD

[Sumit Garg]

> -----Original Message-----
> From: York Sun [mailto:york.sun at nxp.com]
> Sent: Tuesday, April 11, 2017 9:09 PM
> To: Sumit Garg <sumit.garg at nxp.com>; u-boot at lists.denx.de
> Cc: Ruchika Gupta <ruchika.gupta at nxp.com>; Prabhakar Kushwaha
> <prabhakar.kushwaha at nxp.com>; Vini Pillai <vinitha.pillai at nxp.com>; Udit
> Agarwal <udit.agarwal at nxp.com>
> Subject: Re: [PATCH 1/3] fsl: PPA: add support PPA image validation from
> NAND and SD
> 
> On 04/11/2017 04:59 AM, Sumit Garg wrote:
> >> -----Original Message-----
> >> From: York Sun [mailto:york.sun at nxp.com]
> >> Sent: Tuesday, April 11, 2017 11:44 AM
> >> To: Sumit Garg <sumit.garg at nxp.com>; u-boot at lists.denx.de
> >> Cc: Ruchika Gupta <ruchika.gupta at nxp.com>; Prabhakar Kushwaha
> >> <prabhakar.kushwaha at nxp.com>; Vini Pillai <vinitha.pillai at nxp.com>;
> >> Udit Agarwal <udit.agarwal at nxp.com>
> >> Subject: Re: [PATCH 1/3] fsl: PPA: add support PPA image validation
> >> from NAND and SD
> >>
> >> On 04/10/2017 10:54 PM, Sumit Garg wrote:
> >>>> -----Original Message-----
> >>>> From: York Sun [mailto:york.sun at nxp.com]
> >>>> Sent: Monday, April 10, 2017 10:39 PM
> >>>> To: Sumit Garg <sumit.garg at nxp.com>; u-boot at lists.denx.de
> >>>> Cc: Ruchika Gupta <ruchika.gupta at nxp.com>; Prabhakar Kushwaha
> >>>> <prabhakar.kushwaha at nxp.com>; Vini Pillai <vinitha.pillai at nxp.com>;
> >>>> Udit Agarwal <udit.agarwal at nxp.com>
> >>>> Subject: Re: [PATCH 1/3] fsl: PPA: add support PPA image validation
> >>>> from NAND and SD
> >>>>
> >>>> On 04/07/2017 04:41 AM, Sumit Garg wrote:
> >>>>> Signed-off-by: Sumit Garg <sumit.garg at nxp.com>
> >>>>> Signed-off-by: Udit Agarwal <udit.agarwal at nxp.com>
> >>>>> Tested-by: Vinitha Pillai <vinitha.pillai at nxp.com>
> >>>>> ---
> >>>>>  arch/arm/cpu/armv8/fsl-layerscape/ppa.c | 67
> >>>>> ++++++++++++++++++++++++++++++++-
> >>>>>  1 file changed, 66 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
> >>>>> b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
> >>>>> index 7f87bb8..d8f1d36 100644
> >>>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
> >>>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c
> >>>>> @@ -39,6 +39,10 @@ int ppa_init(void)  #ifdef
> CONFIG_CHAIN_OF_TRUST
> >>>>>  	uintptr_t ppa_esbc_hdr = CONFIG_SYS_LS_PPA_ESBC_ADDR;
> >>>>
> >>>> For MMC and NAND, this CONFIG_SYS_LS_PPA_ESBC_ADDR is actually
> not
> >>>> used.
> >>>> Shall we move the assignment down to XIP section? The Kconfig
> >>>> should also be updated.
> >>>>
> >>>> York
> >>>
> >>> As per PPA verification patch for eMMC/SD and NAND:
> >>>
> >>> --- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
> >>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
> >>> @@ -179,12 +179,22 @@ config SYS_LS_PPA_ESBC_ADDR
> >>>  	default 0x40740000 if SYS_LS_PPA_FW_IN_XIP && ARCH_LS1046A
> >>>  	default 0x40480000 if SYS_LS_PPA_FW_IN_XIP && ARCH_LS1012A
> >>>  	default 0x580c40000 if SYS_LS_PPA_FW_IN_XIP && FSL_LSCH3
> >>> +	default 0x700000 if SYS_LS_PPA_FW_IN_MMC
> >>> +	default 0x700000 if SYS_LS_PPA_FW_IN_NAND
> >>>  	help
> >>>  	  If the PPA header firmware locate at XIP flash, such as NOR or
> >>>  	  QSPI flash, this address is a directly memory-mapped.
> >>>  	  If it is in a serial accessed flash, such as NAND and SD
> >>>  	  card, it is a byte offset.
> >>>
> >>> CONFIG_SYS_LS_PPA_ESBC_ADDR is used to provide offset on eMMC/SD
> >> and NAND.
> >>>
> >>
> >> That's exactly what I was referring to. You are _NOT_ using
> >> CONFIG_SYS_LS_PPA_ESBC_ADDR value for eMMC/SD or NAND. Instead,
> you
> >> use malloc to get a new memory for the header, which I agree it is right to
> do.
> >> So the macro CONFIG_SYS_LS_PPA_ESBC_ADDR is not used for either MMC
> >> or NAND.
> >>
> >> York
> >
> > Actually I am using CONFIG_SYS_LS_PPA_ESBC_ADDR in case of eMMC/SD or
> NAND as follows:
> >
> > For eMMC/SD:
> > +	blk = CONFIG_SYS_LS_PPA_ESBC_ADDR >> 9;
> > +	cnt = DIV_ROUND_UP(CONFIG_LS_PPA_ESBC_HDR_SIZE, 512);
> > +	ret = mmc->block_dev.block_read(&mmc->block_dev, blk, cnt,
> ppa_hdr_ddr);
> > +	if (ret != cnt) {
> > +		free(ppa_hdr_ddr);
> > +		printf("MMC/SD read of PPA header failed\n");
> > +		return -EIO;
> > +	}
> >
> > For NAND:
> > +	ret = nand_read(nand_info[0],
> (loff_t)CONFIG_SYS_LS_PPA_ESBC_ADDR,
> > +		       &fw_length, (u_char *)ppa_hdr_ddr);
> > +	if (ret == -EUCLEAN) {
> > +		free(ppa_hdr_ddr);
> > +		printf("NAND read of PPA firmware at offset 0x%x failed\n",
> > +		       CONFIG_SYS_LS_PPA_FW_ADDR);
> > +		return -EIO;
> > +	}
> >
> 
> So you are using this macro, but not using it directly as the way for XIP. It is
> confusing to see the assignment
> 
> ppa_esbc_hdr = CONFIG_SYS_LS_PPA_ESBC_ADDR;
> 
> I was suggesting to move this assignment down to the XIP section.
> 
> York
 
Yes you are correct, I will move this assignment to XIP section in next version of this patch-set.

Sumit