diff mbox

[U-Boot,1/3] powerpc/p1010rdb: SECURE BOOT- define CONFIG_SYS_RAMBOOT for NAND boot

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

Commit Message

Aneesh Bansal Jan. 20, 2014, 9:25 a.m. UTC
In case of secure boot, boot from NAND is ramboot.
It was removed by some other commit. So defining it again.

Signed-off-by: Aneesh Bansal <aneesh.bansal@freescale.com>
---
 include/configs/P1010RDB.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Scott Wood Jan. 20, 2014, 10:04 p.m. UTC | #1
On Mon, 2014-01-20 at 14:55 +0530, Aneesh Bansal wrote:
> In case of secure boot, boot from NAND is ramboot.
> It was removed by some other commit. So defining it again.

In case of not-secure-boot, it's not ramboot.

What user of CONFIG_SYS_RAMBOOT are you concerned about?  Many of them
look like this:

#elif !defined(CONFIG_SYS_RAMBOOT) && defined(CONFIG_SECURE_BOOT)

...which doesn't make sense if secure boot is always considered ramboot.

-Scott
Aneesh Bansal Jan. 27, 2014, 7:20 a.m. UTC | #2
>> In case of secure boot, boot from NAND is ramboot.
>> It was removed by some other commit. So defining it again.
>
>In case of not-secure-boot, it's not ramboot.
>
>What user of CONFIG_SYS_RAMBOOT are you concerned about?  Many of them
>look like this:
>
>#elif !defined(CONFIG_SYS_RAMBOOT) && defined(CONFIG_SECURE_BOOT)
>
>...which doesn't make sense if secure boot is always considered ramboot.
>
>-Scott

CONFIG_SYS_RAMBOOT is for secure boot from NAND, SPI and SD.
CONFIG_SYS_RAMBOOT is not used for secure boot from NOR.
Wolfgang Denk Jan. 27, 2014, 2:22 p.m. UTC | #3
Dear "aneesh.bansal@freescale.com",

In message <680c371d651d49a08b33ddd4d01fb3bd@DM2PR03MB415.namprd03.prod.outlook.com> you wrote:
> >> In case of secure boot, boot from NAND is ramboot.
> >> It was removed by some other commit. So defining it again.
> >
> >In case of not-secure-boot, it's not ramboot.
> >
> >What user of CONFIG_SYS_RAMBOOT are you concerned about?  Many of them
> >look like this:
> >
> >#elif !defined(CONFIG_SYS_RAMBOOT) && defined(CONFIG_SECURE_BOOT)
> >
> >...which doesn't make sense if secure boot is always considered ramboot.
> >
> >-Scott
> 
> CONFIG_SYS_RAMBOOT is for secure boot from NAND, SPI and SD.

This is a misuse of the variable.  The established meaning does NOT
include booding from standard boot media like NAND, SPI and SD.  It
refers to booting from RAM "directly", after the image has been placed
into RAM by "external" means, say by a JTAG debugger, or by copying it
to a PCI card's memory.

Please avoid such misleading usage.

Best regards,

Wolfgang Denk
Aneesh Bansal Jan. 28, 2014, 4:47 a.m. UTC | #4
I think I have caused some confusion when trying to explain. I will try and elaborate on it further. Please see inline.


> -----Original Message-----
> From: Wolfgang Denk [mailto:wd@denx.de]
> Sent: Monday, January 27, 2014 7:52 PM
> To: Bansal Aneesh-B39320
> Cc: Wood Scott-B07421; u-boot@lists.denx.de; Sun York-R58495
> Subject: Re: [U-Boot] [PATCH 1/3] powerpc/p1010rdb: SECURE BOOT- define
> CONFIG_SYS_RAMBOOT for NAND boot
> 
> Dear "aneesh.bansal@freescale.com",
> 
> In message
> <680c371d651d49a08b33ddd4d01fb3bd@DM2PR03MB415.namprd03.prod.outlook.com>
> you wrote:
> > >> In case of secure boot, boot from NAND is ramboot.
> > >> It was removed by some other commit. So defining it again.
> > >
> > >In case of not-secure-boot, it's not ramboot.
> > >
> > >What user of CONFIG_SYS_RAMBOOT are you concerned about?  Many of
> > >them look like this:
> > >
> > >#elif !defined(CONFIG_SYS_RAMBOOT) && defined(CONFIG_SECURE_BOOT)
> > >
> > >...which doesn't make sense if secure boot is always considered
> ramboot.

This code snippet is from the file start.S. This for the scenarios when
1.  We are NOT booting from RAM and 
2.  Secure booting is configured

i.e. the above case  is for secure Boot from NOR.

