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

[Mark Rutland]

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.

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

> +
> +	/*
> +	 * 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.

> +	if (images->ep != dst) {
> +		void *src;
> +
> +		debug("Moving Image from 0x%lx to 0x%llx\n", images->ep, dst);
> +
> +		src = (void *)images->ep;
> +		images->ep = dst;
> +		memmove((void *)dst, src, le32_to_cpu(ih->image_size));

Likewise.

> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Image booting support
> + */
> +static int booti_start(cmd_tbl_t *cmdtp, int flag, int argc,
> +			char * const argv[], bootm_headers_t *images)
> +{
> +	int ret;
> +	struct Image_header *ih;
> +
> +	ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START,
> +			      images, 1);
> +
> +	/* Setup Linux kernel Image entry point */
> +	if (!argc) {
> +		images->ep = load_addr;
> +		debug("*  kernel: default image load address = 0x%08lx\n",
> +				load_addr);
> +	} else {
> +		images->ep = simple_strtoul(argv[0], NULL, 16);
> +		debug("*  kernel: cmdline image address = 0x%08lx\n",
> +			images->ep);
> +	}
> +
> +	ret = booti_setup(images);
> +	if (ret != 0)
> +		return 1;
> +
> +	ih = (struct Image_header *)map_sysmem(images->ep, 0);
> +
> +	lmb_reserve(&images->lmb, images->ep, le32_to_cpu(ih->image_size));

Likewise.

[...]

> +#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,
Mark.