diff mbox

[U-Boot,RFC,v1,1/2] armv8: fsl-layerscape: Reserve memory for PPA

Message ID 1447183031-8043-2-git-send-email-yorksun@freescale.com
State Superseded
Headers show

Commit Message

York Sun Nov. 10, 2015, 7:17 p.m. UTC
Primary Protected Application (PPA) is the base of TrustZone for
Freescale Layerscape SoCs. It needs to run in secure memory while
the rest of u-boot can run in non-secure memory. The secure memory
is reserved at the very end of DDR, before debug server and MC
reservations. The address varies depending on the total size of
intalled DDR.

Signed-off-by: York Sun <yorksun@freescale.com>

---

Changes in v1:
  Initial patch.
  Depends on http://patchwork.ozlabs.org/patch/540248/

 README                                            |   14 ++++++++++---
 arch/arm/cpu/armv8/fsl-layerscape/cpu.c           |   23 +++++++++++++++++++++
 arch/arm/include/asm/arch-fsl-layerscape/config.h |    3 +++
 board/freescale/ls2085a/ls2085a.c                 |   17 ---------------
 board/freescale/ls2085aqds/ls2085aqds.c           |   17 ---------------
 board/freescale/ls2085ardb/ls2085ardb.c           |   17 ---------------
 include/configs/ls2085a_common.h                  |    4 ++--
 7 files changed, 39 insertions(+), 56 deletions(-)

Comments

Scott Wood Nov. 10, 2015, 7:31 p.m. UTC | #1
On Tue, 2015-11-10 at 11:17 -0800, York Sun wrote:
> Primary Protected Application (PPA) is the base of TrustZone for
> Freescale Layerscape SoCs. It needs to run in secure memory while
> the rest of u-boot can run in non-secure memory. The secure memory
> is reserved at the very end of DDR, before debug server and MC
> reservations. The address varies depending on the total size of
> intalled DDR.
> 
> Signed-off-by: York Sun <yorksun@freescale.com>
> 
> ---
> 
> Changes in v1:
>   Initial patch.
>   Depends on http://patchwork.ozlabs.org/patch/540248/
> 
>  README                                            |   14 ++++++++++---
>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c           |   23
> +++++++++++++++++++++
>  arch/arm/include/asm/arch-fsl-layerscape/config.h |    3 +++
>  board/freescale/ls2085a/ls2085a.c                 |   17 ---------------
>  board/freescale/ls2085aqds/ls2085aqds.c           |   17 ---------------
>  board/freescale/ls2085ardb/ls2085ardb.c           |   17 ---------------
>  include/configs/ls2085a_common.h                  |    4 ++--
>  7 files changed, 39 insertions(+), 56 deletions(-)
> 
> diff --git a/README b/README
> index ef8d437..e1ca7c2 100644
> --- a/README
> +++ b/README
> @@ -3881,7 +3881,7 @@ Configuration Settings:
>  		Scratch address used by the alternate memory test
>  		You only need to set this if address zero isn't writeable
>  
> -- CONFIG_SYS_MEM_TOP_HIDE (PPC only):
> +- CONFIG_SYS_MEM_TOP_HIDE:
>  		If CONFIG_SYS_MEM_TOP_HIDE is defined in the board config
> header,
>  		this specified memory area will get subtracted from the top
>  		(end) of RAM and won't get "touched" at all by U-Boot. By
> @@ -5048,6 +5048,10 @@ within that device.
>  	normal addressable memory via the LBC. CONFIG_SYS_LS_MC_FW_ADDR is
> the
>  	virtual address in NOR flash.
>  
> +- CONFIG_SYS_MEM_TOP_HIDE_MIN
> +	Define minimum DDR size to be hided from top of the DDR memory.

Defines the minimum DDR size to be hidden from the top of DDR memory.

How is this different from CONFIG_SYS_MEM_TOP_HIDE?

> +	MC requires the region to be aligned with 512MB.

Why is this generically named symbol documented in the Layerscape section,
with a reference to MC?

Why is a new symbol being added here rather than Kconfig?

Why does this talk about MC alignment as if this entire region were for MC?

> +
>  Freescale Layerscape Debug Server Support:
>  -------------------------------------------
>  The Freescale Layerscape Debug Server Support supports the loading of
> @@ -5060,8 +5064,12 @@ This firmware often needs to be loaded during U-Boot
> booting.
>  - CONFIG_SYS_DEBUG_SERVER_DRAM_BLOCK_MIN_SIZE
>  	Define minimum DDR size required for debug server image
>  
> -- CONFIG_SYS_MEM_TOP_HIDE_MIN
> -	Define minimum DDR size to be hided from top of the DDR memory
> +Freescale Layerscape Primary Protected Application (PPA) support
> +----------------------------------------------------------------
> +Freescale PPA runs in secure DDR, reserved from DDR pool.
> +
> +- CONFIG_FSL_PPA_RESERVED_DRAM_SIZE
> +	If defined, this is reserved in highest address as secure memory

What is Freescale-specific about the concept of reserving memory for a secure
monitor?


> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/config.h
> b/arch/arm/include/asm/arch-fsl-layerscape/config.h
> index 87bb937..77637a9 100644
> --- a/arch/arm/include/asm/arch-fsl-layerscape/config.h
> +++ b/arch/arm/include/asm/arch-fsl-layerscape/config.h
> @@ -17,6 +17,9 @@
>  #define CONFIG_SYS_FSL_DDR		/* Freescale DDR driver */
>  #define CONFIG_SYS_FSL_DDR_VER		FSL_DDR_VER_5_0
>  
> +/* High memory for PPA, MC, debug server when applicable */
> +#define CONFIG_SYS_MEM_TOP_HIDE		get_dram_size_to_hide()

Hiding code in config symbols is not friendly to kconfig conversion (and it's
hard to read besides).

-Scott
York Sun Nov. 10, 2015, 7:44 p.m. UTC | #2
On 11/10/2015 11:31 AM, Scott Wood wrote:
> On Tue, 2015-11-10 at 11:17 -0800, York Sun wrote:
>> Primary Protected Application (PPA) is the base of TrustZone for
>> Freescale Layerscape SoCs. It needs to run in secure memory while
>> the rest of u-boot can run in non-secure memory. The secure memory
>> is reserved at the very end of DDR, before debug server and MC
>> reservations. The address varies depending on the total size of
>> intalled DDR.
>>
>> Signed-off-by: York Sun <yorksun@freescale.com>
>>
>> ---
>>
>> Changes in v1:
>>   Initial patch.
>>   Depends on http://patchwork.ozlabs.org/patch/540248/
>>
>>  README                                            |   14 ++++++++++---
>>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c           |   23
>> +++++++++++++++++++++
>>  arch/arm/include/asm/arch-fsl-layerscape/config.h |    3 +++
>>  board/freescale/ls2085a/ls2085a.c                 |   17 ---------------
>>  board/freescale/ls2085aqds/ls2085aqds.c           |   17 ---------------
>>  board/freescale/ls2085ardb/ls2085ardb.c           |   17 ---------------
>>  include/configs/ls2085a_common.h                  |    4 ++--
>>  7 files changed, 39 insertions(+), 56 deletions(-)
>>
>> diff --git a/README b/README
>> index ef8d437..e1ca7c2 100644
>> --- a/README
>> +++ b/README
>> @@ -3881,7 +3881,7 @@ Configuration Settings:
>>  		Scratch address used by the alternate memory test
>>  		You only need to set this if address zero isn't writeable
>>  
>> -- CONFIG_SYS_MEM_TOP_HIDE (PPC only):
>> +- CONFIG_SYS_MEM_TOP_HIDE:
>>  		If CONFIG_SYS_MEM_TOP_HIDE is defined in the board config
>> header,
>>  		this specified memory area will get subtracted from the top
>>  		(end) of RAM and won't get "touched" at all by U-Boot. By
>> @@ -5048,6 +5048,10 @@ within that device.
>>  	normal addressable memory via the LBC. CONFIG_SYS_LS_MC_FW_ADDR is
>> the
>>  	virtual address in NOR flash.
>>  
>> +- CONFIG_SYS_MEM_TOP_HIDE_MIN
>> +	Define minimum DDR size to be hided from top of the DDR memory.
> 
> Defines the minimum DDR size to be hidden from the top of DDR memory.
> 
> How is this different from CONFIG_SYS_MEM_TOP_HIDE?

This is used by MC to make sure it is aligned.

> 
>> +	MC requires the region to be aligned with 512MB.
> 
> Why is this generically named symbol documented in the Layerscape section,
> with a reference to MC?
> 
> Why is a new symbol being added here rather than Kconfig?
> 
> Why does this talk about MC alignment as if this entire region were for MC?

I moved this section out of MC alignment. Maybe I should keep it there.

> 
>> +
>>  Freescale Layerscape Debug Server Support:
>>  -------------------------------------------
>>  The Freescale Layerscape Debug Server Support supports the loading of
>> @@ -5060,8 +5064,12 @@ This firmware often needs to be loaded during U-Boot
>> booting.
>>  - CONFIG_SYS_DEBUG_SERVER_DRAM_BLOCK_MIN_SIZE
>>  	Define minimum DDR size required for debug server image
>>  
>> -- CONFIG_SYS_MEM_TOP_HIDE_MIN
>> -	Define minimum DDR size to be hided from top of the DDR memory
>> +Freescale Layerscape Primary Protected Application (PPA) support
>> +----------------------------------------------------------------
>> +Freescale PPA runs in secure DDR, reserved from DDR pool.
>> +
>> +- CONFIG_FSL_PPA_RESERVED_DRAM_SIZE
>> +	If defined, this is reserved in highest address as secure memory
> 
> What is Freescale-specific about the concept of reserving memory for a secure
> monitor?

The PPA is a Freescale implementation of TrustZone application.

> 
> 
>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/config.h
>> b/arch/arm/include/asm/arch-fsl-layerscape/config.h
>> index 87bb937..77637a9 100644
>> --- a/arch/arm/include/asm/arch-fsl-layerscape/config.h
>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/config.h
>> @@ -17,6 +17,9 @@
>>  #define CONFIG_SYS_FSL_DDR		/* Freescale DDR driver */
>>  #define CONFIG_SYS_FSL_DDR_VER		FSL_DDR_VER_5_0
>>  
>> +/* High memory for PPA, MC, debug server when applicable */
>> +#define CONFIG_SYS_MEM_TOP_HIDE		get_dram_size_to_hide()
> 
> Hiding code in config symbols is not friendly to kconfig conversion (and it's
> hard to read besides).
> 

