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

[Scott Wood <scottwood@freescale.com>]

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