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

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: Ruchika Gupta <ruchika.gupta@freescale.com>
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 v2:
Squashed with previous patch after removing the ISBC version as per discussion.
Changed the Subject Line.

Dear Aneesh Bansal,

In message <1391419033-14283-1-git-send-email-aneesh.bansal@freescale.com> you 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.
...
> +#if defined(CONFIG_RAMBOOT_PBL)
> +	disable_cpc_sram();
> +#endif

What is the meaning of this undocumented CONFIG_RAMBOOT_PBL option?

As far as I understand, this is not a boot from RAM at all, but a
totally normal step in a boot process form regular boot media?

Best regards,

Wolfgang Denk

On Mon, 2014-02-03 at 14:47 +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: Ruchika Gupta <ruchika.gupta@freescale.com>
> 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 v2:
> Squashed with previous patch after removing the ISBC version as per discussion.
> Changed the Subject Line.
> 
> diff --git a/README b/README
> index 176de61..3ca307f 100644
> --- a/README
> +++ b/README
> @@ -428,6 +428,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_SECURE_HKAREA_CPC
> +		This CONFIG is defined for the SoC's in which the BootROM code uses
> +		the platform cache configured as SRAM for house keeping.

What sort of house keeping?  The bootrom is finished by the time U-Boot
starts, right?  What restrictions are there on U-Boot's use of the SRAM?

-Scott

On Mon, 2014-02-03 at 19:58 +0100, Wolfgang Denk wrote:
> Dear Aneesh Bansal,
> 
> In message <1391419033-14283-1-git-send-email-aneesh.bansal@freescale.com> you 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.
> ...
> > +#if defined(CONFIG_RAMBOOT_PBL)
> > +	disable_cpc_sram();
> > +#endif
> 
> What is the meaning of this undocumented CONFIG_RAMBOOT_PBL option?

I agree it should be documented, but it's not new to this patch.

> As far as I understand, this is not a boot from RAM at all, but a
> totally normal step in a boot process form regular boot media?

FWIW, the only documentation I can find for CONFIG_SYS_RAMBOOT is
doc/README.mpc85xx and doc/README.ramboot-ppc85xx which specify its
usage in "totally normal steps in a boot process from regular boot
media".

Any instance of "booting from RAM" is going to need the image to get
into RAM somehow.  If the symbol is to mean anything at all, it seems
reasonable that it means that U-Boot was executing from RAM when the
current image was entered, regardless of how that came to be.  There
should be some way to distinguish running from SRAM versus running from
DDR, though.

-Scott

Dear Scott,

In message <1391469528.6733.97.camel@snotra.buserror.net> you wrote:
>
> > > +#if defined(CONFIG_RAMBOOT_PBL)
> > > +	disable_cpc_sram();
> > > +#endif
> > 
> > What is the meaning of this undocumented CONFIG_RAMBOOT_PBL option?
> 
> I agree it should be documented, but it's not new to this patch.

Of course you are right.  But this patch clearly shows the need to
document such things, or more and more code will be added based on
incorrect interpretations.

> FWIW, the only documentation I can find for CONFIG_SYS_RAMBOOT is
> doc/README.mpc85xx and doc/README.ramboot-ppc85xx which specify its
> usage in "totally normal steps in a boot process from regular boot
> media".

Agreed.  The original purpose has never been documented, whichi s a
serious pity, as now we only have the misinterpreting documents you
refer to.

> Any instance of "booting from RAM" is going to need the image to get
> into RAM somehow.  If the symbol is to mean anything at all, it seems
> reasonable that it means that U-Boot was executing from RAM when the

Yes, "booting from RAM" has been indeed intended for confiurations
where the U-Boot image was brought to RAM by some magic means totally
outside of normal operations of the CPU.

The first board that would meet such a description (and that I can
remember of) was the PN62 board; this was a PCI card, where the U-Boot
code was loaded into memory from the host while the CPU was helt in
reset, and subsequently it directly started executing from RAM.

Unfortunately I cannot check currently if my memory is correct and
PN62 was also where we introduced that variable (at that time under
the name CFG_RAMBOOT), as this was before U-Boot (still in PPCBoot)
and I only have the old CVS repository for that archived somewhere.

The next example of "correct" use of CONFIG_SYS_RAMBOOT was when
Stefan Roese added support to the PPC440EPx based Sequoia board to run
an U-Boot image that had been loaded to RAM using a JTAG debugger.


In any case, RAMBOOT was supposed to be used for systems, where code
execution started in RAM right out of reset, i. e. without any other
boot device like NAND, SPI flash, SDcard or similar.


> current image was entered, regardless of how that came to be.  There
> should be some way to distinguish running from SRAM versus running from
> DDR, though.

I cannot see how this is related to the RAMBOOT discussion. Actually
I cannot even see why such distinguishing would be needed at all
(unless you have very specific ideas about the characteristics of
SRAM versus DDR on some systems): SRAM and DDR are just examples of
hardware implementations for the more generic term RAM, i. e.
writable system memory from wich code can be executed (in contrast to
some storage device from which no code can be executed without
previously loading it to RAM, like NAND, SPI flash, SDcard, USB
storage devices, hard disk drives, etc. etc.)

Best regards,

Wolfgang Denk

> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Tuesday, February 04, 2014 4:32 AM
> To: Bansal Aneesh-B39320
> Cc: u-boot@lists.denx.de; Sun York-R58495; Gupta Ruchika-R66431
> Subject: Re: [PATCH][v3] powerpc/mpc85xx: SECURE BOOT- Add secure boot
> target for B4860QDS
> 
> On Mon, 2014-02-03 at 14:47 +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: Ruchika Gupta <ruchika.gupta@freescale.com>
> > 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 v2:
> > Squashed with previous patch after removing the ISBC version as per
> discussion.
> > Changed the Subject Line.
> >
> > diff --git a/README b/README
> > index 176de61..3ca307f 100644
> > --- a/README
> > +++ b/README
> > @@ -428,6 +428,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_SECURE_HKAREA_CPC
> > +		This CONFIG is defined for the SoC's in which the BootROM
> code uses
> > +		the platform cache configured as SRAM for house keeping.
> 
> What sort of house keeping?  The bootrom is finished by the time U-Boot
> starts, right?  What restrictions are there on U-Boot's use of the SRAM?
> 
> -Scott
> 
For Secure Boot, CPC is configured as SRAM by the PBI commands and BootROM uses
it for sending commands to SEC block. In U-boot CPC is to be configured as platform
cache and before doing so, the configuration of CPC as SRAM needs to be removed.

On Tue, 2014-02-04 at 01:41 -0600, Bansal Aneesh-B39320 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Tuesday, February 04, 2014 4:32 AM
> > To: Bansal Aneesh-B39320
> > Cc: u-boot@lists.denx.de; Sun York-R58495; Gupta Ruchika-R66431
> > Subject: Re: [PATCH][v3] powerpc/mpc85xx: SECURE BOOT- Add secure boot
> > target for B4860QDS
> > 
> > On Mon, 2014-02-03 at 14:47 +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: Ruchika Gupta <ruchika.gupta@freescale.com>
> > > 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 v2:
> > > Squashed with previous patch after removing the ISBC version as per
> > discussion.
> > > Changed the Subject Line.
> > >
> > > diff --git a/README b/README
> > > index 176de61..3ca307f 100644
> > > --- a/README
> > > +++ b/README
> > > @@ -428,6 +428,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_SECURE_HKAREA_CPC
> > > +		This CONFIG is defined for the SoC's in which the BootROM
> > code uses
> > > +		the platform cache configured as SRAM for house keeping.
> > 
> > What sort of house keeping?  The bootrom is finished by the time U-Boot
> > starts, right?  What restrictions are there on U-Boot's use of the SRAM?
> > 
> > -Scott
> > 
> For Secure Boot, CPC is configured as SRAM by the PBI commands and BootROM uses
> it for sending commands to SEC block.

Why does U-Boot care?

>  In U-boot CPC is to be configured as platform
> cache and before doing so, the configuration of CPC as SRAM needs to be removed.

So all you need is something saying that U-Boot is entered with the CPC
configured as SRAM.  We don't care whether it's for "secure hkarea".
Right?

-Scott

On Tue, 2014-02-04 at 08:03 +0100, Wolfgang Denk wrote:
> Dear Scott,
> 
> In message <1391469528.6733.97.camel@snotra.buserror.net> you wrote:
> >
> > > > +#if defined(CONFIG_RAMBOOT_PBL)
> > > > +	disable_cpc_sram();
> > > > +#endif
> > > 
> > > What is the meaning of this undocumented CONFIG_RAMBOOT_PBL option?
> > 
> > I agree it should be documented, but it's not new to this patch.
> 
> Of course you are right.  But this patch clearly shows the need to
> document such things, or more and more code will be added based on
> incorrect interpretations.
> 
> > FWIW, the only documentation I can find for CONFIG_SYS_RAMBOOT is
> > doc/README.mpc85xx and doc/README.ramboot-ppc85xx which specify its
> > usage in "totally normal steps in a boot process from regular boot
> > media".
> 
> Agreed.  The original purpose has never been documented, whichi s a
> serious pity, as now we only have the misinterpreting documents you
> refer to.
> 
> > Any instance of "booting from RAM" is going to need the image to get
> > into RAM somehow.  If the symbol is to mean anything at all, it seems
> > reasonable that it means that U-Boot was executing from RAM when the
> 
> Yes, "booting from RAM" has been indeed intended for confiurations
> where the U-Boot image was brought to RAM by some magic means totally
> outside of normal operations of the CPU.

Why does U-Boot care, though, whether the means were totally outside the
normal operations of the CPU, or something that was done by some other
software component running on the CPU?  What U-Boot needs to know is the
present state of the system.

> > current image was entered, regardless of how that came to be.  There
> > should be some way to distinguish running from SRAM versus running from
> > DDR, though.
> 
> I cannot see how this is related to the RAMBOOT discussion.

It's related because it's part of what CONFIG_RAMBOOT_PBL is being used
to indicate.

> Actually I cannot even see why such distinguishing would be needed at all
> (unless you have very specific ideas about the characteristics of
> SRAM versus DDR on some systems): SRAM and DDR are just examples of
> hardware implementations for the more generic term RAM, i. e.
> writable system memory from wich code can be executed (in contrast to
> some storage device from which no code can be executed without
> previously loading it to RAM, like NAND, SPI flash, SDcard, USB
> storage devices, hard disk drives, etc. etc.)

The difference is relevant (at least on some platforms) because U-Boot
needs to know which TLB entries to have during early boot, whether it
needs to initialize the DDR controller, whether the cache is currently
configured as an SRAM, etc.

Perhaps it could detect this information at runtime, but currently it
does not for at least some of the above.  It also affects the
pre-relocation link address, which must be compile time (but doesn't
necessarily need a symbol that's visible outside the board config file).

-Scott