It was made complicated by MC and debug server. Maybe we can fix the size for
them. They are fixed anyway for now.

York
Scott Wood Nov. 10, 2015, 7:51 p.m. UTC | #3
On Tue, 2015-11-10 at 11:44 -0800, York Sun wrote:
> 
> On 11/10/2015 11:31 AM, Scott Wood wrote:
> > On Tue, 2015-11-10 at 11:17 -0800, York Sun wrote:
> > > Primary Protected Application (PPA) is the base of TrustZone for
> > > Freescale Layerscape SoCs. It needs to run in secure memory while
> > > the rest of u-boot can run in non-secure memory. The secure memory
> > > is reserved at the very end of DDR, before debug server and MC
> > > reservations. The address varies depending on the total size of
> > > intalled DDR.
> > > 
> > > Signed-off-by: York Sun <yorksun@freescale.com>
> > > 
> > > ---
> > > 
> > > Changes in v1:
> > >   Initial patch.
> > >   Depends on http://patchwork.ozlabs.org/patch/540248/
> > > 
> > >  README                                            |   14 ++++++++++---
> > >  arch/arm/cpu/armv8/fsl-layerscape/cpu.c           |   23
> > > +++++++++++++++++++++
> > >  arch/arm/include/asm/arch-fsl-layerscape/config.h |    3 +++
> > >  board/freescale/ls2085a/ls2085a.c                 |   17 --------------
> > > -
> > >  board/freescale/ls2085aqds/ls2085aqds.c           |   17 --------------
> > > -
> > >  board/freescale/ls2085ardb/ls2085ardb.c           |   17 --------------
> > > -
> > >  include/configs/ls2085a_common.h                  |    4 ++--
> > >  7 files changed, 39 insertions(+), 56 deletions(-)
> > > 
> > > diff --git a/README b/README
> > > index ef8d437..e1ca7c2 100644
> > > --- a/README
> > > +++ b/README
> > > @@ -3881,7 +3881,7 @@ Configuration Settings:
> > >  		Scratch address used by the alternate memory test
> > >  		You only need to set this if address zero isn't
> > > writeable
> > >  
> > > -- CONFIG_SYS_MEM_TOP_HIDE (PPC only):
> > > +- CONFIG_SYS_MEM_TOP_HIDE:
> > >  		If CONFIG_SYS_MEM_TOP_HIDE is defined in the board
> > > config
> > > header,
> > >  		this specified memory area will get subtracted from the
> > > top
> > >  		(end) of RAM and won't get "touched" at all by U-Boot.
> > > By
> > > @@ -5048,6 +5048,10 @@ within that device.
> > >  	normal addressable memory via the LBC. CONFIG_SYS_LS_MC_FW_ADDR
> > > is
> > > the
> > >  	virtual address in NOR flash.
> > >  
> > > +- CONFIG_SYS_MEM_TOP_HIDE_MIN
> > > +	Define minimum DDR size to be hided from top of the DDR memory.
> > 
> > Defines the minimum DDR size to be hidden from the top of DDR memory.
> > 
> > How is this different from CONFIG_SYS_MEM_TOP_HIDE?
> 
> This is used by MC to make sure it is aligned.

So it's alignment, not minimum.  But the user could just adhere to the
alignment when setting CONFIG_SYS_MEM_TOP_HIDE -- there's nothing here about
certain targets replacing that with a calculation.

> 
> > 
> > > +	MC requires the region to be aligned with 512MB.
> > 
> > Why is this generically named symbol documented in the Layerscape section,
> > with a reference to MC?
> > 
> > Why is a new symbol being added here rather than Kconfig?
> > 
> > Why does this talk about MC alignment as if this entire region were for
> > MC?
> 
> I moved this section out of MC alignment. Maybe I should keep it there.

That doesn't answer the questions...

> 
> > 
> > > +
> > >  Freescale Layerscape Debug Server Support:
> > >  -------------------------------------------
> > >  The Freescale Layerscape Debug Server Support supports the loading of
> > > @@ -5060,8 +5064,12 @@ This firmware often needs to be loaded during U
> > > -Boot
> > > booting.
> > >  - CONFIG_SYS_DEBUG_SERVER_DRAM_BLOCK_MIN_SIZE
> > >  	Define minimum DDR size required for debug server image
> > >  
> > > -- CONFIG_SYS_MEM_TOP_HIDE_MIN
> > > -	Define minimum DDR size to be hided from top of the DDR memory
> > > +Freescale Layerscape Primary Protected Application (PPA) support
> > > +----------------------------------------------------------------
> > > +Freescale PPA runs in secure DDR, reserved from DDR pool.
> > > +
> > > +- CONFIG_FSL_PPA_RESERVED_DRAM_SIZE
> > > +	If defined, this is reserved in highest address as secure
> > > memory
> > 
> > What is Freescale-specific about the concept of reserving memory for a
> > secure
> > monitor?
> 
> The PPA is a Freescale implementation of TrustZone application.

