[PATCH v4] xen: arm: introduce uImage probe function for Dom0

[PATCH v4] xen: arm: introduce uImage probe function for Dom0

[Oleksandr Dmytryshyn]

Patch adds a possibility to boot dom0 kernel from uImage.
This is needed to improve boot-time. Comparing to zImage,
uImage is not packed, therefore we can save time needed
to unpack.

uImage header format:
http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=include/image.h

Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>
---
Changed since v1:
 * fixed commit message
 * added uimage structure definition
 * removed checking of the append device tree

Changed since v2:
 * removed "32" in the uimage constants name
 * added printing a warning if the uimage 'load' field != 0
 * added checking of the uimage 'arch' field to know the kernel is 32 or 64 bits

Changed since v3:
 * removed unused CPU Architecture Codes
 * fixed wrong uimage data size comparison
 * added switch() operator instead of the cascade of else if

 xen/arch/arm/kernel.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index d635a7e..6d6c637 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -16,6 +16,9 @@
 
 #include "kernel.h"
 
+#define UIMAGE_MAGIC          0x27051956
+#define UIMAGE_NMLEN          32
+
 #define ZIMAGE32_MAGIC_OFFSET 0x24
 #define ZIMAGE32_START_OFFSET 0x28
 #define ZIMAGE32_END_OFFSET   0x2c
@@ -188,6 +191,77 @@ static void kernel_zimage_load(struct kernel_info *info)
     }
 }
 
