diff mbox

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

Message ID 1424854076-10387-1-git-send-email-aneesh.bansal@freescale.com
State Superseded
Delegated to: York Sun
Headers show

Commit Message

Aneesh Bansal Feb. 25, 2015, 8:47 a.m. UTC
Secure Boot Target is added for NAND for P3041.
Changes:
In PowerPC, the core begins execution from address 0xFFFFFFFC.
In case of secure boot, this default address maps to Boot ROM.
The Boot ROM code requires that the bootloader(U-boot) must lie
in 0 to 3.5G address space i.e. 0x0 - 0xDFFFFFFF.

In case of NAND Secure Boot, CONFIG_SYS_RAMBOOT is enabled and CPC is
configured as SRAM. U-Boot binary will be located on this SRAM at
location 0xBFF40000 with entry point as 0xBFFFFFFC.

Signed-off-by: Ruchika Gupta <ruchika.gupta@freescale.com>
Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
---
Changes in v4:
- Created a patch set.

 Makefile                                   |  4 ++++
 arch/powerpc/cpu/mpc85xx/cpu_init.c        | 17 +++++++++++++++++
 board/freescale/common/p_corenet/tlb.c     | 18 +++++++++++++++++-
 board/freescale/corenet_ds/MAINTAINERS     |  5 +++++
 configs/P3041DS_NAND_SECURE_BOOT_defconfig |  4 ++++
 include/configs/corenet_ds.h               |  9 +++++++++
 6 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 configs/P3041DS_NAND_SECURE_BOOT_defconfig

Comments