> > >
> > >-Scott
> >
> > CONFIG_SYS_RAMBOOT is for secure boot from NAND, SPI and SD.
> 
> This is a misuse of the variable.  The established meaning does NOT
> include booding from standard boot media like NAND, SPI and SD.  It
> refers to booting from RAM "directly", after the image has been placed
> into RAM by "external" means, say by a JTAG debugger, or by copying it to
> a PCI card's memory.
> 
> Please avoid such misleading usage.
> 
> Best regards,
> 
> Wolfgang Denk
> 
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> Time is a drug. Too much of it kills you.
>                                       - Terry Pratchett, _Small Gods_
> 

Sorry for the misleading statement. Yes, CONFIG_SYS_RAMBOOT is for booting from RAM directly and we have used it for the same purpose in the patch. Let me try and explain.

What I meant is that for P1010 like platforms  for normal (non-secure) boot in case of NAND, we don’t use the CONFIG_SYS_RAMBOOT as we don’t boot from RAM directly in this case. 

However in case of secure boot, even for NAND, we boot from RAM directly with boot ROM (ISBC) code copying the image from NAND to RAM. So in P1010RDB.h config file, for NAND Secure boot, we have defined CONFIG_RAMBOOT_NAND and then further are enabling CONFIG_SYS_RAMBOOT for the same.

#ifdef CONFIG_NAND_SECBOOT	/* NAND Boot */
#define CONFIG_RAMBOOT_NAND
.
.
.
#if defined(CONFIG_RAMBOOT_SDCARD) || defined(CONFIG_RAMBOOT_SPIFLASH) || \
	defined(CONFIG_RAMBOOT_NAND)
#define CONFIG_SYS_RAMBOOT

Regards,
Aneesh Bansal
Wolfgang Denk Jan. 28, 2014, 5:35 a.m. UTC | #5
Dear Aneesh,

In message <7f771c64c5c44208b24e38c75e159f28@DM2PR03MB415.namprd03.prod.outlook.com> you wrote:
>
> Sorry for the misleading statement. Yes, CONFIG_SYS_RAMBOOT is for
> booting from RAM directly and we have used it for the same purpose in
> the patch. Let me try and explain.

Mind the "directly" in this sentence, and no, you have not used it in
this way.

> What I meant is that for P1010 like platforms for normal (non-secure)
> boot in case of NAND, we don't use the CONFIG_SYS_RAMBOOT as we
> don't boot from RAM directly in this case.

You also do not boot directly from RAM in the secure case.

> However in case of secure boot, even for NAND, we boot from RAM
> directly with boot ROM (ISBC) code copying the image from NAND to
> RAM. So in P1010RDB.h config file, for NAND Secure boot, we have

This is NOT a RAM boot.  This is a completely normal boot process from
NAND.  You must not declare this as RAM booting, as it is NOT.

> defined CONFIG_RAMBOOT_NAND and then further are enabling
> CONFIG_SYS_RAMBOOT for the same.

This is wrong.  You have the ROM boot loader booting from NAND and
not from RAM.

The name CONFIG_RAMBOOT_NAND is an oxymoron in itself (and it's
undocumented as well).   This should be fixed, as it makes no sense.

You cannot "RAM-boot form NAND" - you can always boot from a single
boot device only - either NOR or NAND or SDCard or ... or RAM.

Best regards,

Wolfgang Denk
York Sun March 4, 2014, 5:11 p.m. UTC | #6
On 01/27/2014 09:35 PM, Wolfgang Denk wrote:
> Dear Aneesh,
> 
> In message <7f771c64c5c44208b24e38c75e159f28@DM2PR03MB415.namprd03.prod.outlook.com> you wrote:
>>
>> Sorry for the misleading statement. Yes, CONFIG_SYS_RAMBOOT is for
>> booting from RAM directly and we have used it for the same purpose in
>> the patch. Let me try and explain.
> 
> Mind the "directly" in this sentence, and no, you have not used it in
> this way.
> 
>> What I meant is that for P1010 like platforms for normal (non-secure)
>> boot in case of NAND, we don't use the CONFIG_SYS_RAMBOOT as we
>> don't boot from RAM directly in this case.
> 
> You also do not boot directly from RAM in the secure case.
> 
>> However in case of secure boot, even for NAND, we boot from RAM
>> directly with boot ROM (ISBC) code copying the image from NAND to
>> RAM. So in P1010RDB.h config file, for NAND Secure boot, we have
> 
> This is NOT a RAM boot.  This is a completely normal boot process from
> NAND.  You must not declare this as RAM booting, as it is NOT.
> 
>> defined CONFIG_RAMBOOT_NAND and then further are enabling
>> CONFIG_SYS_RAMBOOT for the same.
> 
> This is wrong.  You have the ROM boot loader booting from NAND and
> not from RAM.
> 
> The name CONFIG_RAMBOOT_NAND is an oxymoron in itself (and it's
> undocumented as well).   This should be fixed, as it makes no sense.
> 
> You cannot "RAM-boot form NAND" - you can always boot from a single
> boot device only - either NOR or NAND or SDCard or ... or RAM.
> 

