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

[PATCH v2] 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.

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

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

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index d635a7e..20e6884 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -16,6 +16,9 @@
 
 #include "kernel.h"
 
+#define UIMAGE32_MAGIC        0x27051956
+#define UIMAGE32_NMLEN        32
+
 #define ZIMAGE32_MAGIC_OFFSET 0x24
 #define ZIMAGE32_START_OFFSET 0x28
 #define ZIMAGE32_END_OFFSET   0x2c
@@ -188,6 +191,64 @@ static void kernel_zimage_load(struct kernel_info *info)
     }
 }
 
+/*
+ * Check if the image is a 32-bit uImage and setup kernel_info
+ */
+static int kernel_uimage32_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[UIMAGE32_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) != UIMAGE32_MAGIC )
+        return -EINVAL;
+
+    start = be32_to_cpu(uimage.load);
+    len = be32_to_cpu(uimage.size);
+
+    if ( len > size )
+        return -EINVAL;
+
+    /*
+     * If start is sizeof(zimage) zImage is position independent -- load it
+     * at 32k from start of RAM.
+     */
+    if ( start == 0 )
+        info->zimage.start = info->mem.bank[0].start + 0x8000;
+    else
+        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
+    info->type = DOMAIN_32BIT;
+#endif
+
+    return 0;
+}
+
 #ifdef CONFIG_ARM_64
 /*
  * Check if the image is a 64-bit Image.
@@ -398,6 +459,8 @@ int kernel_probe(struct kernel_info *info)
     rc = kernel_zimage64_probe(info, start, size);
     if (rc < 0)
 #endif
+        rc = kernel_uimage32_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 v2] xen: arm: introduce uImage probe function for Dom0

[Julien Grall]

Hi Oleksandr,

On 20/08/14 05:04, 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.

Can you add a link to the uImage format in the commit message and/or patch?

> 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
>
>   xen/arch/arm/kernel.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 63 insertions(+)
>
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index d635a7e..20e6884 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -16,6 +16,9 @@
>
>   #include "kernel.h"
>
> +#define UIMAGE32_MAGIC        0x27051956
> +#define UIMAGE32_NMLEN        32
> +

Why did you add "32" in the name? The uimage header is architecture 
independent.

[..]

> +    start = be32_to_cpu(uimage.load);
> +    len = be32_to_cpu(uimage.size);
> +
> +    if ( len > size )
> +        return -EINVAL;
> +
> +    /*
> +     * If start is sizeof(zimage) zImage is position independent -- load it
> +     * at 32k from start of RAM.

The comment doesn't seems relevant. Why are you talking about zimage?

> +     */
> +    if ( start == 0 )
> +        info->zimage.start = info->mem.bank[0].start + 0x8000;
> +    else
> +        info->zimage.start = start;

I know we have this check on zImage. As the kernel should be position 
independent, otherwise it will unlikely boot as DOM0, I think it would 
be better to print a warning if start != 0.

Or maybe we should just ignore it so the same uImage would be able to 
work on both bare-metal and Xen.

> +
> +    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
> +    info->type = DOMAIN_32BIT;
> +#endif

As said a bit earlier, the uImage is architecture independent. You have 
to check the arch field to know the kernel is 32 or 64 bits.

Regards,

-- 
Julien Grall