All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] xen: arm: introduce uImage probe function for Dom0
@ 2014-08-21  9:48 Oleksandr Dmytryshyn
  2014-08-26  9:53 ` Andrii Tseglytskyi
  2014-08-26 20:44 ` Ian Campbell
  0 siblings, 2 replies; 5+ messages in thread
From: Oleksandr Dmytryshyn @ 2014-08-21  9:48 UTC (permalink / raw)
  To: Ian Campbell, Stefano Stabellini, Tim Deegan, xen-devel

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

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

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index d635a7e..3adc6db 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,95 @@ static void kernel_zimage_load(struct kernel_info *info)
     }
 }
 
+/*
+ * Uimage CPU Architecture Codes
+ */
+#define IH_ARCH_INVALID         0       /* Invalid CPU  */
+#define IH_ARCH_ALPHA           1       /* Alpha        */
+#define IH_ARCH_ARM             2       /* ARM          */
+#define IH_ARCH_I386            3       /* Intel x86    */
+#define IH_ARCH_IA64            4       /* IA64         */
+#define IH_ARCH_MIPS            5       /* MIPS         */
+#define IH_ARCH_MIPS64          6       /* MIPS  64 Bit */
+#define IH_ARCH_PPC             7       /* PowerPC      */
+#define IH_ARCH_S390            8       /* IBM S390     */
+#define IH_ARCH_SH              9       /* SuperH       */
+#define IH_ARCH_SPARC           10      /* Sparc        */
+#define IH_ARCH_SPARC64         11      /* Sparc 64 Bit */
+#define IH_ARCH_M68K            12      /* M68K         */
+#define IH_ARCH_MICROBLAZE      14      /* MicroBlaze   */
+#define IH_ARCH_NIOS2           15      /* Nios-II      */
+#define IH_ARCH_BLACKFIN        16      /* Blackfin     */
+#define IH_ARCH_AVR32           17      /* AVR32        */
+#define IH_ARCH_ST200           18      /* STMicroelectronics ST200  */
+#define IH_ARCH_SANDBOX         19      /* Sandbox architecture (test only) */
+#define IH_ARCH_NDS32           20      /* ANDES Technology - NDS32  */
+#define IH_ARCH_OPENRISC        21      /* OpenRISC 1000  */
+#define IH_ARCH_ARM64           22      /* ARM64        */
+#define IH_ARCH_ARC             23      /* Synopsys DesignWare ARC */
+
+/*
+ * 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 )
+        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
+    if ( uimage->arch == IH_ARCH_ARM )
+        info->type = DOMAIN_32BIT;
+    else if ( uimage->arch == IH_ARCH_ARM64 )
+        info->type = DOMAIN_64BIT;
+    else
+    {
+        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 +490,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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] xen: arm: introduce uImage probe function for Dom0
  2014-08-21  9:48 [PATCH v3] xen: arm: introduce uImage probe function for Dom0 Oleksandr Dmytryshyn
@ 2014-08-26  9:53 ` Andrii Tseglytskyi
  2014-08-26 19:47   ` Ian Campbell
  2014-08-26 20:44 ` Ian Campbell
  1 sibling, 1 reply; 5+ messages in thread
From: Andrii Tseglytskyi @ 2014-08-26  9:53 UTC (permalink / raw)
  To: Oleksandr Dmytryshyn, Ian Campbell, Stefano Stabellini, Julien Grall
  Cc: Tim Deegan, xen-devel

Hi,

Julien, Ian, Stefano - could you please comment on this?
As soon as we (GlobalLogic) are going to switch to Xen 4.5 release -
we need this patch to be reviewed / merged.
We are targeting to have as less local patches as possible.

Thank you in advance,

Regards,
Andrii

On Thu, Aug 21, 2014 at 12:48 PM, Oleksandr Dmytryshyn
<oleksandr.dmytryshyn@globallogic.com> 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.
>
> 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
>
>  xen/arch/arm/kernel.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 94 insertions(+)
>
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index d635a7e..3adc6db 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,95 @@ static void kernel_zimage_load(struct kernel_info *info)
>      }
>  }
>
> +/*
> + * Uimage CPU Architecture Codes
> + */
> +#define IH_ARCH_INVALID         0       /* Invalid CPU  */
> +#define IH_ARCH_ALPHA           1       /* Alpha        */
> +#define IH_ARCH_ARM             2       /* ARM          */
> +#define IH_ARCH_I386            3       /* Intel x86    */
> +#define IH_ARCH_IA64            4       /* IA64         */
> +#define IH_ARCH_MIPS            5       /* MIPS         */
> +#define IH_ARCH_MIPS64          6       /* MIPS  64 Bit */
> +#define IH_ARCH_PPC             7       /* PowerPC      */
> +#define IH_ARCH_S390            8       /* IBM S390     */
> +#define IH_ARCH_SH              9       /* SuperH       */
> +#define IH_ARCH_SPARC           10      /* Sparc        */
> +#define IH_ARCH_SPARC64         11      /* Sparc 64 Bit */
> +#define IH_ARCH_M68K            12      /* M68K         */
> +#define IH_ARCH_MICROBLAZE      14      /* MicroBlaze   */
> +#define IH_ARCH_NIOS2           15      /* Nios-II      */
> +#define IH_ARCH_BLACKFIN        16      /* Blackfin     */
> +#define IH_ARCH_AVR32           17      /* AVR32        */
> +#define IH_ARCH_ST200           18      /* STMicroelectronics ST200  */
> +#define IH_ARCH_SANDBOX         19      /* Sandbox architecture (test only) */
> +#define IH_ARCH_NDS32           20      /* ANDES Technology - NDS32  */
> +#define IH_ARCH_OPENRISC        21      /* OpenRISC 1000  */
> +#define IH_ARCH_ARM64           22      /* ARM64        */
> +#define IH_ARCH_ARC             23      /* Synopsys DesignWare ARC */
> +
> +/*
> + * 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 )
> +        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
> +    if ( uimage->arch == IH_ARCH_ARM )
> +        info->type = DOMAIN_32BIT;
> +    else if ( uimage->arch == IH_ARCH_ARM64 )
> +        info->type = DOMAIN_64BIT;
> +    else
> +    {
> +        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 +490,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
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel



-- 

Andrii Tseglytskyi | Embedded Dev
GlobalLogic
www.globallogic.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] xen: arm: introduce uImage probe function for Dom0
  2014-08-26  9:53 ` Andrii Tseglytskyi
@ 2014-08-26 19:47   ` Ian Campbell
  0 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2014-08-26 19:47 UTC (permalink / raw)
  To: Andrii Tseglytskyi
  Cc: Oleksandr Dmytryshyn, Julien Grall, Tim Deegan, xen-devel,
	Stefano Stabellini

On Tue, 2014-08-26 at 12:53 +0300, Andrii Tseglytskyi wrote:
> Hi,
> 
> Julien, Ian, Stefano - could you please comment on this?
> As soon as we (GlobalLogic) are going to switch to Xen 4.5 release -
> we need this patch to be reviewed / merged.
> We are targeting to have as less local patches as possible.

I'm travelling right now so I'm way behind on the my review patch. In
principal this looks fine to me and I'm prertty confident we can get it
(or something like it) into 4.5 no problem.

Ian.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] xen: arm: introduce uImage probe function for Dom0
  2014-08-21  9:48 [PATCH v3] xen: arm: introduce uImage probe function for Dom0 Oleksandr Dmytryshyn
  2014-08-26  9:53 ` Andrii Tseglytskyi
@ 2014-08-26 20:44 ` Ian Campbell
  2014-08-27  8:01   ` Oleksandr Dmytryshyn
  1 sibling, 1 reply; 5+ messages in thread
From: Ian Campbell @ 2014-08-26 20:44 UTC (permalink / raw)
  To: Oleksandr Dmytryshyn; +Cc: Tim Deegan, Stefano Stabellini, xen-devel

On Thu, 2014-08-21 at 12:48 +0300, 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.
> 
> 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>

Mostly looks good, a few minor things.

> +/*
> + * Uimage CPU Architecture Codes
> + */