Let me try to get the bottom of the CONFIG_SYS_RAMBOOT.

When a complete u-boot image boots from RAM (DDR, SRAM, etc), we can call it
RAMBOOT regardless how the image is loaded there. It can be JTAG, PBI, or other
bootloader. This line becomes blurred when we have SPL, TPL, secure boot. We
need to go back to see why we had CONFIG_SYS_RAMBOOT at the first place. If I
remember correctly, we had this RAMBOOT because we didn't want u-boot to
initialized DRAM for obvious reason. If we still have the same purpose of
RAMBOOT, then it is not difficult to draw the line for current situations.

Any booting method which initializes DRAM in its complete image is not a
RAMBOOT. So clearly NAND boot has its own DDR init code, even sometimes using
fixed register settings. So a normal NAND boot is not RAMBOOT. If a u-boot image
is wrapped by another bootloader, and the bootloader is responsible to
initialize DRAM, the u-boot image runs from DRAM doesn't init DRAM again, this
u-boot is a RAMBOOT.

Now we are back to secure NAND boot. Does the secure boot mechanism initialize
DRAM and load a complete u-boot image from NAND to DRAM? If true, then we can
call that image a RAMBOOT. But if the image has its own code to initialize DRAM,
it is not a RAMBOOT.

Do we all agree on this clarification?

York
Aneesh Bansal March 5, 2014, 5:30 a.m. UTC | #7
> -----Original Message-----
> From: Sun York-R58495
> Sent: Tuesday, March 04, 2014 10:42 PM
> To: Wolfgang Denk; Bansal Aneesh-B39320
> Cc: Wood Scott-B07421; u-boot@lists.denx.de; Gupta Ruchika-R66431
> Subject: Re: [U-Boot] [PATCH 1/3] powerpc/p1010rdb: SECURE BOOT-
> define CONFIG_SYS_RAMBOOT for NAND boot
> 
> On 01/27/2014 09:35 PM, Wolfgang Denk wrote:
> > Dear Aneesh,
> >
> > In message
> <7f771c64c5c44208b24e38c75e159f28@DM2PR03MB415.namprd03.prod.outlook.c
> om> you wrote:
> >>
> >> Sorry for the misleading statement. Yes, CONFIG_SYS_RAMBOOT is for
> >> booting from RAM directly and we have used it for the same purpose
> in
> >> the patch. Let me try and explain.
> >
> > Mind the "directly" in this sentence, and no, you have not used it
> in
> > this way.
> >
> >> What I meant is that for P1010 like platforms for normal (non-
> secure)
> >> boot in case of NAND, we don't use the CONFIG_SYS_RAMBOOT as we
> don't
> >> boot from RAM directly in this case.
> >
> > You also do not boot directly from RAM in the secure case.
> >
> >> However in case of secure boot, even for NAND, we boot from RAM
> >> directly with boot ROM (ISBC) code copying the image from NAND to
> >> RAM. So in P1010RDB.h config file, for NAND Secure boot, we have
> >
> > This is NOT a RAM boot.  This is a completely normal boot process
> from
> > NAND.  You must not declare this as RAM booting, as it is NOT.
> >
> >> defined CONFIG_RAMBOOT_NAND and then further are enabling
> >> CONFIG_SYS_RAMBOOT for the same.
> >
> > This is wrong.  You have the ROM boot loader booting from NAND and
> not
> > from RAM.
> >
> > The name CONFIG_RAMBOOT_NAND is an oxymoron in itself (and it's
> > undocumented as well).   This should be fixed, as it makes no sense.
> >
> > You cannot "RAM-boot form NAND" - you can always boot from a single
> > boot device only - either NOR or NAND or SDCard or ... or RAM.
> >
> 
> Let me try to get the bottom of the CONFIG_SYS_RAMBOOT.
> 
> When a complete u-boot image boots from RAM (DDR, SRAM, etc), we can
> call it RAMBOOT regardless how the image is loaded there. It can be
> JTAG, PBI, or other bootloader. This line becomes blurred when we have
> SPL, TPL, secure boot. We need to go back to see why we had
> CONFIG_SYS_RAMBOOT at the first place. If I remember correctly, we had
> this RAMBOOT because we didn't want u-boot to initialized DRAM for
> obvious reason. If we still have the same purpose of RAMBOOT, then it
> is not difficult to draw the line for current situations.
> 
> Any booting method which initializes DRAM in its complete image is not
> a RAMBOOT. So clearly NAND boot has its own DDR init code, even
> sometimes using fixed register settings. So a normal NAND boot is not
> RAMBOOT. If a u-boot image is wrapped by another bootloader, and the
> bootloader is responsible to initialize DRAM, the u-boot image runs
> from DRAM doesn't init DRAM again, this u-boot is a RAMBOOT.
> 
> Now we are back to secure NAND boot. Does the secure boot mechanism
> initialize DRAM and load a complete u-boot image from NAND to DRAM? If
> true, then we can call that image a RAMBOOT. But if the image has its
> own code to initialize DRAM, it is not a RAMBOOT.
> 
> Do we all agree on this clarification?
> 
> York


