[U-Boot] [PATCH v2] armv8: fsl-layerscale: Rewrite reserving memory for MC and debug server

[Scott Wood]

On Wed, 2015-11-18 at 14:48 -0800, York Sun wrote:
> 
> On 11/18/2015 02:15 PM, Scott Wood wrote:
> > On Wed, 2015-11-18 at 10:05 -0800, York Sun wrote:
> > > MC and debug server are not board-specific. Instead of reserving
> > > memory in each board file, a weak function is introduced in board_f.c
> > > to replace macro CONFIG_SYS_MEM_TOP_HIDE for more flexibility.
> > > Legacy use of this macro is still supported. Move the reservation
> > > calculation to SoC file. Reduce debug server memory by 2MB to
> > > make room for secure memory.
> > > 
> > > In the system with MC and debug server, the top of u-boot memory
> > > is not the end of memory. PRAM is not used for this reservation.
> > > 
> > > Signed-off-by: York Sun <yorksun at freescale.com>
> > > 
> > > ---
> > > 
> > > Changes in v2:
> > >   Revise commit message.
> > > 
> > >  README                                  |    6 +++---
> > >  arch/arm/cpu/armv8/fsl-layerscape/cpu.c |   18 ++++++++++++++++++
> > >  board/freescale/ls2085a/ls2085a.c       |   17 -----------------
> > >  board/freescale/ls2085aqds/ls2085aqds.c |   17 -----------------
> > >  board/freescale/ls2085ardb/ls2085ardb.c |   17 -----------------
> > >  common/board_f.c                        |   14 +++++++++++---
> > >  include/configs/ls2085a_common.h        |    5 ++---
> > >  7 files changed, 34 insertions(+), 60 deletions(-)
> > > 
> > > diff --git a/README b/README
> > > index 61cbc82..390ee10 100644
> > > --- a/README
> > > +++ b/README
> > > @@ -3889,7 +3889,7 @@ Configuration Settings:
> > >  		the RAM base is not zero, or RAM is divided into banks,
> > >  		this variable needs to be recalcuated to get the
> > > address.
> > >  
> > > -- 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
> > > @@ -5068,8 +5068,8 @@ 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
> > > +- CONFIG_SYS_MC_RESERV_MEM_ALIGN
> > > +	Define alignment of reserved memory MC requires
> > 
> > Can you make this RESERVE, or RSV or RES?  RESERV is hard to look at, like
> > creat(). :-P
> 
> Sure.
> 
> > 
> > 
> > >  Reproducible builds
> > >  -------------------
> > > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > > b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > > index 501feb3..01e8f52 100644
> > > --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > > +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c
> > > @@ -628,3 +628,21 @@ void reset_cpu(ulong addr)
> > >  	val |= 0x02;
> > >  	scfg_out32(rstcr, val);
> > >  }
> > > +
> > > +unsigned long board_reserve_ram_top(unsigned long ram_size)
> > > +{
> > > +	unsigned long ram_top = ram_size;
> > > +
> > > +/* Carve the Debug Server private DRAM block from the end of DRAM */
> > > +#ifdef CONFIG_FSL_DEBUG_SERVER
> > > +	ram_top -= debug_server_get_dram_block_size();
> > > +#endif
> > > +
> > > +/* Carve the MC private DRAM block from the end of DRAM */
> > > +#ifdef CONFIG_FSL_MC_ENET
> > > +	ram_top -= mc_get_dram_block_size();
> > > +	ram_top &= ~(CONFIG_SYS_MC_RESERV_MEM_ALIGN - 1);
> > > +#endif
> > > +
> > > +	return ram_size - ram_top;
> > > +}
> > 
> > So Layerscape doesn't respect CONFIG_SYS_MEM_TOP_HIDE at all?  If you
> > don't
> > want to add that in (and thus worry about where it should go relative to
> > the
> > other reasons), there should probably at least be an #error if the symbol
> > is
> > defined and non-zero.
> 
> Actually Layerscape was using CONFIG_SYS_MEM_TOP_HIDE and it defines
> CONFIG_SYS_MEM_TOP_HIDE as a function. When I tried to move the function out
> of
> board files, you suggested to "do it more generically and transparently".

Right, but it's still documented as a generic U-Boot feature so we shouldn't
silently ignore it.

> After this patch, CONFIG_SYS_MEM_TOP_HIDE shouldn't be used with this
> function.
> I can add a check for CONFIG_SYS_MEM_TOP_HIDE in this function to throw out
> an
> error if defined.

Thanks.

> > I think it'd be a bit more straightforward for this to return the new
> > ram_top
> > rather than the size to be subtracted.
> 
> That will be inaccurate. For layerscape SoCs, the gd->ram_size is known at
> this
> point, but the address is not known until later in board file dram bank is
> filled int. So this function can only return how much memory is reserved.

Sorry, I should have said offset from the start of RAM instead of address.  So
the caller does "gd->ram_size = board_reserve_ram_top(gd->ram_size);".

-Scott