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

[York Sun]

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