[U-Boot] [PATCH 04/13] arm: K3: Introduce System Firmware loader framework

[Tom Rini <trini@konsulko.com>]

On Thu, May 16, 2019 at 02:14:19PM -0500, Andreas Dannenberg wrote:
> Hi Tom,
> 
> On Thu, May 16, 2019 at 10:47:30AM -0500, Andreas Dannenberg wrote:
> > Hi Tom,
> > 
> > On Wed, May 15, 2019 at 05:50:31PM -0400, Tom Rini wrote:
> > > On Wed, May 15, 2019 at 04:39:22PM -0500, Andreas Dannenberg wrote:
> > > > Hi Tom,
> > > > 
> > > > On Wed, May 15, 2019 at 11:17:22AM -0400, Tom Rini wrote:
> > > > > On Tue, May 07, 2019 at 12:25:33PM -0500, Andreas Dannenberg wrote:
> > > > > 
> > > > > > Introduce a framework that allows loading the System Firmware (SYSFW)
> > > > > > binary as well as the associated configuration data from an image tree
> > > > > > blob named "sysfw.itb" from an FS-based MMC boot media or from an MMC
> > > > > > RAW mode partition or sector.
> > > > > > 
> > > > > > To simplify the handling of and loading from the different boot media
> > > > > > we tap into the existing U-Boot SPL framework usually used for loading
> > > > > > U-Boot by building on an earlier commit that exposes some of that
> > > > > > functionality.
> > > > > > 
> > > > > > Note that this initial implementation only supports FS and RAW-based
> > > > > > eMMC/SD card boot.
> > > > > [snip]
> > > > > > diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
> > > > > > index e677a2e01b..f1731dda58 100644
> > > > > > --- a/arch/arm/mach-k3/Kconfig
> > > > > > +++ b/arch/arm/mach-k3/Kconfig
> > > > > > @@ -58,6 +58,46 @@ config SYS_K3_BOOT_CORE_ID
> > > > > >  	int
> > > > > >  	default 16
> > > > > >  
> > > > > > +config K3_LOAD_SYSFW
> > > > > > +	bool
> > > > > > +	depends on SPL
> > > > > > +	default n
> > > > > 
> > > > > 'n' is already default, you can drop this.
> > > > 
> > > > Ok sure.
> > > > 
> > > > > 
> > > > > [snip]
> > > > > > +config K3_SYSFW_IMAGE_SIZE_MAX
> > > > > > +	int "Amount of memory dynamically allocated for loading SYSFW blob"
> > > > > > +	depends on K3_LOAD_SYSFW
> > > > > > +	default	269000
> > > > > > +	help
> > > > > > +	  Amount of memory reserved through dynamic allocation at runtime for
> > > > > > +	  loading the combined System Firmware and configuration image tree
> > > > > > +	  blob. Keep it as tight as possible, as this directly affects the
> > > > > > +	  overall SPL memory footprint.
> > > > > 
> > > > > This is missing a unit, and is 'int' really the best choice here (and
> > > > > really, I guess, 262.6KiB as a default) ?
> > 
> > I think 'int' is a more natural fit as this is to reserve memory for the
> > 'sysfw.itb' blob we load from media. This blob is build outside U-Boot
> > using the "System Firmware Image Generator" project [1], and if after
> > build somebody does an 'll' in the build directory the file size would
> > show up as decimal, and that's what needs to fit inside
> > K3_SYSFW_IMAGE_SIZE_MAX.
> > 
> > > > Will add a unit.
> > > > 
> > > > As for the specific value, in our R5 SPL memory is very very tight. The
> > > > value used here is basically used for a malloc(), and any extra byte
> > > > allocated will be "wasted" and not available for stack etc. Also our
> > > > SYSFW image that is loaded is fixed size (except some +/-100 odd bytes
> > > > if the configuration data is changed that's part of the same SYSFW.ITB
> > > > blob), so a rather tailored size makes sense here.
> > > 
> > > Right, that makes sense.  But how did you get to that
> > > not-exactly-"round" number rather than say 0x41A00 or some other
> > > slightly smaller value in hex?  If 0x41AC8 is right, then, OK, it's
> > > right.  It just seems strange at first.
> > 
> > The sysfw.itb blob consumed by the SYSFW loader comprises a fixed
> > component (the SYSFW itself), plus a few smaller chunks of variable data
> > containing user configuration data. During initial development the
> > combined size of this ITB was about 264,900 bytes, and 4KB was added as
> > slack to accommodate configuration changes, resulting in what looks like
> > the arbitrary number under discussion. The sysfw.itb blob has actually
> > since grown in size but there is still plenty of space for it to further
> > grow if needed. Since we use this value already in production since last
> > year with no issues I'd rather leave it alone (due to using stack based
> > malloc in R5 SPL increasing that value would directly decrease available
> > stack space with our R5 SPL memory layout where the stack is growing
> > down towards the R5 SPL image itself).
> 
> Small clarification, the size of the early malloc pool is of course
> controlled by CONFIG_SYS_MALLOC_F_LEN with K3_SYSFW_IMAGE_SIZE_MAX using
> the majority of it. However CONFIG_SYS_MALLOC_F_LEN is dimensioned such
> to "tightly wrap" both K3_SYSFW_IMAGE_SIZE_MAX and other users of the
> early malloc pool (fat fs is a major one), so the principle is the same.
> I'd rather leave K3_SYSFW_IMAGE_SIZE_MAX as it stands.
> 
> Some more details on the memory usage to follow...

That's a very helpful explanation, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190517/bc7c34a5/attachment.sig>