Yes, in case of secure boot from NAND, the DRAM is initialized by the BootROM
and complete u-boot image is copied from NAND to DRAM by the BootROM.
So, it should be called RAMBOOT.

Aneesh
Scott Wood March 5, 2014, 6 p.m. UTC | #8
On Tue, 2014-03-04 at 23:30 -0600, Bansal Aneesh-B39320 wrote:
> Yes, in case of secure boot from NAND, the DRAM is initialized by the BootROM
> and complete u-boot image is copied from NAND to DRAM by the BootROM.
> So, it should be called RAMBOOT.

DRAM or SRAM?  Having ROM initialize DDR is a bit scary.

-Scott
Aneesh Bansal March 6, 2014, 9:24 a.m. UTC | #9
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, March 05, 2014 11:30 PM
> To: Bansal Aneesh-B39320
> Cc: Sun York-R58495; Wolfgang Denk; u-boot@lists.denx.de; Gupta
> Ruchika-R66431
> Subject: Re: [U-Boot] [PATCH 1/3] powerpc/p1010rdb: SECURE BOOT-
> define CONFIG_SYS_RAMBOOT for NAND boot
> 
> On Tue, 2014-03-04 at 23:30 -0600, Bansal Aneesh-B39320 wrote:
> > Yes, in case of secure boot from NAND, the DRAM is initialized by
> the
> > BootROM and complete u-boot image is copied from NAND to DRAM by the
> BootROM.
> > So, it should be called RAMBOOT.
> 
> DRAM or SRAM?  Having ROM initialize DDR is a bit scary.
> 
> -Scott
> 
It can be either DDR or SRAM. It is not hardcoded in BootROM to initialize DDR.
This depends on the config words (CF_WORDS) in the CF_HEADER.
The Boot ROM code parses the config words and programs the addresses with data values
accordingly. The user may opt to initialize DDR and get the image copied onto DDR or 
configure CPC as SRAM and get the Image copied onto SRAM.
On 1010, the CPC size is not big enough to accommodate the U-boot image.
So, currently the CF_WORDS are for DDR to be initialized and copy the image on DDR.
York Sun March 7, 2014, 12:35 a.m. UTC | #10
On 03/06/2014 01:24 AM, Bansal Aneesh-B39320 wrote:
>> -----Original Message-----
>> From: Wood Scott-B07421
>> Sent: Wednesday, March 05, 2014 11:30 PM
>> To: Bansal Aneesh-B39320
>> Cc: Sun York-R58495; Wolfgang Denk; u-boot@lists.denx.de; Gupta
>> Ruchika-R66431
>> Subject: Re: [U-Boot] [PATCH 1/3] powerpc/p1010rdb: SECURE BOOT-
>> define CONFIG_SYS_RAMBOOT for NAND boot
>>
>> On Tue, 2014-03-04 at 23:30 -0600, Bansal Aneesh-B39320 wrote:
>>> Yes, in case of secure boot from NAND, the DRAM is initialized by
>> the
>>> BootROM and complete u-boot image is copied from NAND to DRAM by the
>> BootROM.
>>> So, it should be called RAMBOOT.
>>
>> DRAM or SRAM?  Having ROM initialize DDR is a bit scary.
>>
>> -Scott
>>
> It can be either DDR or SRAM. It is not hardcoded in BootROM to initialize DDR.
> This depends on the config words (CF_WORDS) in the CF_HEADER.
> The Boot ROM code parses the config words and programs the addresses with data values
> accordingly. The user may opt to initialize DDR and get the image copied onto DDR or 
> configure CPC as SRAM and get the Image copied onto SRAM.
> On 1010, the CPC size is not big enough to accommodate the U-boot image.
> So, currently the CF_WORDS are for DDR to be initialized and copy the image on DDR.
> 

Does it use similar technique like PBI to initialize DDR? If so, it has static
settings which cannot adapt to various speeds. That's OK if you have to do so.
In this case, your u-boot image is a RAMBOOT. I suggest you to update the
subject to avoid mixing NAND boot with RAMBOOT, and add comment or commit
message to explain the image is loaded into DDR and run directly from there.

