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

[PATCH] 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 bootime. Comparing to zImage,
uImage is not packed, therefore we can save time needed
to unpack.

Change-Id: If92bc895721f0b2fa0a844bb92eddc8f1fdc5498
Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>
---
 xen/arch/arm/kernel.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index d635a7e..45d92c9 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -16,6 +16,12 @@
 
 #include "kernel.h"
 
+#define UIMAGE32_MAGIC_OFFSET 0x8
+#define UIMAGE32_START_OFFSET 0x4
+#define UIMAGE32_END_OFFSET   0x3
+#define UIMAGE32_HEADER_LEN   0x40
+#define UIMAGE32_MAGIC        0x304d4f44
+
 #define ZIMAGE32_MAGIC_OFFSET 0x24
 #define ZIMAGE32_START_OFFSET 0x28
 #define ZIMAGE32_END_OFFSET   0x2c
@@ -188,6 +194,65 @@ 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)
+{
+    uint32_t uimage[UIMAGE32_HEADER_LEN/4];
+    uint32_t start, len;
+    struct minimal_dtb_header dtb_hdr;
+
+    if ( size < UIMAGE32_HEADER_LEN )
+        return -EINVAL;
+
+    copy_from_paddr(uimage, addr, sizeof(uimage));
+
+    if ( uimage[UIMAGE32_MAGIC_OFFSET] != UIMAGE32_MAGIC )
+        return -EINVAL;
+
+    start = be32_to_cpu(uimage[UIMAGE32_START_OFFSET]);
+    len = be32_to_cpu(uimage[UIMAGE32_END_OFFSET]);
+
+    if ( len > size )
+        return -EINVAL;
+
+    /*
+     * Check for an appended DTB.
+     */
+    if ( addr + len + sizeof(dtb_hdr) <= size )
+    {
+        copy_from_paddr(&dtb_hdr, addr + len, sizeof(dtb_hdr));
+
+        if ( be32_to_cpu(dtb_hdr.magic) == DTB_MAGIC )
+            len += be32_to_cpu(dtb_hdr.total_size);
+
+            if ( len - start > addr + size )
+                return -EINVAL;
+    }
+
+    /*
+     * If start is UIMAGE32_HEADER_LEN 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 + UIMAGE32_HEADER_LEN;
+    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 +463,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] xen: arm: introduce uImage probe function for Dom0

[Julien Grall]

Hi Oleksandr,

Thanks you for your patch,

On 19/08/14 05:37, Oleksandr Dmytryshyn wrote:
> Patch adds a possibility to boot dom0 kernel from uImage.
> This is needed to improve bootime. Comparing to zImage,

boot-time

> uImage is not packed, therefore we can save time needed
> to unpack.
>
> Change-Id: If92bc895721f0b2fa0a844bb92eddc8f1fdc5498
> Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>
> ---
>   xen/arch/arm/kernel.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 67 insertions(+)
>
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index d635a7e..45d92c9 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -16,6 +16,12 @@
>
>   #include "kernel.h"
>
> +#define UIMAGE32_MAGIC_OFFSET 0x8
> +#define UIMAGE32_START_OFFSET 0x4
> +#define UIMAGE32_END_OFFSET   0x3

Can you define a structure as it's done for zImage64. It's easier to 
understand. Also, do you have any link to the uImage protocol?

A quick search doesn't allow me to find anything that match your format 
here.

[..]

> +    /*
> +     * Check for an appended DTB.
> +     */
> +    if ( addr + len + sizeof(dtb_hdr) <= size )
> +    {
> +        copy_from_paddr(&dtb_hdr, addr + len, sizeof(dtb_hdr));
> +
> +        if ( be32_to_cpu(dtb_hdr.magic) == DTB_MAGIC )
> +            len += be32_to_cpu(dtb_hdr.total_size);
> +
> +            if ( len - start > addr + size )
> +                return -EINVAL;
> +    }

This is a legacy on zImage, I don't really want to see it on 
implementation of a new format. Xen is creating the device tree for 
DOM0, the developer should not append a device tree.

-- 
Julien Grall

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

[Andrii Tseglytskyi]

Hi Julien,

>> +    /*
>> +     * Check for an appended DTB.
>> +     */
>> +    if ( addr + len + sizeof(dtb_hdr) <= size )
>> +    {
>> +        copy_from_paddr(&dtb_hdr, addr + len, sizeof(dtb_hdr));
>> +
>> +        if ( be32_to_cpu(dtb_hdr.magic) == DTB_MAGIC )
>> +            len += be32_to_cpu(dtb_hdr.total_size);
>> +
>> +            if ( len - start > addr + size )
>> +                return -EINVAL;
>> +    }
>
>
> This is a legacy on zImage, I don't really want to see it on implementation
> of a new format. Xen is creating the device tree for DOM0, the developer
> should not append a device tree.
>

Sorry this is not a topic of this patch, but what about appending
device tree to domU kernel? I found that similar functionality was
removed from tools by the following patch:

http://permalink.gmane.org/gmane.comp.emulators.xen.cvs/31110

But in case if some HW is present in domU you may need this
functionality. Code which supports HW in domU is going to be merged
(iomem and your irq routing series), so maybe appended devtree is also
OK?

Regards,
Andrii

> --
> Julien Grall
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel



-- 

Andrii Tseglytskyi | Embedded Dev
GlobalLogic
www.globallogic.com

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

[Julien Grall]

On 20/08/14 00:17, Andrii Tseglytskyi wrote:
> Hi Julien,

Hello Andrii,

>>> +    /*
>>> +     * Check for an appended DTB.
>>> +     */
>>> +    if ( addr + len + sizeof(dtb_hdr) <= size )
>>> +    {
>>> +        copy_from_paddr(&dtb_hdr, addr + len, sizeof(dtb_hdr));
>>> +
>>> +        if ( be32_to_cpu(dtb_hdr.magic) == DTB_MAGIC )
>>> +            len += be32_to_cpu(dtb_hdr.total_size);
>>> +
>>> +            if ( len - start > addr + size )
>>> +                return -EINVAL;
>>> +    }
>>
>>
>> This is a legacy on zImage, I don't really want to see it on implementation
>> of a new format. Xen is creating the device tree for DOM0, the developer
>> should not append a device tree.
>>
>
> Sorry this is not a topic of this patch, but what about appending
> device tree to domU kernel? I found that similar functionality was
> removed from tools by the following patch:
>
> http://permalink.gmane.org/gmane.comp.emulators.xen.cvs/31110

Only the check has been removed. The full binary is now loading in the 
memory. So appending device tree to kernel should still work.

>
> But in case if some HW is present in domU you may need this
> functionality. Code which supports HW in domU is going to be merged
> (iomem and your irq routing series), so maybe appended devtree is also
> OK?

IIRC, the plan is to allow the user to pass a snippet describing your 
device. The toolstack will add it in the generated device tree.

I don't think someone is working on it.

Regards,

-- 
Julien Grall

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

[Oleksandr Dmytryshyn]

Hi, Julien.

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

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


On Tue, Aug 19, 2014 at 8:06 PM, Julien Grall <julien.grall@linaro.org> wrote:
> Hi Oleksandr,
>
> Thanks you for your patch,
>
>
> On 19/08/14 05:37, Oleksandr Dmytryshyn wrote:
>>
>> Patch adds a possibility to boot dom0 kernel from uImage.
>> This is needed to improve bootime. Comparing to zImage,
>
>
> boot-time
>
>
>> uImage is not packed, therefore we can save time needed
>> to unpack.
>>
>> Change-Id: If92bc895721f0b2fa0a844bb92eddc8f1fdc5498
>> Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>
>> ---
>>   xen/arch/arm/kernel.c | 67
>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 67 insertions(+)
>>
>> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
>> index d635a7e..45d92c9 100644
>> --- a/xen/arch/arm/kernel.c
>> +++ b/xen/arch/arm/kernel.c
>> @@ -16,6 +16,12 @@
>>
>>   #include "kernel.h"
>>
>> +#define UIMAGE32_MAGIC_OFFSET 0x8
>> +#define UIMAGE32_START_OFFSET 0x4
>> +#define UIMAGE32_END_OFFSET   0x3
>
>
> Can you define a structure as it's done for zImage64. It's easier to
> understand. Also, do you have any link to the uImage protocol?
>
> A quick search doesn't allow me to find anything that match your format
> here.
Here is an uImage protocol link
http://www.isysop.com/unpacking-and-repacking-u-boot-uimage-files/

>
> [..]
>
>
>> +    /*
>> +     * Check for an appended DTB.
>> +     */
>> +    if ( addr + len + sizeof(dtb_hdr) <= size )
>> +    {
>> +        copy_from_paddr(&dtb_hdr, addr + len, sizeof(dtb_hdr));
>> +
>> +        if ( be32_to_cpu(dtb_hdr.magic) == DTB_MAGIC )
>> +            len += be32_to_cpu(dtb_hdr.total_size);
>> +
>> +            if ( len - start > addr + size )
>> +                return -EINVAL;
>> +    }
>
>
> This is a legacy on zImage, I don't really want to see it on implementation
> of a new format. Xen is creating the device tree for DOM0, the developer
> should not append a device tree.
>
> --
> Julien Grall