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

[aneesh.bansal at freescale.com]

> -----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
>