Scott Wood Feb. 25, 2015, 10:13 p.m. UTC | #1
[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
Aneesh Bansal Feb. 27, 2015, 4:35 a.m. UTC | #2
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, February 26, 2015 3:43 AM
> To: Bansal Aneesh-B39320
> Cc: u-boot@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
In case of Secure Boot, L3 is used as 1M SRAM and the address of the SRAM is at 0xbff00000.
The PCIE TLB entry conflicts with the above entry and so the entry for PCIE was not created
at that point of time.
This is why it is being created here in cpu_init_r() when U-Boot has relocated to DDR and the
TLB entry for SRAM is not required.
Proper comments have been added in tlb.c to explain why it is being done.
If address for PCI is changed than this also will have to be taken care of.
The code is protected by relevant macros (CONFIG_SYS_RAMBOOT, CONFIG_SYS_INIT_L3_ADDR,
CONFIG_SECURE_BOOT)
Scott Wood Feb. 27, 2015, 4:51 a.m. UTC | #3
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@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?

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.

-Scott
Aneesh Bansal Feb. 27, 2015, 5:50 a.m. UTC | #4
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, February 27, 2015 10:22 AM
> To: Bansal Aneesh-B39320
> Cc: u-boot@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@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
>
Scott Wood March 4, 2015, 9:10 p.m. UTC | #5
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@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@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.

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

> > 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
Aneesh Bansal March 5, 2015, 7:26 a.m. UTC | #6
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, March 05, 2015 2:41 AM
> To: Bansal Aneesh-B39320
> Cc: u-boot@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@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@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
>
Scott Wood March 5, 2015, 5:08 p.m. UTC | #7
On Thu, 2015-03-05 at 01:26 -0600, Bansal Aneesh-B39320 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Thursday, March 05, 2015 2:41 AM
> > To: Bansal Aneesh-B39320
> > Cc: u-boot@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
> > 
> > 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.

Again, you could relocate the virtual address of L3 much earlier.

-Scott
Aneesh Bansal March 10, 2015, 8:50 a.m. UTC | #8
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, March 05, 2015 10:38 PM
> To: Bansal Aneesh-B39320
> Cc: u-boot@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-03-05 at 01:26 -0600, Bansal Aneesh-B39320 wrote:
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Thursday, March 05, 2015 2:41 AM
> > > To: Bansal Aneesh-B39320
> > > Cc: u-boot@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
> > >
> > > 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.
> 
> Again, you could relocate the virtual address of L3 much earlier.
> 
> -Scott
> 
Are you suggesting the following:
1.  PBI Commands to configure CPC as SRAM with address 0xBFF0_0000.
2.  Compile U-boot with TEXT BASE as 0xFFF40000.
3.  Copy the U-boot from NAND via PBI commands to CPC (SRAM) on address 0xBFF4_0000
4.  The BootROM will validate the U-boot and transfer the control to 0xBFFF_FFFC.
5.  When U-boot is executing, then in the last 4K code, when shifting from AS=0 to AS=1, 
      we change the address of SRAM from 0xBFF0_0000 to 0xFFF0_0000. (Similar to what is done for NOR Boot)

- Aneesh
Scott Wood March 10, 2015, 5:03 p.m. UTC | #9
On Tue, 2015-03-10 at 03:50 -0500, Bansal Aneesh-B39320 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Thursday, March 05, 2015 10:38 PM
> > To: Bansal Aneesh-B39320
> > Cc: u-boot@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-03-05 at 01:26 -0600, Bansal Aneesh-B39320 wrote:
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Thursday, March 05, 2015 2:41 AM
> > > > To: Bansal Aneesh-B39320
> > > > Cc: u-boot@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
> > > >
> > > > 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.
> > 
> > Again, you could relocate the virtual address of L3 much earlier.
> > 
> > -Scott
> > 
> Are you suggesting the following:
> 1.  PBI Commands to configure CPC as SRAM with address 0xBFF0_0000.
> 2.  Compile U-boot with TEXT BASE as 0xFFF40000.
> 3.  Copy the U-boot from NAND via PBI commands to CPC (SRAM) on address 0xBFF4_0000
> 4.  The BootROM will validate the U-boot and transfer the control to 0xBFFF_FFFC.
> 5.  When U-boot is executing, then in the last 4K code, when shifting from AS=0 to AS=1, 
>       we change the address of SRAM from 0xBFF0_0000 to 0xFFF0_0000. (Similar to what is done for NOR Boot)

Something like that, except in step 5 it would only be changing the
virtual address, not the physical address (unless you can do a similar
trick as NOR does, to have the L3 cache repeat and cover both addresses
at once).

-Scott
Aneesh Bansal March 10, 2015, 5:52 p.m. UTC | #10
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, March 10, 2015 10:34 PM
> To: Bansal Aneesh-B39320
> Cc: u-boot@lists.denx.de; Sun York-R58495; Gupta Ruchika-R66431;
> Kushwaha Prabhakar-B32579
> Subject: Re: [U-Boot, 1/2, v4] powerpc/mpc85xx: SECURE BOOT- NAND
> secure boot target for P3041
> 
> On Tue, 2015-03-10 at 03:50 -0500, Bansal Aneesh-B39320 wrote:
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Thursday, March 05, 2015 10:38 PM
> > > To: Bansal Aneesh-B39320
> > > Cc: u-boot@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-03-05 at 01:26 -0600, Bansal Aneesh-B39320 wrote:
> > > > > -----Original Message-----
> > > > > From: Wood Scott-B07421
> > > > > Sent: Thursday, March 05, 2015 2:41 AM
> > > > > To: Bansal Aneesh-B39320
> > > > > Cc: u-boot@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
> > > > >
> > > > > 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.
> > >
> > > Again, you could relocate the virtual address of L3 much earlier.
> > >
> > > -Scott
> > >
> > Are you suggesting the following:
> > 1.  PBI Commands to configure CPC as SRAM with address 0xBFF0_0000.
> > 2.  Compile U-boot with TEXT BASE as 0xFFF40000.
> > 3.  Copy the U-boot from NAND via PBI commands to CPC (SRAM) on
> > address 0xBFF4_0000 4.  The BootROM will validate the U-boot and transfer
> the control to 0xBFFF_FFFC.
> > 5.  When U-boot is executing, then in the last 4K code, when shifting from
> AS=0 to AS=1,
> >       we change the address of SRAM from 0xBFF0_0000 to 0xFFF0_0000.
> > (Similar to what is done for NOR Boot)
> 
> Something like that, except in step 5 it would only be changing the virtual
> address, not the physical address (unless you can do a similar trick as NOR
> does, to have the L3 cache repeat and cover both addresses at once).
> 
> -Scott
> 
The problems that I see here are:
1.  Can SRAM address be changed without disabling and re-configuring CPC as SRAM ?
2.  Assuming that above is possible,

We are executing from CPC configured as SRAM with address as 0xBFF0_0000. (Because of 3.5 G Limitation)
We create a LAW for 0xFFF0_0000 to map to CPC and 
change the address of SRAM in CPC controller from BFF0_0000 to 0xFFF0_0000. (If it is possible... need to check this)
But now the Code which was executing is executing from 0xBFFx_xxxx. So the CPC controller will reject this since configured address for SRAM is different.
NOR can have two addresses as IFC controller ignores the upper bit but this is not possible with CPC.

To avoid this, even if we create a TLB Entry to map the virtual address 0xBFFx_xxxx to 0xFFFx_xxxx, then we have a race condition. Which step to do first?
Creation of TLB entry or changing the address of SRAM in CPC controller.
Because the current execution PC is 0xBFFx_xxxx (CPC as SRAM)

-Aneesh
Scott Wood March 10, 2015, 5:59 p.m. UTC | #11
On Tue, 2015-03-10 at 12:52 -0500, Bansal Aneesh-B39320 wrote:
> 
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, March 10, 2015 10:34 PM
> > To: Bansal Aneesh-B39320
> > Cc: u-boot@lists.denx.de; Sun York-R58495; Gupta Ruchika-R66431;
> > Kushwaha Prabhakar-B32579
> > Subject: Re: [U-Boot, 1/2, v4] powerpc/mpc85xx: SECURE BOOT- NAND
> > secure boot target for P3041
> > 
> > On Tue, 2015-03-10 at 03:50 -0500, Bansal Aneesh-B39320 wrote:
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Thursday, March 05, 2015 10:38 PM
> > > > To: Bansal Aneesh-B39320
> > > > Cc: u-boot@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-03-05 at 01:26 -0600, Bansal Aneesh-B39320 wrote:
> > > > > > -----Original Message-----
> > > > > > From: Wood Scott-B07421
> > > > > > Sent: Thursday, March 05, 2015 2:41 AM
> > > > > > To: Bansal Aneesh-B39320
> > > > > > Cc: u-boot@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
> > > > > >
> > > > > > 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.
> > > >
> > > > Again, you could relocate the virtual address of L3 much earlier.
> > > >
> > > > -Scott
> > > >
> > > Are you suggesting the following:
> > > 1.  PBI Commands to configure CPC as SRAM with address 0xBFF0_0000.
> > > 2.  Compile U-boot with TEXT BASE as 0xFFF40000.
> > > 3.  Copy the U-boot from NAND via PBI commands to CPC (SRAM) on
> > > address 0xBFF4_0000 4.  The BootROM will validate the U-boot and transfer
> > the control to 0xBFFF_FFFC.
> > > 5.  When U-boot is executing, then in the last 4K code, when shifting from
> > AS=0 to AS=1,
> > >       we change the address of SRAM from 0xBFF0_0000 to 0xFFF0_0000.
> > > (Similar to what is done for NOR Boot)
> > 
> > Something like that, except in step 5 it would only be changing the virtual
> > address, not the physical address (unless you can do a similar trick as NOR
> > does, to have the L3 cache repeat and cover both addresses at once).
> > 
> > -Scott
> > 
> The problems that I see here are:
> 1.  Can SRAM address be changed without disabling and re-configuring CPC as SRAM ?

Again, I'm only talking about changing the virtual address.

> 2.  Assuming that above is possible,
> 
> We are executing from CPC configured as SRAM with address as 0xBFF0_0000. (Because of 3.5 G Limitation)
> We create a LAW for 0xFFF0_0000 to map to CPC and 
> change the address of SRAM in CPC controller from BFF0_0000 to 0xFFF0_0000. (If it is possible... need to check this)
> But now the Code which was executing is executing from 0xBFFx_xxxx. So the CPC controller will reject this since configured address for SRAM is different.
> NOR can have two addresses as IFC controller ignores the upper bit but this is not possible with CPC.

...and this is why.

> To avoid this, even if we create a TLB Entry to map the virtual address 0xBFFx_xxxx to 0xFFFx_xxxx, then we have a race condition. Which step to do first?

Why is there a race condition?  You can have two virtual addresses
pointing at the same physical address.

-Scott
Aneesh Bansal March 10, 2015, 6:27 p.m. UTC | #12
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, March 10, 2015 11:29 PM
> To: Bansal Aneesh-B39320
> Cc: u-boot@lists.denx.de; Sun York-R58495; Gupta Ruchika-R66431;
> Kushwaha Prabhakar-B32579
> Subject: Re: [U-Boot, 1/2, v4] powerpc/mpc85xx: SECURE BOOT- NAND
> secure boot target for P3041
> 
> On Tue, 2015-03-10 at 12:52 -0500, Bansal Aneesh-B39320 wrote:
> >
> >
> > > -----Original Message-----
> > > From: Wood Scott-B07421
> > > Sent: Tuesday, March 10, 2015 10:34 PM
> > > To: Bansal Aneesh-B39320
> > > Cc: u-boot@lists.denx.de; Sun York-R58495; Gupta Ruchika-R66431;
> > > Kushwaha Prabhakar-B32579
> > > Subject: Re: [U-Boot, 1/2, v4] powerpc/mpc85xx: SECURE BOOT- NAND
> > > secure boot target for P3041
> > >
> > > On Tue, 2015-03-10 at 03:50 -0500, Bansal Aneesh-B39320 wrote:
> > > > > -----Original Message-----
> > > > > From: Wood Scott-B07421
> > > > > Sent: Thursday, March 05, 2015 10:38 PM
> > > > > To: Bansal Aneesh-B39320
> > > > > Cc: u-boot@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-03-05 at 01:26 -0600, Bansal Aneesh-B39320 wrote:
> > > > > > > -----Original Message-----
> > > > > > > From: Wood Scott-B07421
> > > > > > > Sent: Thursday, March 05, 2015 2:41 AM
> > > > > > > To: Bansal Aneesh-B39320
> > > > > > > Cc: u-boot@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
> > > > > > >
> > > > > > > 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.
> > > > >
> > > > > Again, you could relocate the virtual address of L3 much earlier.
> > > > >
> > > > > -Scott
> > > > >
> > > > Are you suggesting the following:
> > > > 1.  PBI Commands to configure CPC as SRAM with address 0xBFF0_0000.
> > > > 2.  Compile U-boot with TEXT BASE as 0xFFF40000.
> > > > 3.  Copy the U-boot from NAND via PBI commands to CPC (SRAM) on
> > > > address 0xBFF4_0000 4.  The BootROM will validate the U-boot and
> > > > transfer
> > > the control to 0xBFFF_FFFC.
> > > > 5.  When U-boot is executing, then in the last 4K code, when
> > > > shifting from
> > > AS=0 to AS=1,
> > > >       we change the address of SRAM from 0xBFF0_0000 to 0xFFF0_0000.
> > > > (Similar to what is done for NOR Boot)
> > >
> > > Something like that, except in step 5 it would only be changing the
> > > virtual address, not the physical address (unless you can do a
> > > similar trick as NOR does, to have the L3 cache repeat and cover both
> addresses at once).
> > >
> > > -Scott
> > >
> > The problems that I see here are:
> > 1.  Can SRAM address be changed without disabling and re-configuring CPC
> as SRAM ?
> 
> Again, I'm only talking about changing the virtual address.
> 
If we just change the virtual address to 0xFFF0_0000, how will it work?
The SRAM address configured in CPC controller is 0xBFF0_0000.
So, won't the CPC controller reject fetches with address 0xFFFx_xxxx.

> > 2.  Assuming that above is possible,
> >
> > We are executing from CPC configured as SRAM with address as
> > 0xBFF0_0000. (Because of 3.5 G Limitation) We create a LAW for
> > 0xFFF0_0000 to map to CPC and change the address of SRAM in CPC
> > controller from BFF0_0000 to 0xFFF0_0000. (If it is possible... need to check
> this) But now the Code which was executing is executing from 0xBFFx_xxxx.
> So the CPC controller will reject this since configured address for SRAM is
> different.
> > NOR can have two addresses as IFC controller ignores the upper bit but this
> is not possible with CPC.
> 
> ...and this is why.
> 
> > To avoid this, even if we create a TLB Entry to map the virtual address
> 0xBFFx_xxxx to 0xFFFx_xxxx, then we have a race condition. Which step to
> do first?
> 
> Why is there a race condition?  You can have two virtual addresses pointing at
> the same physical address.
>
We can have two virtual addresses pointing to same physical address but we can't have
two addresses for SRAM in CPC controller. CPC controller will accept fetches with only one address
0xBFFx_xxxx or 0xFFFx_xxxx.
This is not a problem with NOR because IFC controller neglects the upper bits in address and hence
having  two virtual address was not an issue.
 
The code is currently executing from PC 0xBFFx_xxxx and CPC is configured with SRAM address 0xBFFx_xxxx.
If SRAM address in CPC is changed to 0xFFFx_xxxx before creation of TLB entry for virtual address 0xFFFx_xxxx,
Then the CPC controller will reject fetches for address 0xBFFx_xxxx when PC moves to next instruction.

If TLB entry is created to change the virtual address to 0xFFFx_xxxx, then CPC being still configured with address 0xBFFx_xxxx
will reject fetches for address 0xFFFx_xxxx. 
> -Scott
>
Scott Wood March 10, 2015, 6:33 p.m. UTC | #13
On Tue, 2015-03-10 at 13:27 -0500, Bansal Aneesh-B39320 wrote:
> 
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, March 10, 2015 11:29 PM
> > To: Bansal Aneesh-B39320
> > Cc: u-boot@lists.denx.de; Sun York-R58495; Gupta Ruchika-R66431;
> > Kushwaha Prabhakar-B32579
> > Subject: Re: [U-Boot, 1/2, v4] powerpc/mpc85xx: SECURE BOOT- NAND
> > secure boot target for P3041
> > 
> > On Tue, 2015-03-10 at 12:52 -0500, Bansal Aneesh-B39320 wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Tuesday, March 10, 2015 10:34 PM
> > > > To: Bansal Aneesh-B39320
> > > > Cc: u-boot@lists.denx.de; Sun York-R58495; Gupta Ruchika-R66431;
> > > > Kushwaha Prabhakar-B32579
> > > > Subject: Re: [U-Boot, 1/2, v4] powerpc/mpc85xx: SECURE BOOT- NAND
> > > > secure boot target for P3041
> > > >
> > > > On Tue, 2015-03-10 at 03:50 -0500, Bansal Aneesh-B39320 wrote:
> > > > > > -----Original Message-----
> > > > > > From: Wood Scott-B07421
> > > > > > Sent: Thursday, March 05, 2015 10:38 PM
> > > > > > To: Bansal Aneesh-B39320
> > > > > > Cc: u-boot@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-03-05 at 01:26 -0600, Bansal Aneesh-B39320 wrote:
> > > > > > > > -----Original Message-----
> > > > > > > > From: Wood Scott-B07421
> > > > > > > > Sent: Thursday, March 05, 2015 2:41 AM
> > > > > > > > To: Bansal Aneesh-B39320
> > > > > > > > Cc: u-boot@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
> > > > > > > >
> > > > > > > > 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.
> > > > > >
> > > > > > Again, you could relocate the virtual address of L3 much earlier.
> > > > > >
> > > > > > -Scott
> > > > > >
> > > > > Are you suggesting the following:
> > > > > 1.  PBI Commands to configure CPC as SRAM with address 0xBFF0_0000.
> > > > > 2.  Compile U-boot with TEXT BASE as 0xFFF40000.
> > > > > 3.  Copy the U-boot from NAND via PBI commands to CPC (SRAM) on
> > > > > address 0xBFF4_0000 4.  The BootROM will validate the U-boot and
> > > > > transfer
> > > > the control to 0xBFFF_FFFC.
> > > > > 5.  When U-boot is executing, then in the last 4K code, when
> > > > > shifting from
> > > > AS=0 to AS=1,
> > > > >       we change the address of SRAM from 0xBFF0_0000 to 0xFFF0_0000.
> > > > > (Similar to what is done for NOR Boot)
> > > >
> > > > Something like that, except in step 5 it would only be changing the
> > > > virtual address, not the physical address (unless you can do a
> > > > similar trick as NOR does, to have the L3 cache repeat and cover both
> > addresses at once).
> > > >
> > > > -Scott
> > > >
> > > The problems that I see here are:
> > > 1.  Can SRAM address be changed without disabling and re-configuring CPC
> > as SRAM ?
> > 
> > Again, I'm only talking about changing the virtual address.
> > 
> If we just change the virtual address to 0xFFF0_0000, how will it work?
> The SRAM address configured in CPC controller is 0xBFF0_0000.
> So, won't the CPC controller reject fetches with address 0xFFFx_xxxx.

It's a virtual address.  The CPC will never see it.

> > Why is there a race condition?  You can have two virtual addresses pointing at
> > the same physical address.
> >
> We can have two virtual addresses pointing to same physical address but we can't have
> two addresses for SRAM in CPC controller.

And you don't need to.

-Scott
diff mbox

Patch

diff --git a/Makefile b/Makefile
index bd4abab..acfaa23 100644
--- a/Makefile
+++ b/Makefile
@@ -719,8 +719,12 @@  ALL-$(CONFIG_ONENAND_U_BOOT) += u-boot-onenand.bin
 ifeq ($(CONFIG_SPL_FSL_PBL),y)
 ALL-$(CONFIG_RAMBOOT_PBL) += u-boot-with-spl-pbl.bin
 else
+ifneq ($(CONFIG_SECURE_BOOT), y)
+# For Secure Boot The Image needs to be signed and Header must also
+# be included. So The image has to be built explicitly
 ALL-$(CONFIG_RAMBOOT_PBL) += u-boot.pbl
 endif
+endif
 ALL-$(CONFIG_SPL) += spl/u-boot-spl.bin
 ALL-$(CONFIG_SPL_FRAMEWORK) += u-boot.img
 ALL-$(CONFIG_TPL) += tpl/u-boot-tpl.bin
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
+
 #ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC13
 	{
 		if (SVR_MAJ(svr) < 3) {
diff --git a/board/freescale/common/p_corenet/tlb.c b/board/freescale/common/p_corenet/tlb.c
index 8148e46..1b60cfb 100644
--- a/board/freescale/common/p_corenet/tlb.c
+++ b/board/freescale/common/p_corenet/tlb.c
@@ -42,7 +42,9 @@  struct fsl_e_tlb_entry tlb_table[] = {
 
 	/* TLB 1 */
 	/* *I*** - Covers boot page */
-#if defined(CONFIG_SYS_RAMBOOT) && defined(CONFIG_SYS_INIT_L3_ADDR)
+	/* In Case of Secure RAM Boot L3 address is defined at 0xbff00000 */
+#if defined(CONFIG_SYS_RAMBOOT) && defined(CONFIG_SYS_INIT_L3_ADDR) && \
+	!defined(CONFIG_SECURE_BOOT)
 	/*
 	 * *I*G - L3SRAM. When L3 is used as 1M SRAM, the address of the
 	 * SRAM is at 0xfff00000, it covered the 0xfffff000.
@@ -76,11 +78,25 @@  struct fsl_e_tlb_entry tlb_table[] = {
 		      MAS3_SX|MAS3_SR, MAS2_W|MAS2_G,
 		      0, 2, BOOKE_PAGESZ_256M, 1),
 
+#if defined(CONFIG_SYS_RAMBOOT) && defined(CONFIG_SYS_INIT_L3_ADDR) && \
+	defined(CONFIG_SECURE_BOOT)
+	/* In case of Secure Boot, L3 is used as 1M SRAM
+	 * and the address of the SRAM is at 0xbff00000.
+	 * The PCIE TLB entry conflicts with the above entry.
+	 * So, the entry for PCIE is not created at this point of time.
+	 * It will be created later on in cpu_init_r()
+	 * when U-Boot has relocated to DDR
+	 */
+	SET_TLB_ENTRY(1, CONFIG_SYS_INIT_L3_ADDR, CONFIG_SYS_INIT_L3_ADDR,
+		      MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
+		      0, 3, BOOKE_PAGESZ_1M, 1),
+#else
 	/* *I*G* - PCI */
 	SET_TLB_ENTRY(1, CONFIG_SYS_PCIE1_MEM_VIRT, CONFIG_SYS_PCIE1_MEM_PHYS,
 		      MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,
 		      0, 3, BOOKE_PAGESZ_1G, 1),
 
+#endif
 	/* *I*G* - PCI */
 	SET_TLB_ENTRY(1, CONFIG_SYS_PCIE1_MEM_VIRT + 0x40000000,
 		      CONFIG_SYS_PCIE1_MEM_PHYS + 0x40000000,
diff --git a/board/freescale/corenet_ds/MAINTAINERS b/board/freescale/corenet_ds/MAINTAINERS
index 745847c..6855446 100644
--- a/board/freescale/corenet_ds/MAINTAINERS
+++ b/board/freescale/corenet_ds/MAINTAINERS
@@ -28,3 +28,8 @@  F:	configs/P5040DS_NAND_defconfig
 F:	configs/P5040DS_SDCARD_defconfig
 F:	configs/P5040DS_SPIFLASH_defconfig
 F:	configs/P5040DS_SECURE_BOOT_defconfig
+
+CORENET_DS_SECURE_BOOT BOARD
+M:	Aneesh Bansal <aneesh.bansal@freescale.com>
+S:	Maintained
+F:	configs/P3041DS_NAND_SECURE_BOOT_defconfig
diff --git a/configs/P3041DS_NAND_SECURE_BOOT_defconfig b/configs/P3041DS_NAND_SECURE_BOOT_defconfig
new file mode 100644
index 0000000..e810b1c
--- /dev/null
+++ b/configs/P3041DS_NAND_SECURE_BOOT_defconfig
@@ -0,0 +1,4 @@ 
+CONFIG_SYS_EXTRA_OPTIONS="RAMBOOT_PBL,NAND,SECURE_BOOT,SYS_TEXT_BASE=0xBFF40000"
+CONFIG_PPC=y
+CONFIG_MPC85xx=y
+CONFIG_TARGET_P3041DS=y
diff --git a/include/configs/corenet_ds.h b/include/configs/corenet_ds.h
index 225ffdd..64c6890 100644
--- a/include/configs/corenet_ds.h
+++ b/include/configs/corenet_ds.h
@@ -16,6 +16,14 @@ 
 #include "../board/freescale/common/ics307_clk.h"
 
 #ifdef CONFIG_RAMBOOT_PBL
+#ifdef CONFIG_SECURE_BOOT
+#define CONFIG_RAMBOOT_TEXT_BASE	CONFIG_SYS_TEXT_BASE
+#define CONFIG_RESET_VECTOR_ADDRESS	0xbffffffc
+#define CONFIG_BPTR_VIRT_ADDR		0xbffff000
+#ifdef CONFIG_NAND
+#define CONFIG_RAMBOOT_NAND
+#endif
+#else
 #define CONFIG_RAMBOOT_TEXT_BASE	CONFIG_SYS_TEXT_BASE
 #define CONFIG_RESET_VECTOR_ADDRESS	0xfffffffc
 #define CONFIG_SYS_FSL_PBL_PBI board/freescale/corenet_ds/pbi.cfg
@@ -29,6 +37,7 @@ 
 #define CONFIG_SYS_FSL_PBL_RCW board/freescale/corenet_ds/rcw_p5040ds.cfg
 #endif
 #endif
+#endif
 
 #ifdef CONFIG_SRIO_PCIE_BOOT_SLAVE
 /* Set 1M boot space */