York
Scott Wood March 7, 2014, 6:57 p.m. UTC | #11
On Thu, 2014-03-06 at 03:24 -0600, Bansal Aneesh-B39320 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, March 05, 2014 11:30 PM
> > To: Bansal Aneesh-B39320
> > Cc: Sun York-R58495; Wolfgang Denk; u-boot@lists.denx.de; Gupta
> > Ruchika-R66431
> > Subject: Re: [U-Boot] [PATCH 1/3] powerpc/p1010rdb: SECURE BOOT-
> > define CONFIG_SYS_RAMBOOT for NAND boot
> > 
> > On Tue, 2014-03-04 at 23:30 -0600, Bansal Aneesh-B39320 wrote:
> > > Yes, in case of secure boot from NAND, the DRAM is initialized by
> > the
> > > BootROM and complete u-boot image is copied from NAND to DRAM by the
> > BootROM.
> > > So, it should be called RAMBOOT.
> > 
> > DRAM or SRAM?  Having ROM initialize DDR is a bit scary.
> > 
> > -Scott
> > 
> It can be either DDR or SRAM. It is not hardcoded in BootROM to initialize DDR.
> This depends on the config words (CF_WORDS) in the CF_HEADER.
> The Boot ROM code parses the config words and programs the addresses with data values
> accordingly. The user may opt to initialize DDR and get the image copied onto DDR or 
> configure CPC as SRAM and get the Image copied onto SRAM.
> On 1010,

P1010?  LS1010?  Something else?

> the CPC size is not big enough to accommodate the U-boot image.
> So, currently the CF_WORDS are for DDR to be initialized and copy the image on DDR.

Generally if SPD is present, it should be used to init DDR rather than
using hardcoded values.  If U-Boot doesn't fit in SRAM, you can use SPL
instead of hardcoded init.

-Scott
York Sun March 7, 2014, 7:01 p.m. UTC | #12
On 03/07/2014 10:57 AM, Scott Wood wrote:
> On Thu, 2014-03-06 at 03:24 -0600, Bansal Aneesh-B39320 wrote:
>>> -----Original Message-----
>>> From: Wood Scott-B07421
>>> Sent: Wednesday, March 05, 2014 11:30 PM
>>> To: Bansal Aneesh-B39320
>>> Cc: Sun York-R58495; Wolfgang Denk; u-boot@lists.denx.de; Gupta
>>> Ruchika-R66431
>>> Subject: Re: [U-Boot] [PATCH 1/3] powerpc/p1010rdb: SECURE BOOT-
>>> define CONFIG_SYS_RAMBOOT for NAND boot
>>>
>>> On Tue, 2014-03-04 at 23:30 -0600, Bansal Aneesh-B39320 wrote:
>>>> Yes, in case of secure boot from NAND, the DRAM is initialized by
>>> the
>>>> BootROM and complete u-boot image is copied from NAND to DRAM by the
>>> BootROM.
>>>> So, it should be called RAMBOOT.
>>>
>>> DRAM or SRAM?  Having ROM initialize DDR is a bit scary.
>>>
>>> -Scott
>>>
>> It can be either DDR or SRAM. It is not hardcoded in BootROM to initialize DDR.
>> This depends on the config words (CF_WORDS) in the CF_HEADER.
>> The Boot ROM code parses the config words and programs the addresses with data values
>> accordingly. The user may opt to initialize DDR and get the image copied onto DDR or 
>> configure CPC as SRAM and get the Image copied onto SRAM.
>> On 1010,
> 
> P1010?  LS1010?  Something else?
> 
>> the CPC size is not big enough to accommodate the U-boot image.
>> So, currently the CF_WORDS are for DDR to be initialized and copy the image on DDR.
> 
> Generally if SPD is present, it should be used to init DDR rather than
> using hardcoded values.  If U-Boot doesn't fit in SRAM, you can use SPL
> instead of hardcoded init.

I agree with Scott on this point. Using hardcoded values totally skip DDR
driver. You don't only lose the flexibility of various speeds, you also skip all
workarounds implemented in DDR driver.

