Hi York, Thanks for your comments and suggestion!
> -----Original Message----- > From: York Sun [mailto:york....@nxp.com] > Sent: 2016年5月28日 2:06 > To: Zhiqiang Hou <zhiqiang....@nxp.com>; u-boot@lists.denx.de; > albert.u.b...@aribaud.net; scottw...@freescale.com; > mingkai...@freescale.com; york...@freescale.com; le...@freescale.com; > prabha...@freescale.com; bhupesh.sha...@freescale.com > Cc: Hou Zhiqiang <zhiqiag....@nxp.com> > Subject: Re: [PATCHv4 3/7] ARMv8/layerscape: Add FSL PPA support > > On 05/23/2016 04:48 AM, Zhiqiang Hou wrote: > > From: Hou Zhiqiang <zhiqiang....@nxp.com> > > > > The FSL Primary Protected Application (PPA) is a software component > > loaded during boot which runs in TrustZone and remains resident after > > boot. > > > > Signed-off-by: Hou Zhiqiang <zhiqiag....@nxp.com> > > --- > > V4: > > - Moved secure firmware validation API to this patch. > > - Moved secure firmware getting supported PSCI version API to this patch. > > > > V3: > > - Refactor the code. > > - Add PPA firmware version info output. > > > <snip> > > > +int sec_firmware_validate(void) > > +{ > > + void *ppa_addr; > > + > > +#ifdef CONFIG_SYS_LS_PPA_FW_IN_NOR > > + ppa_addr = (void *)CONFIG_SYS_LS_PPA_FW_ADDR; #else #error "No > > +CONFIG_SYS_LS_PPA_FW_IN_xxx defined" > > +#endif > > + > > + return ppa_firmware_validate(ppa_addr); } > > You are returning 0 when the image is valid. This function name is confusing. > How about declare it as bool, and change the name to is_sec_firmware_valid()? Yes, will correct the confusing name next version. > > + > > +#ifdef CONFIG_ARMV8_PSCI > > +unsigned int sec_firmware_support_psci_version(void) > > +{ > > + unsigned int psci_ver = 0; > > + > > + if (!sec_firmware_validate()) > > + psci_ver = ppa_support_psci_version(); > > + > > + return psci_ver; > > +} > > +#endif > > + > > +int ppa_init_pre(u64 *entry) > > +{ > > + u64 ppa_ram_addr; > > + const void *raw_image_addr; > > + size_t raw_image_size = 0; > > + size_t ppa_ram_size = ppa_get_dram_block_size(); > > + int ret; > > + > > + debug("fsl-ppa: ppa size(0x%lx)\n", ppa_ram_size); > > + > > + /* > > + * The PPA must be stored in secure memory. > > + * Append PPA to secure mmu table. > > + */ > > + ppa_ram_addr = (gd->secure_ram & > MEM_RESERVE_SECURE_ADDR_MASK) + > > + gd->arch.tlb_size; > > + > > Should we check "if (gd->secure_ram & MEM_RESERVE_SECURE_SECURED)" > before using the secure memory? Yes, will add the check next version. Thanks, Zhiqiang _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
