[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[Aneesh Bansal]

Changes:
1. L2 cache is being invalidated by Boot ROM code for e6500 core.
   So removing the invalidation from start.S
2. Clear the LAW and corresponding configuration for CPC. Boot ROM
   code uses it as hosekeeping area.
3. For Secure boot, CPC is configured as SRAM and used as house
   keeping area. This configuration is to be disabled once in uboot.
   Earlier this disabling of CPC as SRAM was happening in cpu_init_r.
   As a result cache invalidation function was getting skipped in
   case CPC is configured as SRAM.This was causing random crashes.

Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
---
 README                                     |  4 ++++
 arch/powerpc/cpu/mpc85xx/cpu_init.c        | 27 ++++++++++++++++++++++-----
 arch/powerpc/cpu/mpc85xx/start.S           |  3 ++-
 arch/powerpc/include/asm/fsl_secure_boot.h |  6 ++++++
 boards.cfg                                 |  1 +
 5 files changed, 35 insertions(+), 6 deletions(-)

Changes from v3:
Renamed the macro to indicate CPC configured as SRAM at U-boot entry to
CONFIG_SYS_CPC_SRAM 

diff --git a/README b/README
index 216f0c7..e26833d 100644
--- a/README
+++ b/README
@@ -427,6 +427,10 @@ The following options need to be configured:
 		In this mode, a single differential clock is used to supply
 		clocks to the sysclock, ddrclock and usbclock.
 
+		CONFIG_SYS_CPC_SRAM
+		This CONFIG is defined when the SPC is configured as SRAM at the
+		time of U-boot entry.
+
 - Generic CPU options:
 		CONFIG_SYS_BIG_ENDIAN, CONFIG_SYS_LITTLE_ENDIAN
 
diff --git a/arch/powerpc/cpu/mpc85xx/cpu_init.c b/arch/powerpc/cpu/mpc85xx/cpu_init.c
index b31efb7..5cd7cb1 100644
--- a/arch/powerpc/cpu/mpc85xx/cpu_init.c
+++ b/arch/powerpc/cpu/mpc85xx/cpu_init.c
@@ -125,17 +125,14 @@ void config_8560_ioports (volatile ccsr_cpm_t * cpm)
 #endif
 
 #ifdef CONFIG_SYS_FSL_CPC
-static void enable_cpc(void)
+#if defined(CONFIG_RAMBOOT_PBL) || defined(CONFIG_SYS_CPC_SRAM)
+static void disable_cpc_sram(void)
 {
 	int i;
-	u32 size = 0;
 
 	cpc_corenet_t *cpc = (cpc_corenet_t *)CONFIG_SYS_FSL_CPC_ADDR;
 
 	for (i = 0; i < CONFIG_SYS_NUM_CPC; i++, cpc++) {
-		u32 cpccfg0 = in_be32(&cpc->cpccfg0);
-		size += CPC_CFG0_SZ_K(cpccfg0);
-#ifdef CONFIG_RAMBOOT_PBL
 		if (in_be32(&cpc->cpcsrcr0) & CPC_SRCR0_SRAMEN) {
 			/* find and disable LAW of SRAM */
 			struct law_entry law = find_law(CONFIG_SYS_INIT_L3_ADDR);
@@ -150,8 +147,21 @@ static void enable_cpc(void)
 			out_be32(&cpc->cpccsr0, 0);
 			out_be32(&cpc->cpcsrcr0, 0);
 		}
+	}
+}
 #endif
 
+static void enable_cpc(void)
+{
+	int i;
+	u32 size = 0;
+
+	cpc_corenet_t *cpc = (cpc_corenet_t *)CONFIG_SYS_FSL_CPC_ADDR;
+
+	for (i = 0; i < CONFIG_SYS_NUM_CPC; i++, cpc++) {
+		u32 cpccfg0 = in_be32(&cpc->cpccfg0);
+		size += CPC_CFG0_SZ_K(cpccfg0);
+
 #ifdef CONFIG_SYS_FSL_ERRATUM_CPC_A002
 		setbits_be32(&cpc->cpchdbcr0, CPC_HDBCR0_TAG_ECC_SCRUB_DIS);
 #endif
@@ -250,6 +260,10 @@ void cpu_init_f (void)
 	law = find_law(CONFIG_SYS_PBI_FLASH_BASE);
 	if (law.index != -1)
 		disable_law(law.index);
+
+#if defined(CONFIG_SYS_CPC_SRAM)
+	disable_cpc_sram();
+#endif
 #endif
 
 #ifdef CONFIG_CPM2
@@ -550,6 +564,9 @@ skip_l2:
 	puts("disabled\n");
 #endif
 
+#if defined(CONFIG_RAMBOOT_PBL)
+	disable_cpc_sram();
+#endif
 	enable_cpc();
 
 #ifndef CONFIG_SYS_FSL_NO_SERDES
diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
index dbbd8e5..4ef0985 100644
--- a/arch/powerpc/cpu/mpc85xx/start.S
+++ b/arch/powerpc/cpu/mpc85xx/start.S
@@ -115,7 +115,8 @@ _start_e500:
 #endif
 
 
-#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_E500MC)
+#if defined(CONFIG_SECURE_BOOT) && defined(CONFIG_E500MC) && \
+	!defined(CONFIG_E6500)
 	/* ISBC uses L2 as stack.
 	 * Disable L2 cache here so that u-boot can enable it later
 	 * as part of it's normal flow
diff --git a/arch/powerpc/include/asm/fsl_secure_boot.h b/arch/powerpc/include/asm/fsl_secure_boot.h
index 4c7f0b1..68498df 100644
--- a/arch/powerpc/include/asm/fsl_secure_boot.h
+++ b/arch/powerpc/include/asm/fsl_secure_boot.h
@@ -15,5 +15,11 @@
 #endif
 #define CONFIG_SYS_PBI_FLASH_WINDOW		0xcff80000
 
+#if defined(CONFIG_B4860QDS)
+#define CONFIG_SYS_CPC_SRAM
+#undef CONFIG_SYS_INIT_L3_ADDR
+#define CONFIG_SYS_INIT_L3_ADDR			0xbff00000
+#endif
+
 #endif
 #endif
diff --git a/boards.cfg b/boards.cfg
index 14cd151..65678ca 100644
--- a/boards.cfg
+++ b/boards.cfg
@@ -782,6 +782,7 @@ Active  powerpc     mpc85xx        -           freescale       b4860qds
 Active  powerpc     mpc85xx        -           freescale       b4860qds            B4420QDS_NAND                        B4860QDS:PPC_B4420,RAMBOOT_PBL,NAND,SYS_TEXT_BASE=0xFFF40000                                                                      -
 Active  powerpc     mpc85xx        -           freescale       b4860qds            B4420QDS_SPIFLASH                    B4860QDS:PPC_B4420,RAMBOOT_PBL,SPIFLASH,SYS_TEXT_BASE=0xFFF40000                                                                  -
 Active  powerpc     mpc85xx        -           freescale       b4860qds            B4860QDS                             B4860QDS:PPC_B4860                                                                                                                -
+Active  powerpc     mpc85xx        -           freescale       b4860qds            B4860QDS_SECURE_BOOT                 B4860QDS:PPC_B4860,SECURE_BOOT                                                                                                    Aneesh Bansal <aneesh.bansal@freescale.com>
 Active  powerpc     mpc85xx        -           freescale       b4860qds            B4860QDS_NAND                        B4860QDS:PPC_B4860,RAMBOOT_PBL,NAND,SYS_TEXT_BASE=0xFFF40000                                                                      -
 Active  powerpc     mpc85xx        -           freescale       b4860qds            B4860QDS_SPIFLASH                    B4860QDS:PPC_B4860,RAMBOOT_PBL,SPIFLASH,SYS_TEXT_BASE=0xFFF40000                                                                  -
 Active  powerpc     mpc85xx        -           freescale       b4860qds            B4860QDS_SRIO_PCIE_BOOT              B4860QDS:PPC_B4860,SRIO_PCIE_BOOT_SLAVE,SYS_TEXT_BASE=0xFFF40000                                                                  -
-- 
1.8.1.4

[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[York Sun]

On 03/07/2014 05:23 AM, Aneesh Bansal wrote:
> Changes:
> 1. L2 cache is being invalidated by Boot ROM code for e6500 core.
>    So removing the invalidation from start.S
> 2. Clear the LAW and corresponding configuration for CPC. Boot ROM
>    code uses it as hosekeeping area.
> 3. For Secure boot, CPC is configured as SRAM and used as house
>    keeping area. This configuration is to be disabled once in uboot.
>    Earlier this disabling of CPC as SRAM was happening in cpu_init_r.
>    As a result cache invalidation function was getting skipped in
>    case CPC is configured as SRAM.This was causing random crashes.
> 
> Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
> ---
>  README                                     |  4 ++++
>  arch/powerpc/cpu/mpc85xx/cpu_init.c        | 27 ++++++++++++++++++++++-----
>  arch/powerpc/cpu/mpc85xx/start.S           |  3 ++-
>  arch/powerpc/include/asm/fsl_secure_boot.h |  6 ++++++
>  boards.cfg                                 |  1 +
>  5 files changed, 35 insertions(+), 6 deletions(-)
> 
> Changes from v3:
> Renamed the macro to indicate CPC configured as SRAM at U-boot entry to
> CONFIG_SYS_CPC_SRAM 

Aneesh,

I understand you are trying to address the comment about when CPC needs to be
disabled before initializing as normal CPC. But introducing CONFIG_SYS_CPC_SRAM
seems even more confusing. Let's take one step back.

CPC as SRAM can be used for many reasons. There is only one reason we should
keep it as SRAM, i.e. u-boot is indeed using SRAM as its final destination. For
all other usages, we can safely reconfigure it as normal CPC after u-boot
relocates itself into DDR. If u-boot's final destination is SRAM, it is a
RAMBOOT. Can we use this condition to deal with CPC?

Do you see the need to disable CPC before relocation? You are doing so in this
patch. Does the temporary LAW or address conflict with u-boot?

York

[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[Scott Wood]

On Fri, 2014-03-07 at 18:53 +0530, Aneesh Bansal wrote:
> Changes:
> 1. L2 cache is being invalidated by Boot ROM code for e6500 core.
>    So removing the invalidation from start.S
> 2. Clear the LAW and corresponding configuration for CPC. Boot ROM
>    code uses it as hosekeeping area.
> 3. For Secure boot, CPC is configured as SRAM and used as house
>    keeping area. This configuration is to be disabled once in uboot.
>    Earlier this disabling of CPC as SRAM was happening in cpu_init_r.
>    As a result cache invalidation function was getting skipped in
>    case CPC is configured as SRAM.This was causing random crashes.
> 
> Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
> ---
>  README                                     |  4 ++++
>  arch/powerpc/cpu/mpc85xx/cpu_init.c        | 27 ++++++++++++++++++++++-----
>  arch/powerpc/cpu/mpc85xx/start.S           |  3 ++-
>  arch/powerpc/include/asm/fsl_secure_boot.h |  6 ++++++
>  boards.cfg                                 |  1 +
>  5 files changed, 35 insertions(+), 6 deletions(-)
> 
> Changes from v3:
> Renamed the macro to indicate CPC configured as SRAM at U-boot entry to
> CONFIG_SYS_CPC_SRAM 
> 
> diff --git a/README b/README
> index 216f0c7..e26833d 100644
> --- a/README
> +++ b/README
> @@ -427,6 +427,10 @@ The following options need to be configured:
>  		In this mode, a single differential clock is used to supply
>  		clocks to the sysclock, ddrclock and usbclock.
>  
> +		CONFIG_SYS_CPC_SRAM
> +		This CONFIG is defined when the SPC is configured as SRAM at the
> +		time of U-boot entry.

SPC?

-Scott

[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[Scott Wood]

On Fri, 2014-03-07 at 10:30 -0800, York Sun wrote:
> On 03/07/2014 05:23 AM, Aneesh Bansal wrote:
> > Changes:
> > 1. L2 cache is being invalidated by Boot ROM code for e6500 core.
> >    So removing the invalidation from start.S
> > 2. Clear the LAW and corresponding configuration for CPC. Boot ROM
> >    code uses it as hosekeeping area.
> > 3. For Secure boot, CPC is configured as SRAM and used as house
> >    keeping area. This configuration is to be disabled once in uboot.
> >    Earlier this disabling of CPC as SRAM was happening in cpu_init_r.
> >    As a result cache invalidation function was getting skipped in
> >    case CPC is configured as SRAM.This was causing random crashes.
> > 
> > Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
> > ---
> >  README                                     |  4 ++++
> >  arch/powerpc/cpu/mpc85xx/cpu_init.c        | 27 ++++++++++++++++++++++-----
> >  arch/powerpc/cpu/mpc85xx/start.S           |  3 ++-
> >  arch/powerpc/include/asm/fsl_secure_boot.h |  6 ++++++
> >  boards.cfg                                 |  1 +
> >  5 files changed, 35 insertions(+), 6 deletions(-)
> > 
> > Changes from v3:
> > Renamed the macro to indicate CPC configured as SRAM at U-boot entry to
> > CONFIG_SYS_CPC_SRAM 
> 
> Aneesh,
> 
> I understand you are trying to address the comment about when CPC needs to be
> disabled before initializing as normal CPC. But introducing CONFIG_SYS_CPC_SRAM
> seems even more confusing. Let's take one step back.
> 
> CPC as SRAM can be used for many reasons. There is only one reason we should
> keep it as SRAM, i.e. u-boot is indeed using SRAM as its final destination. For
> all other usages, we can safely reconfigure it as normal CPC after u-boot
> relocates itself into DDR. If u-boot's final destination is SRAM, it is a
> RAMBOOT. Can we use this condition to deal with CPC?

Even if CPC is the final destination, shouldn't RAMBOOT imply that DDR
has been initialized?

-Scott

[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[York Sun]

On 03/07/2014 10:54 AM, Scott Wood wrote:
> On Fri, 2014-03-07 at 10:30 -0800, York Sun wrote:
>> On 03/07/2014 05:23 AM, Aneesh Bansal wrote:
>>> Changes:
>>> 1. L2 cache is being invalidated by Boot ROM code for e6500 core.
>>>    So removing the invalidation from start.S
>>> 2. Clear the LAW and corresponding configuration for CPC. Boot ROM
>>>    code uses it as hosekeeping area.
>>> 3. For Secure boot, CPC is configured as SRAM and used as house
>>>    keeping area. This configuration is to be disabled once in uboot.
>>>    Earlier this disabling of CPC as SRAM was happening in cpu_init_r.
>>>    As a result cache invalidation function was getting skipped in
>>>    case CPC is configured as SRAM.This was causing random crashes.
>>>
>>> Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
>>> ---
>>>  README                                     |  4 ++++
>>>  arch/powerpc/cpu/mpc85xx/cpu_init.c        | 27 ++++++++++++++++++++++-----
>>>  arch/powerpc/cpu/mpc85xx/start.S           |  3 ++-
>>>  arch/powerpc/include/asm/fsl_secure_boot.h |  6 ++++++
>>>  boards.cfg                                 |  1 +
>>>  5 files changed, 35 insertions(+), 6 deletions(-)
>>>
>>> Changes from v3:
>>> Renamed the macro to indicate CPC configured as SRAM at U-boot entry to
>>> CONFIG_SYS_CPC_SRAM 
>>
>> Aneesh,
>>
>> I understand you are trying to address the comment about when CPC needs to be
>> disabled before initializing as normal CPC. But introducing CONFIG_SYS_CPC_SRAM
>> seems even more confusing. Let's take one step back.
>>
>> CPC as SRAM can be used for many reasons. There is only one reason we should
>> keep it as SRAM, i.e. u-boot is indeed using SRAM as its final destination. For
>> all other usages, we can safely reconfigure it as normal CPC after u-boot
>> relocates itself into DDR. If u-boot's final destination is SRAM, it is a
>> RAMBOOT. Can we use this condition to deal with CPC?
> 
> Even if CPC is the final destination, shouldn't RAMBOOT imply that DDR
> has been initialized?

No. RAMBOOT implies u-boot shouldn't initialize RAM. There may be cases where
u-boot needs to run in SRAM without DDR. It doesn't matter if the RAM is SRAM or
DRAM.

York

[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[Scott Wood]

On Fri, 2014-03-07 at 10:58 -0800, York Sun wrote:
> On 03/07/2014 10:54 AM, Scott Wood wrote:
> > On Fri, 2014-03-07 at 10:30 -0800, York Sun wrote:
> >> On 03/07/2014 05:23 AM, Aneesh Bansal wrote:
> >>> Changes:
> >>> 1. L2 cache is being invalidated by Boot ROM code for e6500 core.
> >>>    So removing the invalidation from start.S
> >>> 2. Clear the LAW and corresponding configuration for CPC. Boot ROM
> >>>    code uses it as hosekeeping area.
> >>> 3. For Secure boot, CPC is configured as SRAM and used as house
> >>>    keeping area. This configuration is to be disabled once in uboot.
> >>>    Earlier this disabling of CPC as SRAM was happening in cpu_init_r.
> >>>    As a result cache invalidation function was getting skipped in
> >>>    case CPC is configured as SRAM.This was causing random crashes.
> >>>
> >>> Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
> >>> ---
> >>>  README                                     |  4 ++++
> >>>  arch/powerpc/cpu/mpc85xx/cpu_init.c        | 27 ++++++++++++++++++++++-----
> >>>  arch/powerpc/cpu/mpc85xx/start.S           |  3 ++-
> >>>  arch/powerpc/include/asm/fsl_secure_boot.h |  6 ++++++
> >>>  boards.cfg                                 |  1 +
> >>>  5 files changed, 35 insertions(+), 6 deletions(-)
> >>>
> >>> Changes from v3:
> >>> Renamed the macro to indicate CPC configured as SRAM at U-boot entry to
> >>> CONFIG_SYS_CPC_SRAM 
> >>
> >> Aneesh,
> >>
> >> I understand you are trying to address the comment about when CPC needs to be
> >> disabled before initializing as normal CPC. But introducing CONFIG_SYS_CPC_SRAM
> >> seems even more confusing. Let's take one step back.
> >>
> >> CPC as SRAM can be used for many reasons. There is only one reason we should
> >> keep it as SRAM, i.e. u-boot is indeed using SRAM as its final destination. For
> >> all other usages, we can safely reconfigure it as normal CPC after u-boot
> >> relocates itself into DDR. If u-boot's final destination is SRAM, it is a
> >> RAMBOOT. Can we use this condition to deal with CPC?
> > 
> > Even if CPC is the final destination, shouldn't RAMBOOT imply that DDR
> > has been initialized?
> 
> No. RAMBOOT implies u-boot shouldn't initialize RAM. There may be cases where
> u-boot needs to run in SRAM without DDR. It doesn't matter if the RAM is SRAM or
> DRAM.

There are some situations where we run U-Boot out of SRAM without
relocating to DDR, but I can't think of a situation where we wouldn't
want to use DDR at all (e.g. for loading an OS to).

-Scott

[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[York Sun]

On 03/07/2014 11:00 AM, Scott Wood wrote:
> On Fri, 2014-03-07 at 10:58 -0800, York Sun wrote:
>> On 03/07/2014 10:54 AM, Scott Wood wrote:
>>> On Fri, 2014-03-07 at 10:30 -0800, York Sun wrote:
>>>> On 03/07/2014 05:23 AM, Aneesh Bansal wrote:
>>>>> Changes:
>>>>> 1. L2 cache is being invalidated by Boot ROM code for e6500 core.
>>>>>    So removing the invalidation from start.S
>>>>> 2. Clear the LAW and corresponding configuration for CPC. Boot ROM
>>>>>    code uses it as hosekeeping area.
>>>>> 3. For Secure boot, CPC is configured as SRAM and used as house
>>>>>    keeping area. This configuration is to be disabled once in uboot.
>>>>>    Earlier this disabling of CPC as SRAM was happening in cpu_init_r.
>>>>>    As a result cache invalidation function was getting skipped in
>>>>>    case CPC is configured as SRAM.This was causing random crashes.
>>>>>
>>>>> Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
>>>>> ---
>>>>>  README                                     |  4 ++++
>>>>>  arch/powerpc/cpu/mpc85xx/cpu_init.c        | 27 ++++++++++++++++++++++-----
>>>>>  arch/powerpc/cpu/mpc85xx/start.S           |  3 ++-
>>>>>  arch/powerpc/include/asm/fsl_secure_boot.h |  6 ++++++
>>>>>  boards.cfg                                 |  1 +
>>>>>  5 files changed, 35 insertions(+), 6 deletions(-)
>>>>>
>>>>> Changes from v3:
>>>>> Renamed the macro to indicate CPC configured as SRAM at U-boot entry to
>>>>> CONFIG_SYS_CPC_SRAM 
>>>>
>>>> Aneesh,
>>>>
>>>> I understand you are trying to address the comment about when CPC needs to be
>>>> disabled before initializing as normal CPC. But introducing CONFIG_SYS_CPC_SRAM
>>>> seems even more confusing. Let's take one step back.
>>>>
>>>> CPC as SRAM can be used for many reasons. There is only one reason we should
>>>> keep it as SRAM, i.e. u-boot is indeed using SRAM as its final destination. For
>>>> all other usages, we can safely reconfigure it as normal CPC after u-boot
>>>> relocates itself into DDR. If u-boot's final destination is SRAM, it is a
>>>> RAMBOOT. Can we use this condition to deal with CPC?
>>>
>>> Even if CPC is the final destination, shouldn't RAMBOOT imply that DDR
>>> has been initialized?
>>
>> No. RAMBOOT implies u-boot shouldn't initialize RAM. There may be cases where
>> u-boot needs to run in SRAM without DDR. It doesn't matter if the RAM is SRAM or
>> DRAM.
> 
> There are some situations where we run U-Boot out of SRAM without
> relocating to DDR, but I can't think of a situation where we wouldn't
> want to use DDR at all (e.g. for loading an OS to).
> 

I generally agree with you. Loading OS needs more RAM.

I am trying to simplify the condition to reinitialize CPC. My point is only one
situation requires the CPC to act as SRAM. We can brainstorm how useful it is.
For example, as a recovery method if using JTAG tool to enable CPC as SRAM and
to load u-boot into it, one can skip the steps to initialize DDR to run an
full-feature u-boot and reflash the board.

York

[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[aneesh.bansal at freescale.com]

> -----Original Message-----
> From: Sun York-R58495
> Sent: Saturday, March 08, 2014 12:01 AM
> To: Bansal Aneesh-B39320; u-boot at lists.denx.de
> Cc: Wood Scott-B07421
> Subject: Re: [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot
> target for B4860QDS
> 
> On 03/07/2014 05:23 AM, Aneesh Bansal wrote:
> > Changes:
> > 1. L2 cache is being invalidated by Boot ROM code for e6500 core.
> >    So removing the invalidation from start.S 2. Clear the LAW and
> > corresponding configuration for CPC. Boot ROM
> >    code uses it as hosekeeping area.
> > 3. For Secure boot, CPC is configured as SRAM and used as house
> >    keeping area. This configuration is to be disabled once in uboot.
> >    Earlier this disabling of CPC as SRAM was happening in
> cpu_init_r.
> >    As a result cache invalidation function was getting skipped in
> >    case CPC is configured as SRAM.This was causing random crashes.
> >
> > Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
> > ---
> >  README                                     |  4 ++++
> >  arch/powerpc/cpu/mpc85xx/cpu_init.c        | 27
> ++++++++++++++++++++++-----
> >  arch/powerpc/cpu/mpc85xx/start.S           |  3 ++-
> >  arch/powerpc/include/asm/fsl_secure_boot.h |  6 ++++++
> >  boards.cfg                                 |  1 +
> >  5 files changed, 35 insertions(+), 6 deletions(-)
> >
> > Changes from v3:
> > Renamed the macro to indicate CPC configured as SRAM at U-boot entry
> > to CONFIG_SYS_CPC_SRAM
> 
> Aneesh,
> 
> I understand you are trying to address the comment about when CPC
> needs to be disabled before initializing as normal CPC. But
> introducing CONFIG_SYS_CPC_SRAM seems even more confusing. Let's take
> one step back.
In the previous patch we had defined it as CONFIG_SYS_SECURE_HKAREA_CPC. But as per the discussion on previous patch, we were advised to make it generic to have a macro stating that U-Boot is entered with the CPC configured as SRAM.
> 
> CPC as SRAM can be used for many reasons. There is only one reason we
> should keep it as SRAM, i.e. u-boot is indeed using SRAM as its final
> destination. For all other usages, we can safely reconfigure it as
> normal CPC after u-boot relocates itself into DDR. If u-boot's final
> destination is SRAM, it is a RAMBOOT. Can we use this condition to
> deal with CPC?
There is a separate condition for secure boot in which CPC is required as SRAM. For NOR secure boot, user needs to initialize a housekeeping area using PBI commands and provide pointer to this housekeeping area to boot ROM.  Currently we are using CPC configured as SRAM for this area. Since, this area was initialized as SRAM by PBI commands, there is no cleanup of this area by Boot ROM. Since out of ROM, this are is no longer required, in case of NOR secure boot we need to disable this setting. NOR secure boot is not a RAMBOOT.
 
> 
> Do you see the need to disable CPC before relocation? You are doing so
> in this patch. Does the temporary LAW or address conflict with u-boot?
Earlier we were disabling cpc in cpu_init_r. However since cache invalidation function was getting skipped in the process, it was causing random crashes. Skipping invalidation works in RAMBOOT scenario, however since we don?t have valid data when CPC is configured as cache for the secure boot scenario which is not RAMBOOT, these crashes were occurring in few platforms. As a result we had to move this disable code early in the sequence, so that invalidation of cache doesn?t get skipped.
> 
> York
> 

[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[York Sun]

On 03/10/2014 02:14 AM, Bansal Aneesh-B39320 wrote:
>> -----Original Message-----
>> From: Sun York-R58495
>> Sent: Saturday, March 08, 2014 12:01 AM
>> To: Bansal Aneesh-B39320; u-boot at lists.denx.de
>> Cc: Wood Scott-B07421
>> Subject: Re: [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot
>> target for B4860QDS
>>
>> On 03/07/2014 05:23 AM, Aneesh Bansal wrote:
>>> Changes:
>>> 1. L2 cache is being invalidated by Boot ROM code for e6500 core.
>>>    So removing the invalidation from start.S 2. Clear the LAW and
>>> corresponding configuration for CPC. Boot ROM
>>>    code uses it as hosekeeping area.
>>> 3. For Secure boot, CPC is configured as SRAM and used as house
>>>    keeping area. This configuration is to be disabled once in uboot.
>>>    Earlier this disabling of CPC as SRAM was happening in
>> cpu_init_r.
>>>    As a result cache invalidation function was getting skipped in
>>>    case CPC is configured as SRAM.This was causing random crashes.
>>>
>>> Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
>>> ---
>>>  README                                     |  4 ++++
>>>  arch/powerpc/cpu/mpc85xx/cpu_init.c        | 27
>> ++++++++++++++++++++++-----
>>>  arch/powerpc/cpu/mpc85xx/start.S           |  3 ++-
>>>  arch/powerpc/include/asm/fsl_secure_boot.h |  6 ++++++
>>>  boards.cfg                                 |  1 +
>>>  5 files changed, 35 insertions(+), 6 deletions(-)
>>>
>>> Changes from v3:
>>> Renamed the macro to indicate CPC configured as SRAM at U-boot entry
>>> to CONFIG_SYS_CPC_SRAM
>>
>> Aneesh,
>>
>> I understand you are trying to address the comment about when CPC
>> needs to be disabled before initializing as normal CPC. But
>> introducing CONFIG_SYS_CPC_SRAM seems even more confusing. Let's take
>> one step back.
> In the previous patch we had defined it as CONFIG_SYS_SECURE_HKAREA_CPC. But as per the discussion on previous patch, we were advised to make it generic to have a macro stating that U-Boot is entered with the CPC configured as SRAM.

In your patch, you are using this new macro to disable SRAM. It would be more
clear if the macro is named CONFIG_SYS_CPC_REINIT_F.


>>
>> CPC as SRAM can be used for many reasons. There is only one reason we
>> should keep it as SRAM, i.e. u-boot is indeed using SRAM as its final
>> destination. For all other usages, we can safely reconfigure it as
>> normal CPC after u-boot relocates itself into DDR. If u-boot's final
>> destination is SRAM, it is a RAMBOOT. Can we use this condition to
>> deal with CPC?
> There is a separate condition for secure boot in which CPC is required as SRAM. For NOR secure boot, user needs to initialize a housekeeping area using PBI commands and provide pointer to this housekeeping area to boot ROM.  Currently we are using CPC configured as SRAM for this area. Since, this area was initialized as SRAM by PBI commands, there is no cleanup of this area by Boot ROM. Since out of ROM, this are is no longer required, in case of NOR secure boot we need to disable this setting. NOR secure boot is not a RAMBOOT.

That's not conflict with my suggestion. This is NOT a RAMBOOT, so CPC should be
reinitialized.

>  
>>
>> Do you see the need to disable CPC before relocation? You are doing so
>> in this patch. Does the temporary LAW or address conflict with u-boot?
> Earlier we were disabling cpc in cpu_init_r. However since cache invalidation function was getting skipped in the process, it was causing random crashes. Skipping invalidation works in RAMBOOT scenario, however since we don?t have valid data when CPC is configured as cache for the secure boot scenario which is not RAMBOOT, these crashes were occurring in few platforms. As a result we had to move this disable code early in the sequence, so that invalidation of cache doesn?t get skipped.
>>

As I suggested, if you have to do this before relocation, a macro
CONFIG_SYS_CPC_REINIT_F makes more sense.

York

[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[Scott Wood]

On Fri, 2014-03-07 at 11:07 -0800, York Sun wrote:
> On 03/07/2014 11:00 AM, Scott Wood wrote:
> > On Fri, 2014-03-07 at 10:58 -0800, York Sun wrote:
> >> On 03/07/2014 10:54 AM, Scott Wood wrote:
> >>> On Fri, 2014-03-07 at 10:30 -0800, York Sun wrote:
> >>>> On 03/07/2014 05:23 AM, Aneesh Bansal wrote:
> >>>>> Changes:
> >>>>> 1. L2 cache is being invalidated by Boot ROM code for e6500 core.
> >>>>>    So removing the invalidation from start.S
> >>>>> 2. Clear the LAW and corresponding configuration for CPC. Boot ROM
> >>>>>    code uses it as hosekeeping area.
> >>>>> 3. For Secure boot, CPC is configured as SRAM and used as house
> >>>>>    keeping area. This configuration is to be disabled once in uboot.
> >>>>>    Earlier this disabling of CPC as SRAM was happening in cpu_init_r.
> >>>>>    As a result cache invalidation function was getting skipped in
> >>>>>    case CPC is configured as SRAM.This was causing random crashes.
> >>>>>
> >>>>> Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
> >>>>> ---
> >>>>>  README                                     |  4 ++++
> >>>>>  arch/powerpc/cpu/mpc85xx/cpu_init.c        | 27 ++++++++++++++++++++++-----
> >>>>>  arch/powerpc/cpu/mpc85xx/start.S           |  3 ++-
> >>>>>  arch/powerpc/include/asm/fsl_secure_boot.h |  6 ++++++
> >>>>>  boards.cfg                                 |  1 +
> >>>>>  5 files changed, 35 insertions(+), 6 deletions(-)
> >>>>>
> >>>>> Changes from v3:
> >>>>> Renamed the macro to indicate CPC configured as SRAM at U-boot entry to
> >>>>> CONFIG_SYS_CPC_SRAM 
> >>>>
> >>>> Aneesh,
> >>>>
> >>>> I understand you are trying to address the comment about when CPC needs to be
> >>>> disabled before initializing as normal CPC. But introducing CONFIG_SYS_CPC_SRAM
> >>>> seems even more confusing. Let's take one step back.
> >>>>
> >>>> CPC as SRAM can be used for many reasons. There is only one reason we should
> >>>> keep it as SRAM, i.e. u-boot is indeed using SRAM as its final destination. For
> >>>> all other usages, we can safely reconfigure it as normal CPC after u-boot
> >>>> relocates itself into DDR. If u-boot's final destination is SRAM, it is a
> >>>> RAMBOOT. Can we use this condition to deal with CPC?
> >>>
> >>> Even if CPC is the final destination, shouldn't RAMBOOT imply that DDR
> >>> has been initialized?
> >>
> >> No. RAMBOOT implies u-boot shouldn't initialize RAM. There may be cases where
> >> u-boot needs to run in SRAM without DDR. It doesn't matter if the RAM is SRAM or
> >> DRAM.
> > 
> > There are some situations where we run U-Boot out of SRAM without
> > relocating to DDR, but I can't think of a situation where we wouldn't
> > want to use DDR at all (e.g. for loading an OS to).
> > 
> 
> I generally agree with you. Loading OS needs more RAM.
> 
> I am trying to simplify the condition to reinitialize CPC. My point is only one
> situation requires the CPC to act as SRAM. We can brainstorm how useful it is.
> For example, as a recovery method if using JTAG tool to enable CPC as SRAM and
> to load u-boot into it, one can skip the steps to initialize DDR to run an
> full-feature u-boot and reflash the board.

What's relevant isn't whether there is a use case for not needing DDR,
but whether there's a use case for having a special build of U-Boot that
skips initializing DDR.

-Scott

[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[Scott Wood]

On Mon, 2014-03-10 at 08:38 -0700, York Sun wrote:
> On 03/10/2014 02:14 AM, Bansal Aneesh-B39320 wrote:
> >> -----Original Message-----
> >> From: Sun York-R58495
> >> Sent: Saturday, March 08, 2014 12:01 AM
> >> To: Bansal Aneesh-B39320; u-boot at lists.denx.de
> >> Cc: Wood Scott-B07421
> >> Subject: Re: [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot
> >> target for B4860QDS
> >>
> >> Do you see the need to disable CPC before relocation? You are doing so
> >> in this patch. Does the temporary LAW or address conflict with u-boot?
> > Earlier we were disabling cpc in cpu_init_r. However since cache invalidation function was getting skipped in the process, it was causing random crashes. Skipping invalidation works in RAMBOOT scenario, however since we don?t have valid data when CPC is configured as cache for the secure boot scenario which is not RAMBOOT, these crashes were occurring in few platforms. As a result we had to move this disable code early in the sequence, so that invalidation of cache doesn?t get skipped.
> >>
> 
> As I suggested, if you have to do this before relocation, a macro
> CONFIG_SYS_CPC_REINIT_F makes more sense.

How hard would it be to check the status of CPC at runtime, or just
unconditionally reinit (for non-ramboot)?

I don't think I ever saw an answer to the question of what harm it does
to leave CPC alone until the normal place where we init CPC.

-Scott

[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[York Sun]

On 03/10/2014 04:55 PM, Scott Wood wrote:
> On Mon, 2014-03-10 at 08:38 -0700, York Sun wrote:
>> On 03/10/2014 02:14 AM, Bansal Aneesh-B39320 wrote:
>>>> -----Original Message-----
>>>> From: Sun York-R58495
>>>> Sent: Saturday, March 08, 2014 12:01 AM
>>>> To: Bansal Aneesh-B39320; u-boot at lists.denx.de
>>>> Cc: Wood Scott-B07421
>>>> Subject: Re: [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot
>>>> target for B4860QDS
>>>>
>>>> Do you see the need to disable CPC before relocation? You are doing so
>>>> in this patch. Does the temporary LAW or address conflict with u-boot?
>>> Earlier we were disabling cpc in cpu_init_r. However since cache invalidation function was getting skipped in the process, it was causing random crashes. Skipping invalidation works in RAMBOOT scenario, however since we don?t have valid data when CPC is configured as cache for the secure boot scenario which is not RAMBOOT, these crashes were occurring in few platforms. As a result we had to move this disable code early in the sequence, so that invalidation of cache doesn?t get skipped.
>>>>
>>
>> As I suggested, if you have to do this before relocation, a macro
>> CONFIG_SYS_CPC_REINIT_F makes more sense.
> 
> How hard would it be to check the status of CPC at runtime, or just
> unconditionally reinit (for non-ramboot)?

That's what I was suggesting.

> 
> I don't think I ever saw an answer to the question of what harm it does
> to leave CPC alone until the normal place where we init CPC.
> 

Aneesh seems to believe disabling and re-initializing CPC in cpu_inti_r is too
late for SECURE BOOT.

York

[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[York Sun]

On 03/10/2014 04:24 PM, Scott Wood wrote:
> 
> On Fri, 2014-03-07 at 11:07 -0800, York Sun wrote:
>> On 03/07/2014 11:00 AM, Scott Wood wrote:
>>> On Fri, 2014-03-07 at 10:58 -0800, York Sun wrote:
>>>> On 03/07/2014 10:54 AM, Scott Wood wrote:
>>>>> On Fri, 2014-03-07 at 10:30 -0800, York Sun wrote:
>>>>>> On 03/07/2014 05:23 AM, Aneesh Bansal wrote:
>>>>>>> Changes:
>>>>>>> 1. L2 cache is being invalidated by Boot ROM code for e6500 core.
>>>>>>>    So removing the invalidation from start.S
>>>>>>> 2. Clear the LAW and corresponding configuration for CPC. Boot ROM
>>>>>>>    code uses it as hosekeeping area.
>>>>>>> 3. For Secure boot, CPC is configured as SRAM and used as house
>>>>>>>    keeping area. This configuration is to be disabled once in uboot.
>>>>>>>    Earlier this disabling of CPC as SRAM was happening in cpu_init_r.
>>>>>>>    As a result cache invalidation function was getting skipped in
>>>>>>>    case CPC is configured as SRAM.This was causing random crashes.
>>>>>>>
>>>>>>> Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
>>>>>>> ---
>>>>>>>  README                                     |  4 ++++
>>>>>>>  arch/powerpc/cpu/mpc85xx/cpu_init.c        | 27 ++++++++++++++++++++++-----
>>>>>>>  arch/powerpc/cpu/mpc85xx/start.S           |  3 ++-
>>>>>>>  arch/powerpc/include/asm/fsl_secure_boot.h |  6 ++++++
>>>>>>>  boards.cfg                                 |  1 +
>>>>>>>  5 files changed, 35 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> Changes from v3:
>>>>>>> Renamed the macro to indicate CPC configured as SRAM at U-boot entry to
>>>>>>> CONFIG_SYS_CPC_SRAM 
>>>>>>
>>>>>> Aneesh,
>>>>>>
>>>>>> I understand you are trying to address the comment about when CPC needs to be
>>>>>> disabled before initializing as normal CPC. But introducing CONFIG_SYS_CPC_SRAM
>>>>>> seems even more confusing. Let's take one step back.
>>>>>>
>>>>>> CPC as SRAM can be used for many reasons. There is only one reason we should
>>>>>> keep it as SRAM, i.e. u-boot is indeed using SRAM as its final destination. For
>>>>>> all other usages, we can safely reconfigure it as normal CPC after u-boot
>>>>>> relocates itself into DDR. If u-boot's final destination is SRAM, it is a
>>>>>> RAMBOOT. Can we use this condition to deal with CPC?
>>>>>
>>>>> Even if CPC is the final destination, shouldn't RAMBOOT imply that DDR
>>>>> has been initialized?
>>>>
>>>> No. RAMBOOT implies u-boot shouldn't initialize RAM. There may be cases where
>>>> u-boot needs to run in SRAM without DDR. It doesn't matter if the RAM is SRAM or
>>>> DRAM.
>>>
>>> There are some situations where we run U-Boot out of SRAM without
>>> relocating to DDR, but I can't think of a situation where we wouldn't
>>> want to use DDR at all (e.g. for loading an OS to).
>>>
>>
>> I generally agree with you. Loading OS needs more RAM.
>>
>> I am trying to simplify the condition to reinitialize CPC. My point is only one
>> situation requires the CPC to act as SRAM. We can brainstorm how useful it is.
>> For example, as a recovery method if using JTAG tool to enable CPC as SRAM and
>> to load u-boot into it, one can skip the steps to initialize DDR to run an
>> full-feature u-boot and reflash the board.
> 
> What's relevant isn't whether there is a use case for not needing DDR,
> but whether there's a use case for having a special build of U-Boot that
> skips initializing DDR.
> 

There are cases, such as using JTAG tools before, using PBI commands recently.
There have been other cases when DDR is not or cannot be used, a RAMBOOT u-boot
will use SRAM. All these cases need u-boot skipping initializing DDR.

York

[U-Boot] [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot target for B4860QDS

[aneesh.bansal at freescale.com]

> -----Original Message-----
> From: Sun York-R58495
> Sent: Tuesday, March 11, 2014 5:31 AM
> To: Wood Scott-B07421
> Cc: Bansal Aneesh-B39320; u-boot at lists.denx.de; Gupta Ruchika-R66431
> Subject: Re: [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure boot
> target for B4860QDS
> 
> On 03/10/2014 04:55 PM, Scott Wood wrote:
> > On Mon, 2014-03-10 at 08:38 -0700, York Sun wrote:
> >> On 03/10/2014 02:14 AM, Bansal Aneesh-B39320 wrote:
> >>>> -----Original Message-----
> >>>> From: Sun York-R58495
> >>>> Sent: Saturday, March 08, 2014 12:01 AM
> >>>> To: Bansal Aneesh-B39320; u-boot at lists.denx.de
> >>>> Cc: Wood Scott-B07421
> >>>> Subject: Re: [PATCH][v4] powerpc/mpc85xx: SECURE BOOT- Add secure
> >>>> boot target for B4860QDS
> >>>>
> >>>> Do you see the need to disable CPC before relocation? You are
> doing
> >>>> so in this patch. Does the temporary LAW or address conflict with
> u-boot?
> >>> Earlier we were disabling cpc in cpu_init_r. However since cache
> invalidation function was getting skipped in the process, it was
> causing random crashes. Skipping invalidation works in RAMBOOT
> scenario, however since we don?t have valid data when CPC is
> configured as cache for the secure boot scenario which is not RAMBOOT,
> these crashes were occurring in few platforms. As a result we had to
> move this disable code early in the sequence, so that invalidation of
> cache doesn?t get skipped.
> >>>>
> >>
> >> As I suggested, if you have to do this before relocation, a macro
> >> CONFIG_SYS_CPC_REINIT_F makes more sense.
> >
> > How hard would it be to check the status of CPC at runtime, or just
> > unconditionally reinit (for non-ramboot)?
> 
> That's what I was suggesting.
> 
> >
> > I don't think I ever saw an answer to the question of what harm it
> > does to leave CPC alone until the normal place where we init CPC.
> >
> 
> Aneesh seems to believe disabling and re-initializing CPC in
> cpu_inti_r is too late for SECURE BOOT.
> 
> York
> 
Yes, we have seen random crashes occur during Secure Boot if CPC is not disabled
before cache invalidation and re-initialization.

Aneesh