[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: Thursday, March 05, 2015 2:41 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 23:46 -0600, Bansal Aneesh-B39320 wrote:
> >
> >
> > Regards,
> > Aneesh Bansal
> >
> > > -----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.
> 
> Note that on b4860qds, it's SRIO2_MEM_VIRT that conflicts with 0xbff00000,
> not PCIE1_MEM_VIRT.
> 
This is taken care now in v5, where we have defined the addresses in corenet_ds.h.
So this will not impact other platforms like B4860.
> > >
> > > > 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.
> 
> Where does the 3.5G limitation come from?  Even if the physical address
> needs to be elsewhere due to bootrom constraints, we should be able to
> map it wherever we want in the TLB once U-Boot takes control.
> 
The 3.5G limitation comes from BootROM in case of secure Boot.
Initially U-Boot has to run from CPC configured as SRAM with address
Within 3.5G. Once U-boot has relocated to DDR, we have removed the
Corresponding TLB entry.
> > > If it is hardcoded, and we don't want to change the PCIE1 virtual
> > > address,
> 
> Actually it's PCIE2 that conflicts -- U-Boot just happens to use one big TLB
> entry to cover multiple PCIEs.
> 
> > > 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);
> 
> If you can't remove the 3.5G limitation, yes.
> 
> > 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
> 
> No, they are board-specific and need to be defined in the same place that
> the rest of the address map is defined (which I see you did in the
> v5 patch).
> 
> -Scott
>