York
Aneesh Bansal March 10, 2014, 9:14 a.m. UTC | #13
> -----Original Message-----
> From: Sun York-R58495
> Sent: Saturday, March 08, 2014 12:31 AM
> To: Bansal Aneesh-B39320
> Cc: Wood Scott-B07421; Wolfgang Denk; u-boot@lists.denx.de; Gupta
> Ruchika-R66431
> Subject: Re: [U-Boot] [PATCH 1/3] powerpc/p1010rdb: SECURE BOOT-
> define CONFIG_SYS_RAMBOOT for NAND boot
> 
> On 03/07/2014 10:57 AM, Scott Wood wrote:
> > On Thu, 2014-03-06 at 03:24 -0600, Bansal Aneesh-B39320 wrote:
> >>> -----Original Message-----
> >>> From: Wood Scott-B07421
> >>> Sent: Wednesday, March 05, 2014 11:30 PM
> >>> To: Bansal Aneesh-B39320
> >>> Cc: Sun York-R58495; Wolfgang Denk; u-boot@lists.denx.de; Gupta
> >>> Ruchika-R66431
> >>> Subject: Re: [U-Boot] [PATCH 1/3] powerpc/p1010rdb: SECURE BOOT-
> >>> define CONFIG_SYS_RAMBOOT for NAND boot
> >>>
> >>> On Tue, 2014-03-04 at 23:30 -0600, Bansal Aneesh-B39320 wrote:
> >>>> Yes, in case of secure boot from NAND, the DRAM is initialized by
> >>> the
> >>>> BootROM and complete u-boot image is copied from NAND to DRAM by
> >>>> the
> >>> BootROM.
> >>>> So, it should be called RAMBOOT.
> >>>
> >>> DRAM or SRAM?  Having ROM initialize DDR is a bit scary.
> >>>
> >>> -Scott
> >>>
> >> It can be either DDR or SRAM. It is not hardcoded in BootROM to
> initialize DDR.
> >> This depends on the config words (CF_WORDS) in the CF_HEADER.
> >> The Boot ROM code parses the config words and programs the
> addresses
> >> with data values accordingly. The user may opt to initialize DDR
> and
> >> get the image copied onto DDR or configure CPC as SRAM and get the
> Image copied onto SRAM.
> >> On 1010,
> >
> > P1010?  LS1010?  Something else?
> >
> >> the CPC size is not big enough to accommodate the U-boot image.
> >> So, currently the CF_WORDS are for DDR to be initialized and copy
> the image on DDR.
> >
> > Generally if SPD is present, it should be used to init DDR rather
> than
> > using hardcoded values.  If U-Boot doesn't fit in SRAM, you can use
> > SPL instead of hardcoded init.
> 
> I agree with Scott on this point. Using hardcoded values totally skip
> DDR driver. You don't only lose the flexibility of various speeds, you
> also skip all workarounds implemented in DDR driver.
> 
> York
Currently we are following the same approach which was there for SPI and SD on P1010 i.e hardcoded initialization of DDR in platforms like P1010, 9131/9132 using config words.

Aneesh
York Sun March 10, 2014, 3:26 p.m. UTC | #14
On 03/10/2014 02:14 AM, Bansal Aneesh-B39320 wrote:
>> -----Original Message-----
>> From: Sun York-R58495
>> Sent: Saturday, March 08, 2014 12:31 AM
>> To: Bansal Aneesh-B39320
>> Cc: Wood Scott-B07421; Wolfgang Denk; u-boot@lists.denx.de; Gupta
>> Ruchika-R66431
>> Subject: Re: [U-Boot] [PATCH 1/3] powerpc/p1010rdb: SECURE BOOT-
>> define CONFIG_SYS_RAMBOOT for NAND boot
>>
>> On 03/07/2014 10:57 AM, Scott Wood wrote:
>>> On Thu, 2014-03-06 at 03:24 -0600, Bansal Aneesh-B39320 wrote:
>>>>> -----Original Message-----
>>>>> From: Wood Scott-B07421
>>>>> Sent: Wednesday, March 05, 2014 11:30 PM
>>>>> To: Bansal Aneesh-B39320
>>>>> Cc: Sun York-R58495; Wolfgang Denk; u-boot@lists.denx.de; Gupta
>>>>> Ruchika-R66431
>>>>> Subject: Re: [U-Boot] [PATCH 1/3] powerpc/p1010rdb: SECURE BOOT-
>>>>> define CONFIG_SYS_RAMBOOT for NAND boot
>>>>>
>>>>> On Tue, 2014-03-04 at 23:30 -0600, Bansal Aneesh-B39320 wrote:
>>>>>> Yes, in case of secure boot from NAND, the DRAM is initialized by
>>>>> the
>>>>>> BootROM and complete u-boot image is copied from NAND to DRAM by
>>>>>> the
>>>>> BootROM.
>>>>>> So, it should be called RAMBOOT.
>>>>>
>>>>> DRAM or SRAM?  Having ROM initialize DDR is a bit scary.
>>>>>
>>>>> -Scott
>>>>>
>>>> It can be either DDR or SRAM. It is not hardcoded in BootROM to
>> initialize DDR.
>>>> This depends on the config words (CF_WORDS) in the CF_HEADER.
>>>> The Boot ROM code parses the config words and programs the
>> addresses
>>>> with data values accordingly. The user may opt to initialize DDR
>> and
>>>> get the image copied onto DDR or configure CPC as SRAM and get the
>> Image copied onto SRAM.
>>>> On 1010,
>>>
>>> P1010?  LS1010?  Something else?
>>>
>>>> the CPC size is not big enough to accommodate the U-boot image.
>>>> So, currently the CF_WORDS are for DDR to be initialized and copy
>> the image on DDR.
>>>
>>> Generally if SPD is present, it should be used to init DDR rather
>> than
>>> using hardcoded values.  If U-Boot doesn't fit in SRAM, you can use
>>> SPL instead of hardcoded init.
>>
>> I agree with Scott on this point. Using hardcoded values totally skip
>> DDR driver. You don't only lose the flexibility of various speeds, you
>> also skip all workarounds implemented in DDR driver.
>>
>> York
> Currently we are following the same approach which was there for SPI and SD on P1010 i.e hardcoded initialization of DDR in platforms like P1010, 9131/9132 using config words.
> 