So?  How much of it is inherently specific to Freescale hardware?  PSCI on
armv7 has some common code -- why shouldn't it on armv8?

In any case, I was asking about the infrastructure for reserving memory for
such purposes, rather than the code that uses it.

> > > diff --git a/arch/arm/include/asm/arch-fsl-layerscape/config.h
> > > b/arch/arm/include/asm/arch-fsl-layerscape/config.h
> > > index 87bb937..77637a9 100644
> > > --- a/arch/arm/include/asm/arch-fsl-layerscape/config.h
> > > +++ b/arch/arm/include/asm/arch-fsl-layerscape/config.h
> > > @@ -17,6 +17,9 @@
> > >  #define CONFIG_SYS_FSL_DDR		/* Freescale DDR driver */
> > >  #define CONFIG_SYS_FSL_DDR_VER		FSL_DDR_VER_5_0
> > >  
> > > +/* High memory for PPA, MC, debug server when applicable */
> > > +#define CONFIG_SYS_MEM_TOP_HIDE		get_dram_size_to_hide()
> > 
> > Hiding code in config symbols is not friendly to kconfig conversion (and
> > it's
> > hard to read besides).
> > 
> 
> It was made complicated by MC and debug server. Maybe we can fix the size
> for
> them. They are fixed anyway for now.

I'm not saying that the size to hide shouldn't be calculated -- I'm suggesting
that it would be better to do it more generically and transparently.

-Scott
Wolfgang Denk Nov. 10, 2015, 7:52 p.m. UTC | #4
Dear York,

In message <5642490C.9090407@freescale.com> you wrote:
> 
> >> +- CONFIG_FSL_PPA_RESERVED_DRAM_SIZE
> >> +	If defined, this is reserved in highest address as secure memory
> > 
> > What is Freescale-specific about the concept of reserving memory for a secure
> > monitor?
> 
> The PPA is a Freescale implementation of TrustZone application.

That does not answer the question.  What is so special here that you
cannot use the existing CONFIG_PRAM ?

Best regards,

Wolfgang Denk
York Sun Nov. 10, 2015, 8:11 p.m. UTC | #5
On 11/10/2015 11:52 AM, Wolfgang Denk wrote:
> Dear York,
> 
> In message <5642490C.9090407@freescale.com> you wrote:
>>
>>>> +- CONFIG_FSL_PPA_RESERVED_DRAM_SIZE
>>>> +	If defined, this is reserved in highest address as secure memory
>>>
>>> What is Freescale-specific about the concept of reserving memory for a secure
>>> monitor?
>>
>> The PPA is a Freescale implementation of TrustZone application.
> 
> That does not answer the question.  What is so special here that you
> cannot use the existing CONFIG_PRAM ?
> 

CONFIG_PRAM is reserved from the top of u-boot memory, which is not necessarily
the real top of memory.

I am putting this reserved memory under the name
CONFIG_FSL_PPA_RESERVED_DRAM_SIZE for now, because I don't know if this will
serve other SoCs. I would use a generic solution if I don't have MC and debug
server in my way. I am open to suggestions.

York
York Sun Nov. 10, 2015, 8:24 p.m. UTC | #6
On 11/10/2015 11:51 AM, Scott Wood wrote:
> On Tue, 2015-11-10 at 11:44 -0800, York Sun wrote:
>>
>> On 11/10/2015 11:31 AM, Scott Wood wrote:
>>> On Tue, 2015-11-10 at 11:17 -0800, York Sun wrote:
>>>> Primary Protected Application (PPA) is the base of TrustZone for
>>>> Freescale Layerscape SoCs. It needs to run in secure memory while
>>>> the rest of u-boot can run in non-secure memory. The secure memory
>>>> is reserved at the very end of DDR, before debug server and MC
>>>> reservations. The address varies depending on the total size of
>>>> intalled DDR.
>>>>
>>>> Signed-off-by: York Sun <yorksun@freescale.com>
>>>>
>>>> ---
>>>>
>>>> Changes in v1:
>>>>   Initial patch.
>>>>   Depends on http://patchwork.ozlabs.org/patch/540248/
>>>>
>>>>  README                                            |   14 ++++++++++---
>>>>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c           |   23
>>>> +++++++++++++++++++++
>>>>  arch/arm/include/asm/arch-fsl-layerscape/config.h |    3 +++
>>>>  board/freescale/ls2085a/ls2085a.c                 |   17 --------------
>>>> -
>>>>  board/freescale/ls2085aqds/ls2085aqds.c           |   17 --------------
>>>> -
>>>>  board/freescale/ls2085ardb/ls2085ardb.c           |   17 --------------
>>>> -
>>>>  include/configs/ls2085a_common.h                  |    4 ++--
>>>>  7 files changed, 39 insertions(+), 56 deletions(-)
>>>>
>>>> diff --git a/README b/README
>>>> index ef8d437..e1ca7c2 100644
>>>> --- a/README
>>>> +++ b/README
>>>> @@ -3881,7 +3881,7 @@ Configuration Settings:
>>>>  		Scratch address used by the alternate memory test
>>>>  		You only need to set this if address zero isn't
>>>> writeable
>>>>  
>>>> -- CONFIG_SYS_MEM_TOP_HIDE (PPC only):
>>>> +- CONFIG_SYS_MEM_TOP_HIDE:
>>>>  		If CONFIG_SYS_MEM_TOP_HIDE is defined in the board
>>>> config
>>>> header,
>>>>  		this specified memory area will get subtracted from the
>>>> top
>>>>  		(end) of RAM and won't get "touched" at all by U-Boot.
>>>> By
>>>> @@ -5048,6 +5048,10 @@ within that device.
>>>>  	normal addressable memory via the LBC. CONFIG_SYS_LS_MC_FW_ADDR
>>>> is
>>>> the
>>>>  	virtual address in NOR flash.
>>>>  
>>>> +- CONFIG_SYS_MEM_TOP_HIDE_MIN
>>>> +	Define minimum DDR size to be hided from top of the DDR memory.
>>>
>>> Defines the minimum DDR size to be hidden from the top of DDR memory.
>>>
>>> How is this different from CONFIG_SYS_MEM_TOP_HIDE?
>>
>> This is used by MC to make sure it is aligned.
> 
> So it's alignment, not minimum.  But the user could just adhere to the
> alignment when setting CONFIG_SYS_MEM_TOP_HIDE -- there's nothing here about
> certain targets replacing that with a calculation.