+/*
+ * Uimage CPU Architecture Codes
+ */
+#define IH_ARCH_ARM             2       /* ARM          */
+#define IH_ARCH_ARM64           22      /* ARM64        */
+
+/*
+ * Check if the image is a uImage and setup kernel_info
+ */
+static int kernel_uimage_probe(struct kernel_info *info,
+                                 paddr_t addr, paddr_t size)
+{
+    struct {
+        __be32 magic;   /* Image Header Magic Number */
+        __be32 hcrc;    /* Image Header CRC Checksum */
+        __be32 time;    /* Image Creation Timestamp  */
+        __be32 size;    /* Image Data Size           */
+        __be32 load;    /* Data Load Address         */
+        __be32 ep;      /* Entry Point Address       */
+        __be32 dcrc;    /* Image Data CRC Checksum   */
+        uint8_t os;     /* Operating System          */
+        uint8_t arch;   /* CPU architecture          */
+        uint8_t type;   /* Image Type                */
+        uint8_t comp;   /* Compression Type          */
+        uint8_t name[UIMAGE_NMLEN]; /* Image Name  */
+    } uimage;
+
+    uint32_t start, len;
+
+    if ( size < sizeof(uimage) )
+        return -EINVAL;
+
+    copy_from_paddr(&uimage, addr, sizeof(uimage));
+
+    if ( be32_to_cpu(uimage.magic) != UIMAGE_MAGIC )
+        return -EINVAL;
+
+    start = be32_to_cpu(uimage.load);
+    len = be32_to_cpu(uimage.size);
+
+    if ( len > size - sizeof(uimage) )
+        return -EINVAL;
+
+    if ( start == 0 )
+        printk(XENLOG_ERR "uImage start position is not defined\n");
+
+    info->zimage.start = start;
+    info->zimage.kernel_addr = addr + sizeof(uimage);
+    info->zimage.len = len;
+
+    info->entry = info->zimage.start;
+    info->load = kernel_zimage_load;
+
+#ifdef CONFIG_ARM_64
+    switch ( uimage->arch )
+    {
+    case IH_ARCH_ARM:
+        info->type = DOMAIN_32BIT;
+        break;
+    case IH_ARCH_ARM64:
+        info->type = DOMAIN_64BIT;
+        break;
+    default:
+        printk(XENLOG_ERR "Not supported uImage arch type %d\n", uimage->arch);
+        return -EINVAL;
+    }
+#endif
+
+    return 0;
+}
+
 #ifdef CONFIG_ARM_64
 /*
  * Check if the image is a 64-bit Image.
@@ -398,6 +472,8 @@ int kernel_probe(struct kernel_info *info)
     rc = kernel_zimage64_probe(info, start, size);
     if (rc < 0)
 #endif
+        rc = kernel_uimage_probe(info, start, size);
+    if(rc < 0 )
         rc = kernel_zimage32_probe(info, start, size);
     if (rc < 0)
         rc = kernel_elf_probe(info, start, size);
-- 
1.9.1

Re: [PATCH v4] xen: arm: introduce uImage probe function for Dom0

[Julien Grall]

Hi Oleksandr,

On 27/08/14 09:45, Oleksandr Dmytryshyn wrote:
> Patch adds a possibility to boot dom0 kernel from uImage.
> This is needed to improve boot-time. Comparing to zImage,
> uImage is not packed, therefore we can save time needed
> to unpack.

This is not entirely true. uImage is a container format to encapsulate 
anything you want. For instance I'm using a zImage in it.

OOI, if you don't want compress image, why not using ELF?

In any case, I think this is a good patch, it will avoid some hackish 
command in U-boot when the kernel is provided as a uImage :).

> +
> +    if ( start == 0 )
> +        printk(XENLOG_ERR "uImage start position is not defined\n");

> +
> +    info->zimage.start = start;

I think you misunderstood my point on the previous version. As Xen may 
allocate DOM0 bank anywhere in the memory, the uImage should contain a 
position independent kernel image. Otherwise the user may see failure if 
it decides to change the amount of memory for DOM0.

In general use case, I think Xen should ignore the start field and 
decide itself where to load the kernel in memory.

This will allow the user to use the same uImage to boot on bare metal 
and as DOM0. It will be very useful for Linaro testing as the image 
(kernel + distribution) is built generically. We only provide an 
additional layer with the Xen image.

Regards,

-- 
Julien Grall

Re: [PATCH v4] xen: arm: introduce uImage probe function for Dom0

[Oleksandr Dmytryshyn]

On Wed, Aug 27, 2014 at 5:40 PM, Julien Grall <julien.grall@linaro.org> wrote:
> Hi Oleksandr,
>
>
> On 27/08/14 09:45, Oleksandr Dmytryshyn wrote:
>>
>> Patch adds a possibility to boot dom0 kernel from uImage.
>> This is needed to improve boot-time. Comparing to zImage,
>> uImage is not packed, therefore we can save time needed
>> to unpack.
>
>
> This is not entirely true. uImage is a container format to encapsulate
> anything you want. For instance I'm using a zImage in it.
>
> OOI, if you don't want compress image, why not using ELF?
>
> In any case, I think this is a good patch, it will avoid some hackish
> command in U-boot when the kernel is provided as a uImage :).
I'll fix the commit message in the next patch.

>> +
>> +    if ( start == 0 )
>> +        printk(XENLOG_ERR "uImage start position is not defined\n");
>
>
>> +
>> +    info->zimage.start = start;
>
>
> I think you misunderstood my point on the previous version. As Xen may
> allocate DOM0 bank anywhere in the memory, the uImage should contain a
> position independent kernel image. Otherwise the user may see failure if it
> decides to change the amount of memory for DOM0.
>
> In general use case, I think Xen should ignore the start field and decide
> itself where to load the kernel in memory.
>
> This will allow the user to use the same uImage to boot on bare metal and as
> DOM0. It will be very useful for Linaro testing as the image (kernel +
> distribution) is built generically. We only provide an additional layer with
> the Xen image.
I've checked it (when Xen ignores the 'start' field). It works. I'll
fix it in the next patch.

> Regards,
>
> --
> Julien Grall

-- 
Oleksandr Dmytryshyn | Product Engineering and Development
GlobalLogic
M +38.067.382.2525
www.globallogic.com

http://www.globallogic.com/email_disclaimer.txt