On Tue, May 13, 2014 at 08:55:17PM -0500, Rob Herring wrote:

On Mon, May 5, 2014 at 10:26 AM, Tom Rini trini@ti.com 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 and reserved fields.

This does not support automatic detection of a gzip compressed image.

Signed-off-by: Tom Rini trini@ti.com

README | 1 + common/cmd_bootm.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+)

diff --git a/README b/README index 12758dc..e7c8b2e 100644 --- a/README +++ b/README @@ -946,6 +946,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
  

As I mentioned at ELC, I would prefer to see bootz and booti combined so we have a single "boot a plain kernel image" command. I'd rather see the command be smart enough to figure what the image is rather than the user have to figure out the right command. Of course with this argument, then everything should be a bootm command. IIRC, there would be some challenges to add bootz or booti functionality into bootm.

Yes, this is an idea worth exploring.

...

            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 c243a5b..3b9635c 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -1929,3 +1929,142 @@ 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 */
  

•   uint64_t        res0;           /* reserved */
  

•   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 +/* XXX: Hack 16MB image size for now */ +#define HACK_ARM64_IMAGE_SIZE (16 << 20)

+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 != LINUX_ARM64_IMAGE_MAGIC) {
  

•           puts("Bad Linux ARM64 Image magic!\n");
  

•           return 1;
  

•   }
  

•   /*
  

•    * 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 + ih->text_offset;
  

•   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, HACK_ARM64_IMAGE_SIZE);
  

Don't you at least know the load size or the decompressed size? Only the .bss size is missing and you only need that to have some checks for overlapping images.

No, we don't know for certain what the size is today. For example, my workflow is to load the kernel, load the dtb so filesize would have had the Image size, but then we've overwritten it. Since this information will be part of the header, so long as it's merged into the kernel soon I'm OK with saying at least this part of the series is just for RFC and people can apply it locally as needed. If we handled Image.gz we would know the decompress size but today I expect to handle Image.gz as a 2 step unzip and then booti. As I said in 0/4 I have some concern about doing it all as a single step, especially once randomized text offset happens (at least, depending on the range the offset could be within).

On Mon, May 5, 2014 at 10:26 AM, Tom Rini trini@ti.com wrote:

• Drop DEBUG
• Drop defines we can use the default of.
• Drop CONFIG_GICV3 and related defines, we aren't using interrupts and do not need to configure the interrupt controller.
• Provide a larger malloc pool.
• Correct default locations for kernel / initrd / device tree

Signed-off-by: Tom Rini trini@ti.com

[...]

-#define CONFIG_GICV3

/*#define CONFIG_ARMV8_SWITCH_TO_EL1*/

/*#define CONFIG_SYS_GENERIC_BOARD*/ @@ -94,20 +90,11 @@ /* Generic Timer Definitions */ #define COUNTER_FREQUENCY (0x1800000) /* 24MHz */

-/* Generic Interrupt Controller Definitions */ -#ifdef CONFIG_GICV3 -#define GICD_BASE (0x2f000000) -#define GICR_BASE (0x2f100000) -#else -#define GICD_BASE (0x2C001000) -#define GICC_BASE (0x2C002000) -#endif

IIRC, these should be needed if secure to non-secure switching is done.

#define CONFIG_SYS_MEMTEST_START V2M_BASE #define CONFIG_SYS_MEMTEST_END (V2M_BASE + 0x80000000)

/* Size of malloc() pool */ -#define CONFIG_SYS_MALLOC_LEN (CONFIG_ENV_SIZE + 128 * 1024) +#define CONFIG_SYS_MALLOC_LEN (CONFIG_ENV_SIZE + (8 << 20))

This seems like another candidate for some sane default value.

/* SMSC91C111 Ethernet Configuration */ #define CONFIG_SMC91111 1 @@ -121,7 +108,6 @@ #define CONFIG_CONS_INDEX 0

#define CONFIG_BAUDRATE 115200 -#define CONFIG_SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200 } #define CONFIG_SYS_SERIAL0 V2M_UART0 #define CONFIG_SYS_SERIAL1 V2M_UART1

@@ -166,10 +152,9 @@

/* Initial environment variables */ #define CONFIG_EXTRA_ENV_SETTINGS \

•                                   "kernel_addr=0x200000\0"        \
  

•                                   "initrd_addr=0xa00000\0"        \
  

•                                   "initrd_size=0x2000000\0"       \
  

•                                   "fdt_addr=0x100000\0"           \
  

•                                   "kernel_addr=0x80080000\0"      \
  

•                                   "initrd_addr=0x90000000\0"      \
  

•                                   "fdt_addr=0x88000000\0"         \
  

Don't we want the _r variants here?

Rob