It is alignment requirement from MC. Maybe we should rename it.

> 
>>
>>>
>>>> +	MC requires the region to be aligned with 512MB.
>>>
>>> Why is this generically named symbol documented in the Layerscape section,
>>> with a reference to MC?
>>>
>>> Why is a new symbol being added here rather than Kconfig?
>>>
>>> Why does this talk about MC alignment as if this entire region were for
>>> MC?
>>
>> I moved this section out of MC alignment. Maybe I should keep it there.
> 
> That doesn't answer the questions...

Because this macro was introduced while Kconfig wasn't finalized?
This section talks about MC because it was in MC section and I shouldn't have
moved it out?

> 
>>
>>>
>>>> +
>>>>  Freescale Layerscape Debug Server Support:
>>>>  -------------------------------------------
>>>>  The Freescale Layerscape Debug Server Support supports the loading of
>>>> @@ -5060,8 +5064,12 @@ This firmware often needs to be loaded during U
>>>> -Boot
>>>> booting.
>>>>  - CONFIG_SYS_DEBUG_SERVER_DRAM_BLOCK_MIN_SIZE
>>>>  	Define minimum DDR size required for debug server image
>>>>  
>>>> -- CONFIG_SYS_MEM_TOP_HIDE_MIN
>>>> -	Define minimum DDR size to be hided from top of the DDR memory
>>>> +Freescale Layerscape Primary Protected Application (PPA) support
>>>> +----------------------------------------------------------------
>>>> +Freescale PPA runs in secure DDR, reserved from DDR pool.
>>>> +
>>>> +- CONFIG_FSL_PPA_RESERVED_DRAM_SIZE
>>>> +	If defined, this is reserved in highest address as secure
>>>> memory
>>>
>>> What is Freescale-specific about the concept of reserving memory for a
>>> secure
>>> monitor?
>>
>> The PPA is a Freescale implementation of TrustZone application.
> 
> So?  How much of it is inherently specific to Freescale hardware?  PSCI on
> armv7 has some common code -- why shouldn't it on armv8?
> 
> In any case, I was asking about the infrastructure for reserving memory for
> such purposes, rather than the code that uses it.

I see your point. There is no secure vs non-secure implementation in u-boot. I
don't think u-boot should care much about secure memory. But u-boot needs to
setup the secure memory for TrustZone to use. It is not clear how this will be
done. At this moment, Freescale PPA will be loaded by u-boot to secure memory. I
doubt this process will be adopted by other SoCs. At this moment, I don't see it
as generic.

> 
>>>> diff --git a/arch/arm/include/asm/arch-fsl-layerscape/config.h
>>>> b/arch/arm/include/asm/arch-fsl-layerscape/config.h
>>>> index 87bb937..77637a9 100644
>>>> --- a/arch/arm/include/asm/arch-fsl-layerscape/config.h
>>>> +++ b/arch/arm/include/asm/arch-fsl-layerscape/config.h
>>>> @@ -17,6 +17,9 @@
>>>>  #define CONFIG_SYS_FSL_DDR		/* Freescale DDR driver */
>>>>  #define CONFIG_SYS_FSL_DDR_VER		FSL_DDR_VER_5_0
>>>>  
>>>> +/* High memory for PPA, MC, debug server when applicable */
>>>> +#define CONFIG_SYS_MEM_TOP_HIDE		get_dram_size_to_hide()
>>>
>>> Hiding code in config symbols is not friendly to kconfig conversion (and
>>> it's
>>> hard to read besides).
>>>
>>
>> It was made complicated by MC and debug server. Maybe we can fix the size
>> for
>> them. They are fixed anyway for now.
> 
> I'm not saying that the size to hide shouldn't be calculated -- I'm suggesting
> that it would be better to do it more generically and transparently.
> 

Agree.
We can add a new global data variable to deal with this generically.