Understood.

York
Scott Wood March 10, 2014, 11:58 p.m. UTC | #15
On Mon, 2014-03-10 at 04:14 -0500, Bansal Aneesh-B39320 wrote:
> > -----Original Message-----
> > From: Sun York-R58495
> > Sent: Saturday, March 08, 2014 12:31 AM
> > To: Bansal Aneesh-B39320
> > Cc: Wood Scott-B07421; Wolfgang Denk; u-boot@lists.denx.de; Gupta
> > Ruchika-R66431
> > Subject: Re: [U-Boot] [PATCH 1/3] powerpc/p1010rdb: SECURE BOOT-
> > define CONFIG_SYS_RAMBOOT for NAND boot
> > 
> > On 03/07/2014 10:57 AM, Scott Wood wrote:
> > > On Thu, 2014-03-06 at 03:24 -0600, Bansal Aneesh-B39320 wrote:
> > >>> -----Original Message-----
> > >>> From: Wood Scott-B07421
> > >>> Sent: Wednesday, March 05, 2014 11:30 PM
> > >>> To: Bansal Aneesh-B39320
> > >>> Cc: Sun York-R58495; Wolfgang Denk; u-boot@lists.denx.de; Gupta
> > >>> Ruchika-R66431
> > >>> Subject: Re: [U-Boot] [PATCH 1/3] powerpc/p1010rdb: SECURE BOOT-
> > >>> define CONFIG_SYS_RAMBOOT for NAND boot
> > >>>
> > >>> On Tue, 2014-03-04 at 23:30 -0600, Bansal Aneesh-B39320 wrote:
> > >>>> Yes, in case of secure boot from NAND, the DRAM is initialized by
> > >>> the
> > >>>> BootROM and complete u-boot image is copied from NAND to DRAM by
> > >>>> the
> > >>> BootROM.
> > >>>> So, it should be called RAMBOOT.
> > >>>
> > >>> DRAM or SRAM?  Having ROM initialize DDR is a bit scary.
> > >>>
> > >>> -Scott
> > >>>
> > >> It can be either DDR or SRAM. It is not hardcoded in BootROM to
> > initialize DDR.
> > >> This depends on the config words (CF_WORDS) in the CF_HEADER.
> > >> The Boot ROM code parses the config words and programs the
> > addresses
> > >> with data values accordingly. The user may opt to initialize DDR
> > and
> > >> get the image copied onto DDR or configure CPC as SRAM and get the
> > Image copied onto SRAM.
> > >> On 1010,
> > >
> > > P1010?  LS1010?  Something else?
> > >
> > >> the CPC size is not big enough to accommodate the U-boot image.
> > >> So, currently the CF_WORDS are for DDR to be initialized and copy
> > the image on DDR.
> > >
> > > Generally if SPD is present, it should be used to init DDR rather
> > than
> > > using hardcoded values.  If U-Boot doesn't fit in SRAM, you can use
> > > SPL instead of hardcoded init.
> > 
> > I agree with Scott on this point. Using hardcoded values totally skip
> > DDR driver. You don't only lose the flexibility of various speeds, you
> > also skip all workarounds implemented in DDR driver.
> > 
> > York
>
> Currently we are following the same approach which was there for SPI
> and SD on P1010 i.e hardcoded initialization of DDR in platforms like
> P1010, 9131/9132 using config words.

I don't know about 9131/9132, but on P1010RDB we now use SPL to init DDR
when booting from SPI/SD.

In any case, "we've always done it this way" isn't really a good reason.

