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