York
Scott Wood Nov. 11, 2015, 2:08 a.m. UTC | #7
On Tue, 2015-11-10 at 12:24 -0800, York Sun wrote:
> 
> On 11/10/2015 11:51 AM, Scott Wood wrote:
> > On Tue, 2015-11-10 at 11:44 -0800, York Sun wrote:
> > > 
> > > On 11/10/2015 11:31 AM, Scott Wood wrote:
> > > > On Tue, 2015-11-10 at 11:17 -0800, York Sun wrote:
> > > > > Primary Protected Application (PPA) is the base of TrustZone for
> > > > > Freescale Layerscape SoCs. It needs to run in secure memory while
> > > > > the rest of u-boot can run in non-secure memory. The secure memory
> > > > > is reserved at the very end of DDR, before debug server and MC
> > > > > reservations. The address varies depending on the total size of
> > > > > intalled DDR.
> > > > > 
> > > > > Signed-off-by: York Sun <yorksun@freescale.com>
> > > > > 
> > > > > ---
> > > > > 
> > > > > Changes in v1:
> > > > >   Initial patch.
> > > > >   Depends on http://patchwork.ozlabs.org/patch/540248/
> > > > > 
> > > > >  README                                            |   14 ++++++++++
> > > > > ---
> > > > >  arch/arm/cpu/armv8/fsl-layerscape/cpu.c           |   23
> > > > > +++++++++++++++++++++
> > > > >  arch/arm/include/asm/arch-fsl-layerscape/config.h |    3 +++
> > > > >  board/freescale/ls2085a/ls2085a.c                 |   17 ----------
> > > > > ----
> > > > > -
> > > > >  board/freescale/ls2085aqds/ls2085aqds.c           |   17 ----------
> > > > > ----
> > > > > -
> > > > >  board/freescale/ls2085ardb/ls2085ardb.c           |   17 ----------
> > > > > ----
> > > > > -
> > > > >  include/configs/ls2085a_common.h                  |    4 ++--
> > > > >  7 files changed, 39 insertions(+), 56 deletions(-)
> > > > > 
> > > > > diff --git a/README b/README
> > > > > index ef8d437..e1ca7c2 100644
> > > > > --- a/README
> > > > > +++ b/README
> > > > > @@ -3881,7 +3881,7 @@ Configuration Settings:
> > > > >  		Scratch address used by the alternate memory test
> > > > >  		You only need to set this if address zero isn't
> > > > > writeable
> > > > >  
> > > > > -- CONFIG_SYS_MEM_TOP_HIDE (PPC only):
> > > > > +- CONFIG_SYS_MEM_TOP_HIDE:
> > > > >  		If CONFIG_SYS_MEM_TOP_HIDE is defined in the board
> > > > > config
> > > > > header,
> > > > >  		this specified memory area will get subtracted from
> > > > > the
> > > > > top
> > > > >  		(end) of RAM and won't get "touched" at all by U
> > > > > -Boot.
> > > > > By
> > > > > @@ -5048,6 +5048,10 @@ within that device.
> > > > >  	normal addressable memory via the LBC.
> > > > > CONFIG_SYS_LS_MC_FW_ADDR
> > > > > is
> > > > > the
> > > > >  	virtual address in NOR flash.
> > > > >  
> > > > > +- CONFIG_SYS_MEM_TOP_HIDE_MIN
> > > > > +	Define minimum DDR size to be hided from top of the DDR
> > > > > memory.
> > > > 
> > > > Defines the minimum DDR size to be hidden from the top of DDR memory.
> > > > 
> > > > How is this different from CONFIG_SYS_MEM_TOP_HIDE?
> > > 
> > > This is used by MC to make sure it is aligned.
> > 
> > So it's alignment, not minimum.  But the user could just adhere to the
> > alignment when setting CONFIG_SYS_MEM_TOP_HIDE -- there's nothing here
> > about
> > certain targets replacing that with a calculation.
> 
> It is alignment requirement from MC. Maybe we should rename it.
> 
> > 
> > > 
> > > > 
> > > > > +	MC requires the region to be aligned with 512MB.
> > > > 
> > > > Why is this generically named symbol documented in the Layerscape
> > > > section,
> > > > with a reference to MC?
> > > > 
> > > > Why is a new symbol being added here rather than Kconfig?
> > > > 
> > > > Why does this talk about MC alignment as if this entire region were
> > > > for
> > > > MC?
> > > 
> > > I moved this section out of MC alignment. Maybe I should keep it there.
> > 
> > That doesn't answer the questions...
> 
> Because this macro was introduced while Kconfig wasn't finalized?
> This section talks about MC because it was in MC section and I shouldn't
> have
> moved it out?
> 
> > 
> > > 
> > > > 
> > > > > +
> > > > >  Freescale Layerscape Debug Server Support:
> > > > >  -------------------------------------------
> > > > >  The Freescale Layerscape Debug Server Support supports the loading
> > > > > of
> > > > > @@ -5060,8 +5064,12 @@ This firmware often needs to be loaded during
> > > > > U
> > > > > -Boot
> > > > > booting.
> > > > >  - CONFIG_SYS_DEBUG_SERVER_DRAM_BLOCK_MIN_SIZE
> > > > >  	Define minimum DDR size required for debug server image
> > > > >  
> > > > > -- CONFIG_SYS_MEM_TOP_HIDE_MIN
> > > > > -	Define minimum DDR size to be hided from top of the DDR
> > > > > memory
> > > > > +Freescale Layerscape Primary Protected Application (PPA) support
> > > > > +----------------------------------------------------------------
> > > > > +Freescale PPA runs in secure DDR, reserved from DDR pool.
> > > > > +
> > > > > +- CONFIG_FSL_PPA_RESERVED_DRAM_SIZE
> > > > > +	If defined, this is reserved in highest address as secure
> > > > > memory
> > > > 
> > > > What is Freescale-specific about the concept of reserving memory for a
> > > > secure
> > > > monitor?
> > > 
> > > The PPA is a Freescale implementation of TrustZone application.
> > 
> > So?  How much of it is inherently specific to Freescale hardware?  PSCI on
> > armv7 has some common code -- why shouldn't it on armv8?
> > 
> > In any case, I was asking about the infrastructure for reserving memory
> > for
> > such purposes, rather than the code that uses it.
> 
> I see your point. There is no secure vs non-secure implementation in u-boot.
> I
> don't think u-boot should care much about secure memory. But u-boot needs to
> setup the secure memory for TrustZone to use. It is not clear how this will
> be
> done. At this moment, Freescale PPA will be loaded by u-boot to secure
> memory. I
> doubt this process will be adopted by other SoCs. At this moment, I don't
> see it
> as generic.

