[U-Boot] [PATCH v2 2/4] cmd_bootm.c: Add 'booti' for ARM64 Linux kernel Images

[Tom Rini]

On Thu, Aug 14, 2014 at 04:16:50PM +0100, Mark Rutland wrote:
> Hi Tom,
> 
> On Thu, Aug 14, 2014 at 11:42:36AM +0100, Tom Rini wrote:
> > The default format for arm64 Linux kernels is the "Image" format,
> > described in Documentation/arm64/booting.txt.  This, along with an
> > optional gzip compression on top is all that is generated by default.
> > The Image format has a magic number within the header for verification,
> > a text_offset where the Image must be run from, an image_size that
> > includes the BSS and reserved fields.
> > 
> > This does not support automatic detection of a gzip compressed image.
> > 
> > Signed-off-by: Tom Rini <trini at ti.com>
> > 
> > ---
> > Changes in v1:
> > - Adopt to Mark Rutland's changes now in mainline kernel wrt text_offset
> >   / image_size
> > ---
> >  README             |    1 +
> >  common/cmd_bootm.c |  140 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/bootm.h    |    2 +-
> >  3 files changed, 142 insertions(+), 1 deletion(-)
> > 
> > diff --git a/README b/README
> > index 1d71359..b9af7ac 100644
> > --- a/README
> > +++ b/README
> > @@ -959,6 +959,7 @@ The following options need to be configured:
> >  		CONFIG_CMD_BMP		* BMP support
> >  		CONFIG_CMD_BSP		* Board specific commands
> >  		CONFIG_CMD_BOOTD	  bootd
> > +		CONFIG_CMD_BOOTI	* ARM64 Linux kernel Image support
> >  		CONFIG_CMD_CACHE	* icache, dcache
> >  		CONFIG_CMD_CLK   	* clock command support
> >  		CONFIG_CMD_CONSOLE	  coninfo
> > diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
> > index 8b897c8d..843ec6e 100644
> > --- a/common/cmd_bootm.c
> > +++ b/common/cmd_bootm.c
> > @@ -627,3 +627,143 @@ U_BOOT_CMD(
> >  	"boot Linux zImage image from memory", bootz_help_text
> >  );
> >  #endif	/* CONFIG_CMD_BOOTZ */
> > +
> > +#ifdef CONFIG_CMD_BOOTI
> > +/* See Documentation/arm64/booting.txt in the Linux kernel */
> > +struct Image_header {
> > +	uint32_t	code0;		/* Executable code */
> > +	uint32_t	code1;		/* Executable code */
> > +	uint64_t	text_offset;	/* Image load offset, LE */
> > +	uint64_t	image_size;	/* Effective Image size, LE */
> > +	uint64_t	res1;		/* reserved */
> > +	uint64_t	res2;		/* reserved */
> > +	uint64_t	res3;		/* reserved */
> > +	uint64_t	res4;		/* reserved */
> > +	uint32_t	magic;		/* Magic number */
> > +	uint32_t	res5;
> > +};
> > +
> > +#define LINUX_ARM64_IMAGE_MAGIC	0x644d5241
> > +
> > +static int booti_setup(bootm_headers_t *images)
> > +{
> > +	struct Image_header *ih;
> > +	uint64_t dst;
> > +
> > +	ih = (struct Image_header *)map_sysmem(images->ep, 0);
> > +
> > +	if (ih->magic != le32_to_cpu(LINUX_ARM64_IMAGE_MAGIC)) {
> > +		puts("Bad Linux ARM64 Image magic!\n");
> > +		return 1;
> > +	}
> > +	
> > +	if (ih->image_size == 0) {
> > +		puts("Image lacks image_size field, assuming 16MiB\n");
> > +		ih->image_size = (16 << 20);
> > +	}
> 
> This should work for a defconfig, but it might be possible to build a
> larger kernel. From experiments with an allyesconfig, I can build a
> ~60MB kernel with ~20MB of uninitialised data after the end of the
> Image.

Part of me just wants to error out in this case.  Today people are
wrapping vmlinux up with a legacy header and making uImages.  My hope is
that with this and 3.17 we can encourage Image/Image.*/FIT Image usage
instead.  We could just as easily whack in 128MB, all the same.

> Modifying the Image feels a little dodgy, but I can't think of anything
> this would break.

Yeah.  In my mind, an Image without this information is the corner case,
not the normal case.  Doing it this way (a fixup to the data) means we
don't have to error check this twice or play some other games.

> > +	/*
> > +	 * If we are not at the correct run-time location, set the new
> > +	 * correct location and then move the image there.
> > +	 */
> > +	dst = gd->bd->bi_dram[0].start + le32_to_cpu(ih->text_offset);
> 
> This should be le64_to_cpu(ih->text_offset) to be strictly correct.
> 
> I wouldn't imagine we'd ever have a text_offset larger than 4GB, but it
> would be nice to keep things consistent with the documentation and
> kernel code.

Oh that's right, sixty-four bits is what we have...

> > +#ifdef CONFIG_SYS_LONGHELP
> > +static char booti_help_text[] =
> > +	"[addr [initrd[:size]] [fdt]]\n"
> > +	"    - boot Linux Image stored in memory\n"
> > +	"\tThe argument 'initrd' is optional and specifies the address\n"
> > +	"\tof the initrd in memory. The optional argument ':size' allows\n"
> > +	"\tspecifying the size of RAW initrd.\n"
> > +#if defined(CONFIG_OF_LIBFDT)
> > +	"\tSince booting a Linux kernelrequires a flat device-tree\n"
> 
> Nit: space between "kernel" and "requires"

Thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140814/af14487a/attachment.pgp>