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

[York Sun]

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