U-Boot already has a PSCI implementation for armv7.  It would be nice if U
-Boot had something similar for armv8, without needing this external blob just
for PSCI -- but in any case, when an external secure monitor is used, the
mechanism for loading and reserving it should not be Freescale-specific.

-Scott
diff mbox

Patch

diff --git a/README b/README
index ef8d437..e1ca7c2 100644
--- a/README
+++ b/README
@@ -3881,7 +3881,7 @@  Configuration Settings:
 		Scratch address used by the alternate memory test
 		You only need to set this if address zero isn't writeable
 
-- CONFIG_SYS_MEM_TOP_HIDE (PPC only):
+- CONFIG_SYS_MEM_TOP_HIDE:
 		If CONFIG_SYS_MEM_TOP_HIDE is defined in the board config header,
 		this specified memory area will get subtracted from the top
 		(end) of RAM and won't get "touched" at all by U-Boot. By
@@ -5048,6 +5048,10 @@  within that device.
 	normal addressable memory via the LBC. CONFIG_SYS_LS_MC_FW_ADDR is the
 	virtual address in NOR flash.
 
+- CONFIG_SYS_MEM_TOP_HIDE_MIN
+	Define minimum DDR size to be hided from top of the DDR memory.
+	MC requires the region to be aligned with 512MB.
+
 Freescale Layerscape Debug Server Support:
 -------------------------------------------
 The Freescale Layerscape Debug Server Support supports the loading of
@@ -5060,8 +5064,12 @@  This firmware often needs to be loaded during U-Boot booting.
 - CONFIG_SYS_DEBUG_SERVER_DRAM_BLOCK_MIN_SIZE
 	Define minimum DDR size required for debug server image
 
-- CONFIG_SYS_MEM_TOP_HIDE_MIN
-	Define minimum DDR size to be hided from top of the DDR memory
+Freescale Layerscape Primary Protected Application (PPA) support
+----------------------------------------------------------------
+Freescale PPA runs in secure DDR, reserved from DDR pool.
+
+- CONFIG_FSL_PPA_RESERVED_DRAM_SIZE
+	If defined, this is reserved in highest address as secure memory
 
 Reproducible builds
 -------------------
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
index 9d1c70f..690c263 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
@@ -537,3 +537,26 @@  void reset_cpu(ulong addr)
 	val |= 0x02;
 	scfg_out32(rstcr, val);
 }
+
+#ifndef CONFIG_FSL_PPA_RESERVED_DRAM_SIZE
+#define CONFIG_FSL_PPA_RESERVED_DRAM_SIZE	0
+#endif
+#ifndef CONFIG_SYS_MEM_TOP_HIDE_MIN
+#define CONFIG_SYS_MEM_TOP_HIDE_MIN	0
+#endif
+unsigned long get_dram_size_to_hide(void)
+{
+	unsigned long dram_to_hide = CONFIG_FSL_PPA_RESERVED_DRAM_SIZE;
+
+/* Carve the Debug Server private DRAM block from the end of DRAM */
+#ifdef CONFIG_FSL_DEBUG_SERVER
+	dram_to_hide += debug_server_get_dram_block_size();
+#endif
+
+/* Carve the MC private DRAM block from the end of DRAM */
+#ifdef CONFIG_FSL_MC_ENET
+	dram_to_hide += mc_get_dram_block_size();
+#endif
+
+	return roundup(dram_to_hide, CONFIG_SYS_MEM_TOP_HIDE_MIN);
+}
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/config.h b/arch/arm/include/asm/arch-fsl-layerscape/config.h
index 87bb937..77637a9 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/config.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/config.h
@@ -17,6 +17,9 @@ 
 #define CONFIG_SYS_FSL_DDR		/* Freescale DDR driver */
 #define CONFIG_SYS_FSL_DDR_VER		FSL_DDR_VER_5_0
 