I think we can get away with only defining the two arm ones here.

> +    start = be32_to_cpu(uimage.load);
> +    len = be32_to_cpu(uimage.size);
> +
> +    if ( len > size )

Does len include the header? (IOW do you need to subtract sizeof(uimage)
from something?)

> +    info->entry = info->zimage.start;
> +    info->load = kernel_zimage_load;
> +
> +#ifdef CONFIG_ARM_64
> +    if ( uimage->arch == IH_ARCH_ARM )
> +        info->type = DOMAIN_32BIT;
> +    else if ( uimage->arch == IH_ARCH_ARM64 )
> +        info->type = DOMAIN_64BIT;
> +    else

Use switch() rather than a cascade of else if please.

Ian.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] xen: arm: introduce uImage probe function for Dom0
  2014-08-26 20:44 ` Ian Campbell
@ 2014-08-27  8:01   ` Oleksandr Dmytryshyn
  0 siblings, 0 replies; 5+ messages in thread
From: Oleksandr Dmytryshyn @ 2014-08-27  8:01 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Tim Deegan, Stefano Stabellini, xen-devel

On Tue, Aug 26, 2014 at 11:44 PM, Ian Campbell <ian.campbell@citrix.com> wrote:
> On Thu, 2014-08-21 at 12:48 +0300, 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.
>>
>> 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>
>
> Mostly looks good, a few minor things.
>
>> +/*
>> + * Uimage CPU Architecture Codes
>> + */
>
> I think we can get away with only defining the two arm ones here.
I'll fix this in the next version of the patch.

>> +    start = be32_to_cpu(uimage.load);
>> +    len = be32_to_cpu(uimage.size);
>> +
>> +    if ( len > size )
>
> Does len include the header? (IOW do you need to subtract sizeof(uimage)
> from something?)
len doesn't include the header. I'll fix this in the next version of the patch.

>> +    info->entry = info->zimage.start;
>> +    info->load = kernel_zimage_load;
>> +
>> +#ifdef CONFIG_ARM_64
>> +    if ( uimage->arch == IH_ARCH_ARM )
>> +        info->type = DOMAIN_32BIT;
>> +    else if ( uimage->arch == IH_ARCH_ARM64 )
>> +        info->type = DOMAIN_64BIT;
>> +    else
>
> Use switch() rather than a cascade of else if please.
I'll fix this in the next version of the patch.

>
> Ian.
>

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

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-08-27  8:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-21  9:48 [PATCH v3] xen: arm: introduce uImage probe function for Dom0 Oleksandr Dmytryshyn
2014-08-26  9:53 ` Andrii Tseglytskyi
2014-08-26 19:47   ` Ian Campbell
2014-08-26 20:44 ` Ian Campbell
2014-08-27  8:01   ` Oleksandr Dmytryshyn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.