[U-Boot] [U-Boot, 1/2, v4] powerpc/mpc85xx: SECURE BOOT- NAND secure boot target for P3041

aneesh.bansal at freescale.com
2015-02-27T05:50:45Z

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, February 27, 2015 10:22 AM
> To: Bansal Aneesh-B39320
> Cc: u-boot at lists.denx.de; Sun York-R58495; Gupta Ruchika-R66431
> Subject: Re: [U-Boot, 1/2, v4] powerpc/mpc85xx: SECURE BOOT- NAND
> secure boot target for P3041
>
> On Thu, 2015-02-26 at 22:35 -0600, Bansal Aneesh-B39320 wrote:
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Thursday, February 26, 2015 3:43 AM
> > > To: Bansal Aneesh-B39320
> > > Cc: u-boot at lists.denx.de; Sun York-R58495; Gupta Ruchika-R66431
> > > Subject: Re: [U-Boot, 1/2, v4] powerpc/mpc85xx: SECURE BOOT- NAND
> > > secure boot target for P3041
> > >
> > > [Reposting comment on v4 as York requested]
> > >
> > > On Wed, Feb 25, 2015 at 02:17:56PM +0530, Aneesh Bansal wrote:
> > > > diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c
> > > > b/arch/powerpc/cpu/mpc85xx/cpu_init.c
> > > > index 4cf8853..ef56cc0 100644
> > > > --- a/arch/powerpc/cpu/mpc85xx/cpu_init.c
> > > > +++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c
> > > > @@ -843,6 +843,23 @@ int cpu_init_r(void)
> > > > setup_mp();
> > > > #endif
> > > >
> > > > +#if defined(CONFIG_SYS_RAMBOOT) &&
> > > defined(CONFIG_SYS_INIT_L3_ADDR) && \
> > > > + defined(CONFIG_SECURE_BOOT)
> > > > + /* Disable the TLB Created for L3 and create the TLB required for
> > > > + * PCIE (CONFIG_SYS_PCIE1_MEM_VIRT) which was not created
> > > earlier.
> > > > + */
> > > > + int tlb_index;
> > > > + tlb_index = find_tlb_idx((void *)CONFIG_BPTR_VIRT_ADDR, 1);
> > > > + if (tlb_index != -1) {
> > > > + disable_tlb(tlb_index);
> > > > +
> > > > + set_tlb(1, CONFIG_SYS_PCIE1_MEM_VIRT,
> > > > + CONFIG_SYS_PCIE1_MEM_PHYS,
> > > > + MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
> > > > + 0, tlb_index, BOOKE_PAGESZ_1G, 1);
> > > > + }
> > > > +#endif
> > >
> > > Why are you assuming in generic 85xx code that the TLB for PCIE1
> > > needs to be created? e500mc should have enough TLB1 entries that
> > > you don't need to share (or if it's due to address conflicts, a
> > > board may have PCI at a different address), and PCI may not exist
> > > at all on
> some boards.
> > >
> > > -Scott
> >
> > TLB's are created in freescale/common/p_corenet/tlb.c
>
> Which doesn't apply to all 85xx boards (even custom corenet-based
> boards might not use it -- or if that's not the case, it should be
> moved out of the board directory). It's also not obvious to anyone
> modifying that tlb.c file or the address of PCIE1 that this would be affected.
>
> > In case of Secure Boot, L3 is used as 1M SRAM and the address of the
> SRAM is at 0xbff00000.
>
> Is this hardcoded into the silicon, or determined by PBI or something similar?
> If it's not hardcoded, can we choose a less problematic address?
It is not hardcoded but we have a restriction of choosing the address within 0 - 3.5G.
0xbff00000 seemed to be the least problematic at this point of time.

> If it is hardcoded, and we don't want to change the PCIE1 virtual
> address, at least create defines for the entry to be created once SRAM
> goes away, rather than hardcoding PCIE1 here.
>
Are you suggesting something like this in cpu_init_r() set_tlb(1, CONFIG_SECBOOT_TLB_VIRT_ADDR,
CONFIG_SECBOOT_TLB_PHYS_ADDR,
CONFIG_SECBOOT_TLB_PERM, CONFIG_SECBOOT_TLB_ATTR,
0, tlb_index, CONFIG_SECBOOT_TLB_PAGESZ, 1);

I plan to define these macros in tlb.c where we have added the code for these TLBS creation

#define CONFIG_SECBOOT_TLB_VIRT_ADDR CONFIG_SYS_PCIE1_MEM_VIRT
#define CONFIG_SECBOOT_TLB_PHYS_ADDR CONFIG_SYS_PCIE1_MEM_PHYS
#define CONFIG_SECBOOT_TLB_PERM MAS3_SW|MAS3_SR
#define CONFIG_SECBOOT_TLB_ATTR MAS2_I|MAS2_G
#define CONFIG_SECBOOT_TLB_PAGESZ BOOKE_PAGESZ_1G
> -Scott
>