+/* High memory for PPA, MC, debug server when applicable */
+#define CONFIG_SYS_MEM_TOP_HIDE		get_dram_size_to_hide()
+
 #if defined(CONFIG_LS2085A)
 #define CONFIG_MAX_CPUS				16
 #define CONFIG_SYS_FSL_IFC_BANK_COUNT		8
diff --git a/board/freescale/ls2085a/ls2085a.c b/board/freescale/ls2085a/ls2085a.c
index 27481e2..6f4c3d4 100644
--- a/board/freescale/ls2085a/ls2085a.c
+++ b/board/freescale/ls2085a/ls2085a.c
@@ -66,23 +66,6 @@  int arch_misc_init(void)
 }
 #endif
 
-unsigned long get_dram_size_to_hide(void)
-{
-	unsigned long dram_to_hide = 0;
-
-/* Carve the Debug Server private DRAM block from the end of DRAM */
-#ifdef CONFIG_FSL_DEBUG_SERVER
-	dram_to_hide += debug_server_get_dram_block_size();
-#endif
-
-/* Carve the MC private DRAM block from the end of DRAM */
-#ifdef CONFIG_FSL_MC_ENET
-	dram_to_hide += mc_get_dram_block_size();
-#endif
-
-	return roundup(dram_to_hide, CONFIG_SYS_MEM_TOP_HIDE_MIN);
-}
-
 int board_eth_init(bd_t *bis)
 {
 	int error = 0;
diff --git a/board/freescale/ls2085aqds/ls2085aqds.c b/board/freescale/ls2085aqds/ls2085aqds.c
index b02d6e8..8898cc3 100644
--- a/board/freescale/ls2085aqds/ls2085aqds.c
+++ b/board/freescale/ls2085aqds/ls2085aqds.c
@@ -251,23 +251,6 @@  int arch_misc_init(void)
 }
 #endif
 
-unsigned long get_dram_size_to_hide(void)
-{
-	unsigned long dram_to_hide = 0;
-
-/* Carve the Debug Server private DRAM block from the end of DRAM */
-#ifdef CONFIG_FSL_DEBUG_SERVER
-	dram_to_hide += debug_server_get_dram_block_size();
-#endif
-
-/* Carve the MC private DRAM block from the end of DRAM */
-#ifdef CONFIG_FSL_MC_ENET
-	dram_to_hide += mc_get_dram_block_size();
-#endif
-
-	return roundup(dram_to_hide, CONFIG_SYS_MEM_TOP_HIDE_MIN);
-}
-
 #ifdef CONFIG_FSL_MC_ENET
 void fdt_fixup_board_enet(void *fdt)
 {
diff --git a/board/freescale/ls2085ardb/ls2085ardb.c b/board/freescale/ls2085ardb/ls2085ardb.c
index 18953b8..efddf74 100644
--- a/board/freescale/ls2085ardb/ls2085ardb.c
+++ b/board/freescale/ls2085ardb/ls2085ardb.c
@@ -217,23 +217,6 @@  int arch_misc_init(void)
 }
 #endif
 
-unsigned long get_dram_size_to_hide(void)
-{
-	unsigned long dram_to_hide = 0;
-
-/* Carve the Debug Server private DRAM block from the end of DRAM */
-#ifdef CONFIG_FSL_DEBUG_SERVER
-	dram_to_hide += debug_server_get_dram_block_size();
-#endif
-
-/* Carve the MC private DRAM block from the end of DRAM */
-#ifdef CONFIG_FSL_MC_ENET
-	dram_to_hide += mc_get_dram_block_size();
-#endif
-
-	return roundup(dram_to_hide, CONFIG_SYS_MEM_TOP_HIDE_MIN);
-}
-
 #ifdef CONFIG_FSL_MC_ENET
 void fdt_fixup_board_enet(void *fdt)
 {
diff --git a/include/configs/ls2085a_common.h b/include/configs/ls2085a_common.h
index 0011e72..1d0d28b 100644
--- a/include/configs/ls2085a_common.h
+++ b/include/configs/ls2085a_common.h
@@ -189,14 +189,14 @@  unsigned long long get_qixis_addr(void);
 /*
  * Carve out a DDR region which will not be used by u-boot/Linux
  *
- * It will be used by MC and Debug Server. The MC region must be
+ * It will be used by PPA, MC and Debug Server. The MC region must be
  * 512MB aligned, so the min size to hide is 512MB.
  */
+#define CONFIG_FSL_PPA_RESERVED_DRAM_SIZE		(2048UL * 1024)
 #if defined(CONFIG_FSL_MC_ENET) || defined(CONFIG_FSL_DEBUG_SERVER)
 #define CONFIG_SYS_DEBUG_SERVER_DRAM_BLOCK_MIN_SIZE	(256UL * 1024 * 1024)
 #define CONFIG_SYS_LS_MC_DRAM_BLOCK_MIN_SIZE		(256UL * 1024 * 1024)
 #define CONFIG_SYS_MEM_TOP_HIDE_MIN			(512UL * 1024 * 1024)
-#define CONFIG_SYS_MEM_TOP_HIDE		get_dram_size_to_hide()
 #endif
 
 /* PCIe */