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

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

[Oleksandr Dmytryshyn]

Patch adds a possibility to boot dom0 kernel from uImage.

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

Changed since v4:
 * fixed the commit message
 * 'start' field is ignored by xen

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

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index d635a7e..88735f5 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,72 @@ 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 len;
+
+    if ( size < sizeof(uimage) )
+        return -EINVAL;
+
+    copy_from_paddr(&uimage, addr, sizeof(uimage));
+
+    if ( be32_to_cpu(uimage.magic) != UIMAGE_MAGIC )
+        return -EINVAL;
+
+    len = be32_to_cpu(uimage.size);
+
+    if ( len > size - sizeof(uimage) )
+        return -EINVAL;
+
+    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 +467,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 v5] xen: arm: introduce uImage probe function for Dom0

[Julien Grall]

Hi Oleksandr,

On 28/08/14 06:54, Oleksandr Dmytryshyn wrote:
>   #ifdef CONFIG_ARM_64
>   /*
>    * Check if the image is a 64-bit Image.
> @@ -398,6 +467,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 )

NIT: the coding style is:

if ( rc < 0 )

It looks like the other if in this function don't respect the coding 
style. I wouldn't resend a new version of this patch this small NIT. 
Maybe Ian can fix it when it will be applied?


Anyway:

Reviewed-by: Julien Grall <julien.grall@linaro.org>

Regards,

-- 
Julien Grall

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

[Ian Campbell]

On Fri, 2014-08-29 at 16:51 -0400, Julien Grall wrote:
> Hi Oleksandr,
> 
> On 28/08/14 06:54, Oleksandr Dmytryshyn wrote:
> >   #ifdef CONFIG_ARM_64
> >   /*
> >    * Check if the image is a 64-bit Image.
> > @@ -398,6 +467,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 )
> 
> NIT: the coding style is:
> 
> if ( rc < 0 )
> 
> It looks like the other if in this function don't respect the coding 
> style. I wouldn't resend a new version of this patch this small NIT. 
> Maybe Ian can fix it when it will be applied?

I fixed it to match the surrounding (but also wrong) coding style, so at
least it is now consistent.

> Anyway:
> 
> Reviewed-by: Julien Grall <julien.grall@linaro.org>

thanks, akced + applied

> +        printk(XENLOG_ERR "Not supported uImage arch type %d\n", uimage->arch);

I made this say "Unsupported ..." which reads more naturally to me. I
hope that's ok.

I also had to switch this and the switch() to use uimage.arch not
uimage->arch. Please at least compile test arm64 hypervisor changes when
touching common code (I don't insist for userspace since it is hard to
get hold of a suitable environment). The hypervisor cross compiles quite
easily using e.g. the linaro toolchains.

Ian.