-Scott
York Sun March 11, 2014, 3:53 p.m. UTC | #16
On 03/10/2014 04:58 PM, Scott Wood wrote:
> On Mon, 2014-03-10 at 04:14 -0500, Bansal Aneesh-B39320 wrote:
>>> -----Original Message-----
>>> From: Sun York-R58495
>>> Sent: Saturday, March 08, 2014 12:31 AM
>>> To: Bansal Aneesh-B39320
>>> Cc: Wood Scott-B07421; Wolfgang Denk; u-boot@lists.denx.de; Gupta
>>> Ruchika-R66431
>>> Subject: Re: [U-Boot] [PATCH 1/3] powerpc/p1010rdb: SECURE BOOT-
>>> define CONFIG_SYS_RAMBOOT for NAND boot
>>>
>>> On 03/07/2014 10:57 AM, Scott Wood wrote:
>>>> On Thu, 2014-03-06 at 03:24 -0600, Bansal Aneesh-B39320 wrote:
>>>>>> -----Original Message-----
>>>>>> From: Wood Scott-B07421
>>>>>> Sent: Wednesday, March 05, 2014 11:30 PM
>>>>>> To: Bansal Aneesh-B39320
>>>>>> Cc: Sun York-R58495; Wolfgang Denk; u-boot@lists.denx.de; Gupta
>>>>>> Ruchika-R66431
>>>>>> Subject: Re: [U-Boot] [PATCH 1/3] powerpc/p1010rdb: SECURE BOOT-
>>>>>> define CONFIG_SYS_RAMBOOT for NAND boot
>>>>>>
>>>>>> On Tue, 2014-03-04 at 23:30 -0600, Bansal Aneesh-B39320 wrote:
>>>>>>> Yes, in case of secure boot from NAND, the DRAM is initialized by
>>>>>> the
>>>>>>> BootROM and complete u-boot image is copied from NAND to DRAM by
>>>>>>> the
>>>>>> BootROM.
>>>>>>> So, it should be called RAMBOOT.
>>>>>>
>>>>>> DRAM or SRAM?  Having ROM initialize DDR is a bit scary.
>>>>>>
>>>>>> -Scott
>>>>>>
>>>>> It can be either DDR or SRAM. It is not hardcoded in BootROM to
>>> initialize DDR.
>>>>> This depends on the config words (CF_WORDS) in the CF_HEADER.
>>>>> The Boot ROM code parses the config words and programs the
>>> addresses
>>>>> with data values accordingly. The user may opt to initialize DDR
>>> and
>>>>> get the image copied onto DDR or configure CPC as SRAM and get the
>>> Image copied onto SRAM.
>>>>> On 1010,
>>>>
>>>> P1010?  LS1010?  Something else?
>>>>
>>>>> the CPC size is not big enough to accommodate the U-boot image.
>>>>> So, currently the CF_WORDS are for DDR to be initialized and copy
>>> the image on DDR.
>>>>
>>>> Generally if SPD is present, it should be used to init DDR rather
>>> than
>>>> using hardcoded values.  If U-Boot doesn't fit in SRAM, you can use
>>>> SPL instead of hardcoded init.
>>>
>>> I agree with Scott on this point. Using hardcoded values totally skip
>>> DDR driver. You don't only lose the flexibility of various speeds, you
>>> also skip all workarounds implemented in DDR driver.
>>>
>>> York
>>
>> Currently we are following the same approach which was there for SPI
>> and SD on P1010 i.e hardcoded initialization of DDR in platforms like
>> P1010, 9131/9132 using config words.
> 
> I don't know about 9131/9132, but on P1010RDB we now use SPL to init DDR
> when booting from SPI/SD.
> 
> In any case, "we've always done it this way" isn't really a good reason.
> 

We should move to SPL for SD/SPI/NAND. Let's take step by step. I intend to
accept the updated patch to get NAND SECURE boot working for this board. Aneesh
will submit patches to move to SPL, for SD/SPI/NAND.

York
diff mbox

Patch

diff --git a/include/configs/P1010RDB.h b/include/configs/P1010RDB.h
index ea5cb65..c21cf07 100644
--- a/include/configs/P1010RDB.h
+++ b/include/configs/P1010RDB.h
@@ -446,7 +446,8 @@  extern unsigned long get_sdram_size(void);
 					FTIM2_GPCM_TWP(0x1f))
 #define CONFIG_SYS_CS3_FTIM3		0x0
 
-#if defined(CONFIG_RAMBOOT_SDCARD) || defined(CONFIG_RAMBOOT_SPIFLASH)
+#if defined(CONFIG_RAMBOOT_SDCARD) || defined(CONFIG_RAMBOOT_SPIFLASH) || \
+	defined(CONFIG_RAMBOOT_NAND)
 #define CONFIG_SYS_RAMBOOT
 #define CONFIG_SYS_EXTRA_ENV_RELOC
 #else