xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] xen: arm: Update arm64 image header
@ 2016-06-23  6:38 Dirk Behme
  2016-06-23 15:18 ` Julien Grall
  0 siblings, 1 reply; 6+ messages in thread
From: Dirk Behme @ 2016-06-23  6:38 UTC (permalink / raw)
  To: xen-devel, Julien Grall, Stefano Stabellini; +Cc: Dirk Behme

With the Linux kernel commits

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/arm64/booting.txt?id=4370eec05a887b0cd4392cd5dc5b2713174745c0

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/arm64/booting.txt?id=a2c1d73b94ed49f5fac12e95052d7b140783f800

the arm64 image header changed. While the size of the header isn't changed,
some members have changed their usage.

Update Xen to this updated image header.

The main changes are that the first magic is gone and that there is an
image size, now.

In case we read a size != 0, let's use this image size, now. This does
allow us to check if the kernel Image is larger than the size given in
the device tree, too.

Additionally, add an error message if the magic is not found. This might
be the case with kernel's < 3.12 prior to

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4370eec05a887b0cd4392cd5dc5b2713174745c0

which introduced the second magic.

This is acceptable as the support of Xen for ARM64 in Linux has been added
in Linux 3.11 and the number of boards supported by Linux 3.11 on ARM64 is
very limited: ARM models and X-gene. And for the latter it was an early
support with only the serial and timer upstreamed.

Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
---

Changes in v3: Just update of the commit message regarding the support
               for kernels < 3.12. No change to the patch itself.


 xen/arch/arm/kernel.c | 43 ++++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 9871bd9..9b9a793 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -28,8 +28,7 @@
 
 #define ZIMAGE32_MAGIC 0x016f2818
 
-#define ZIMAGE64_MAGIC_V0 0x14000008
-#define ZIMAGE64_MAGIC_V1 0x644d5241 /* "ARM\x64" */
+#define ZIMAGE64_MAGIC 0x644d5241 /* "ARM\x64" */
 
 struct minimal_dtb_header {
     uint32_t magic;
@@ -335,17 +334,17 @@ static int kernel_zimage64_probe(struct kernel_info *info,
 {
     /* linux/Documentation/arm64/booting.txt */
     struct {
-        uint32_t magic0;
-        uint32_t res0;
-        uint64_t text_offset;  /* Image load offset */
-        uint64_t res1;
-        uint64_t res2;
+        uint32_t code0;
+        uint32_t code1;
+        uint64_t text_offset;  /* Image load offset, little endian */
+        uint64_t image_size;   /* Effective Image size, little endian */
+        uint64_t flags;
         /* zImage V1 only from here */
+        uint64_t res2;
         uint64_t res3;
         uint64_t res4;
-        uint64_t res5;
-        uint32_t magic1;
-        uint32_t res6;
+        uint32_t magic;        /* Magic number, little endian, "ARM\x64" */
+        uint32_t res5;
     } zimage;
     uint64_t start, end;
 
@@ -354,20 +353,30 @@ static int kernel_zimage64_probe(struct kernel_info *info,
 
     copy_from_paddr(&zimage, addr, sizeof(zimage));
 
-    if ( zimage.magic0 != ZIMAGE64_MAGIC_V0 &&
-         zimage.magic1 != ZIMAGE64_MAGIC_V1 )
+    if ( zimage.magic != ZIMAGE64_MAGIC ) {
+        printk(XENLOG_ERR "No valid magic found in header! Kernel too old?\n");
         return -EINVAL;
+    }
 
-    /* Currently there is no length in the header, so just use the size */
     start = 0;
-    end = size;
 
     /*
-     * Given the above this check is a bit pointless, but leave it
-     * here in case someone adds a length field in the future.
+     * Where image_size is non-zero image_size is little-endian
+     * and must be respected.
      */
-    if ( (end - start) > size )
+    if ( zimage.image_size )
+        end = zimage.image_size;
+    else
+        end = size;
+
+    if ( (end - start) > size ) {
+        if ( zimage.image_size ) {
+            printk(XENLOG_ERR "Error: Kernel Image size: %lu bytes > bootmodule size: %lu bytes\n",
+                   zimage.image_size, (uint64_t)size);
+            printk(XENLOG_ERR "The field 'size' does not match the size of blob!\n");
+        }
         return -EINVAL;
+    }
 
     info->zimage.kernel_addr = addr;
     info->zimage.len = end - start;
-- 
2.8.0


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

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

* Re: [PATCH v3] xen: arm: Update arm64 image header
  2016-06-23  6:38 [PATCH v3] xen: arm: Update arm64 image header Dirk Behme
@ 2016-06-23 15:18 ` Julien Grall
  2016-06-26  5:47   ` Dirk Behme
  0 siblings, 1 reply; 6+ messages in thread
From: Julien Grall @ 2016-06-23 15:18 UTC (permalink / raw)
  To: Dirk Behme, xen-devel, Stefano Stabellini

Hi Dirk,

On 23/06/16 07:38, Dirk Behme wrote:
> With the Linux kernel commits
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/arm64/booting.txt?id=4370eec05a887b0cd4392cd5dc5b2713174745c0
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/arm64/booting.txt?id=a2c1d73b94ed49f5fac12e95052d7b140783f800
>
> the arm64 image header changed. While the size of the header isn't changed,
> some members have changed their usage.
>
> Update Xen to this updated image header.
>
> The main changes are that the first magic is gone and that there is an
> image size, now.
>
> In case we read a size != 0, let's use this image size, now. This does
> allow us to check if the kernel Image is larger than the size given in
> the device tree, too.
>
> Additionally, add an error message if the magic is not found. This might
> be the case with kernel's < 3.12 prior to
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4370eec05a887b0cd4392cd5dc5b2713174745c0
>
> which introduced the second magic.
>
> This is acceptable as the support of Xen for ARM64 in Linux has been added
> in Linux 3.11 and the number of boards supported by Linux 3.11 on ARM64 is
> very limited: ARM models and X-gene. And for the latter it was an early
> support with only the serial and timer upstreamed.
>
> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
> ---
>
> Changes in v3: Just update of the commit message regarding the support
>                 for kernels < 3.12. No change to the patch itself.
>
>
>   xen/arch/arm/kernel.c | 43 ++++++++++++++++++++++++++-----------------
>   1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 9871bd9..9b9a793 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -28,8 +28,7 @@
>
>   #define ZIMAGE32_MAGIC 0x016f2818
>
> -#define ZIMAGE64_MAGIC_V0 0x14000008
> -#define ZIMAGE64_MAGIC_V1 0x644d5241 /* "ARM\x64" */
> +#define ZIMAGE64_MAGIC 0x644d5241 /* "ARM\x64" */
>
>   struct minimal_dtb_header {
>       uint32_t magic;
> @@ -335,17 +334,17 @@ static int kernel_zimage64_probe(struct kernel_info *info,
>   {
>       /* linux/Documentation/arm64/booting.txt */
>       struct {
> -        uint32_t magic0;
> -        uint32_t res0;
> -        uint64_t text_offset;  /* Image load offset */
> -        uint64_t res1;
> -        uint64_t res2;
> +        uint32_t code0;
> +        uint32_t code1;
> +        uint64_t text_offset;  /* Image load offset, little endian */
> +        uint64_t image_size;   /* Effective Image size, little endian */
> +        uint64_t flags;
>           /* zImage V1 only from here */

I think this comment is irrelevant now.

> +        uint64_t res2;
>           uint64_t res3;
>           uint64_t res4;
> -        uint64_t res5;
> -        uint32_t magic1;
> -        uint32_t res6;
> +        uint32_t magic;        /* Magic number, little endian, "ARM\x64" */
> +        uint32_t res5;
>       } zimage;
>       uint64_t start, end;
>
> @@ -354,20 +353,30 @@ static int kernel_zimage64_probe(struct kernel_info *info,
>
>       copy_from_paddr(&zimage, addr, sizeof(zimage));
>
> -    if ( zimage.magic0 != ZIMAGE64_MAGIC_V0 &&
> -         zimage.magic1 != ZIMAGE64_MAGIC_V1 )
> +    if ( zimage.magic != ZIMAGE64_MAGIC ) {
> +        printk(XENLOG_ERR "No valid magic found in header! Kernel too old?\n");

I have found why there were no error messages here before. The function 
kernel_probe will try the different formats supported one by one.

So this message will be printed if the kernel is an ARM32 image, which 
will confuse the user. So I would print this message only when 
zimage.magic0 is equal to ZIMAGE64_MAGIC_V0.

>           return -EINVAL;
> +    }
>
> -    /* Currently there is no length in the header, so just use the size */
>       start = 0;
> -    end = size;
>
>       /*
> -     * Given the above this check is a bit pointless, but leave it
> -     * here in case someone adds a length field in the future.
> +     * Where image_size is non-zero image_size is little-endian
> +     * and must be respected.
>        */
> -    if ( (end - start) > size )
> +    if ( zimage.image_size )
> +        end = zimage.image_size;
> +    else
> +        end = size;
> +
> +    if ( (end - start) > size ) {
> +        if ( zimage.image_size ) {

This check is not necessary. "(end - start) > size" will only succeed 
when zimage.image_size is different than 0.

> +            printk(XENLOG_ERR "Error: Kernel Image size: %lu bytes > bootmodule size: %lu bytes\n",
> +                   zimage.image_size, (uint64_t)size);
> +            printk(XENLOG_ERR "The field 'size' does not match the size of blob!\n");
> +        }
>           return -EINVAL;
> +    }
>
>       info->zimage.kernel_addr = addr;
>       info->zimage.len = end - start;
>

Regards,

-- 
Julien Grall

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

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

* Re: [PATCH v3] xen: arm: Update arm64 image header
  2016-06-23 15:18 ` Julien Grall
@ 2016-06-26  5:47   ` Dirk Behme
  2016-06-26  8:29     ` Julien Grall
  0 siblings, 1 reply; 6+ messages in thread
From: Dirk Behme @ 2016-06-26  5:47 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, Dirk Behme, Stefano Stabellini

Hi Julien,

On 23.06.2016 17:18, Julien Grall wrote:
> Hi Dirk,
>
> On 23/06/16 07:38, Dirk Behme wrote:
>> With the Linux kernel commits
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/arm64/booting.txt?id=4370eec05a887b0cd4392cd5dc5b2713174745c0
>>
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/arm64/booting.txt?id=a2c1d73b94ed49f5fac12e95052d7b140783f800
>>
>>
>> the arm64 image header changed. While the size of the header isn't
>> changed,
>> some members have changed their usage.
>>
>> Update Xen to this updated image header.
>>
>> The main changes are that the first magic is gone and that there is an
>> image size, now.
>>
>> In case we read a size != 0, let's use this image size, now. This does
>> allow us to check if the kernel Image is larger than the size given in
>> the device tree, too.
>>
>> Additionally, add an error message if the magic is not found. This
>> might
>> be the case with kernel's < 3.12 prior to
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=4370eec05a887b0cd4392cd5dc5b2713174745c0
>>
>>
>> which introduced the second magic.
>>
>> This is acceptable as the support of Xen for ARM64 in Linux has been
>> added
>> in Linux 3.11 and the number of boards supported by Linux 3.11 on
>> ARM64 is
>> very limited: ARM models and X-gene. And for the latter it was an early
>> support with only the serial and timer upstreamed.
>>
>> Signed-off-by: Dirk Behme <dirk.behme@de.bosch.com>
>> ---
>>
>> Changes in v3: Just update of the commit message regarding the support
>>                 for kernels < 3.12. No change to the patch itself.
>>
>>
>>   xen/arch/arm/kernel.c | 43
>> ++++++++++++++++++++++++++-----------------
>>   1 file changed, 26 insertions(+), 17 deletions(-)
>>
>> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
>> index 9871bd9..9b9a793 100644
>> --- a/xen/arch/arm/kernel.c
>> +++ b/xen/arch/arm/kernel.c
>> @@ -28,8 +28,7 @@
>>
>>   #define ZIMAGE32_MAGIC 0x016f2818
>>
>> -#define ZIMAGE64_MAGIC_V0 0x14000008
>> -#define ZIMAGE64_MAGIC_V1 0x644d5241 /* "ARM\x64" */
>> +#define ZIMAGE64_MAGIC 0x644d5241 /* "ARM\x64" */
>>
>>   struct minimal_dtb_header {
>>       uint32_t magic;
>> @@ -335,17 +334,17 @@ static int kernel_zimage64_probe(struct
>> kernel_info *info,
>>   {
>>       /* linux/Documentation/arm64/booting.txt */
>>       struct {
>> -        uint32_t magic0;
>> -        uint32_t res0;
>> -        uint64_t text_offset;  /* Image load offset */
>> -        uint64_t res1;
>> -        uint64_t res2;
>> +        uint32_t code0;
>> +        uint32_t code1;
>> +        uint64_t text_offset;  /* Image load offset, little endian */
>> +        uint64_t image_size;   /* Effective Image size, little
>> endian */
>> +        uint64_t flags;
>>           /* zImage V1 only from here */
>
> I think this comment is irrelevant now.


Yes :) I'll remove that.


>> +        uint64_t res2;
>>           uint64_t res3;
>>           uint64_t res4;
>> -        uint64_t res5;
>> -        uint32_t magic1;
>> -        uint32_t res6;
>> +        uint32_t magic;        /* Magic number, little endian,
>> "ARM\x64" */
>> +        uint32_t res5;
>>       } zimage;
>>       uint64_t start, end;
>>
>> @@ -354,20 +353,30 @@ static int kernel_zimage64_probe(struct
>> kernel_info *info,
>>
>>       copy_from_paddr(&zimage, addr, sizeof(zimage));
>>
>> -    if ( zimage.magic0 != ZIMAGE64_MAGIC_V0 &&
>> -         zimage.magic1 != ZIMAGE64_MAGIC_V1 )
>> +    if ( zimage.magic != ZIMAGE64_MAGIC ) {
>> +        printk(XENLOG_ERR "No valid magic found in header! Kernel
>> too old?\n");
>
> I have found why there were no error messages here before. The
> function kernel_probe will try the different formats supported one by
> one.
>
> So this message will be printed if the kernel is an ARM32 image, which
> will confuse the user. So I would print this message only when
> zimage.magic0 is equal to ZIMAGE64_MAGIC_V0.


Which we don't have with the recent header format any more. This does 
mean I drop this message again, as it doesn't make sense if the magic 
is used for the format detection.


>>           return -EINVAL;
>> +    }
>>
>> -    /* Currently there is no length in the header, so just use the
>> size */
>>       start = 0;
>> -    end = size;
>>
>>       /*
>> -     * Given the above this check is a bit pointless, but leave it
>> -     * here in case someone adds a length field in the future.
>> +     * Where image_size is non-zero image_size is little-endian
>> +     * and must be respected.
>>        */
>> -    if ( (end - start) > size )
>> +    if ( zimage.image_size )
>> +        end = zimage.image_size;
>> +    else
>> +        end = size;
>> +
>> +    if ( (end - start) > size ) {
>> +        if ( zimage.image_size ) {
>
> This check is not necessary. "(end - start) > size" will only succeed
> when zimage.image_size is different than 0.


Yes, indeed :) I'll remove that.


>> +            printk(XENLOG_ERR "Error: Kernel Image size: %lu bytes
>> > bootmodule size: %lu bytes\n",
>> +                   zimage.image_size, (uint64_t)size);
>> +            printk(XENLOG_ERR "The field 'size' does not match the
>> size of blob!\n");
>> +        }
>>           return -EINVAL;
>> +    }
>>
>>       info->zimage.kernel_addr = addr;
>>       info->zimage.len = end - start;


Best regards

Dirk

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

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

* Re: [PATCH v3] xen: arm: Update arm64 image header
  2016-06-26  5:47   ` Dirk Behme
@ 2016-06-26  8:29     ` Julien Grall
  2016-06-26  9:30       ` Dirk Behme
  0 siblings, 1 reply; 6+ messages in thread
From: Julien Grall @ 2016-06-26  8:29 UTC (permalink / raw)
  To: Dirk Behme, Julien Grall; +Cc: xen-devel, Dirk Behme, Stefano Stabellini

Hello Dirk,

On 26/06/2016 06:47, Dirk Behme wrote:
> On 23.06.2016 17:18, Julien Grall wrote:
>> On 23/06/16 07:38, Dirk Behme wrote:
>>> +        uint64_t res2;
>>>           uint64_t res3;
>>>           uint64_t res4;
>>> -        uint64_t res5;
>>> -        uint32_t magic1;
>>> -        uint32_t res6;
>>> +        uint32_t magic;        /* Magic number, little endian,
>>> "ARM\x64" */
>>> +        uint32_t res5;
>>>       } zimage;
>>>       uint64_t start, end;
>>>
>>> @@ -354,20 +353,30 @@ static int kernel_zimage64_probe(struct
>>> kernel_info *info,
>>>
>>>       copy_from_paddr(&zimage, addr, sizeof(zimage));
>>>
>>> -    if ( zimage.magic0 != ZIMAGE64_MAGIC_V0 &&
>>> -         zimage.magic1 != ZIMAGE64_MAGIC_V1 )
>>> +    if ( zimage.magic != ZIMAGE64_MAGIC ) {
>>> +        printk(XENLOG_ERR "No valid magic found in header! Kernel
>>> too old?\n");
>>
>> I have found why there were no error messages here before. The
>> function kernel_probe will try the different formats supported one by
>> one.
>>
>> So this message will be printed if the kernel is an ARM32 image, which
>> will confuse the user. So I would print this message only when
>> zimage.magic0 is equal to ZIMAGE64_MAGIC_V0.
>
>
> Which we don't have with the recent header format any more.

Well, we control the structure in Xen. So we could re-introduce the 
field magic0 through an union.

 > This does
> mean I drop this message again, as it doesn't make sense if the magic is
> used for the format detection.

I would still prefer to keep an error message when only MAGIC_V0 is 
present. This will avoid people to spend time understanding why it does 
not work anymore.

Regards,

-- 
Julien Grall

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

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

* Re: [PATCH v3] xen: arm: Update arm64 image header
  2016-06-26  8:29     ` Julien Grall
@ 2016-06-26  9:30       ` Dirk Behme
  2016-06-26 19:39         ` Julien Grall
  0 siblings, 1 reply; 6+ messages in thread
From: Dirk Behme @ 2016-06-26  9:30 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, Dirk Behme, Stefano Stabellini

Hi Julien,

On 26.06.2016 10:29, Julien Grall wrote:
> Hello Dirk,
>
> On 26/06/2016 06:47, Dirk Behme wrote:
>> On 23.06.2016 17:18, Julien Grall wrote:
>>> On 23/06/16 07:38, Dirk Behme wrote:
>>>> +        uint64_t res2;
>>>>           uint64_t res3;
>>>>           uint64_t res4;
>>>> -        uint64_t res5;
>>>> -        uint32_t magic1;
>>>> -        uint32_t res6;
>>>> +        uint32_t magic;        /* Magic number, little endian,
>>>> "ARM\x64" */
>>>> +        uint32_t res5;
>>>>       } zimage;
>>>>       uint64_t start, end;
>>>>
>>>> @@ -354,20 +353,30 @@ static int kernel_zimage64_probe(struct
>>>> kernel_info *info,
>>>>
>>>>       copy_from_paddr(&zimage, addr, sizeof(zimage));
>>>>
>>>> -    if ( zimage.magic0 != ZIMAGE64_MAGIC_V0 &&
>>>> -         zimage.magic1 != ZIMAGE64_MAGIC_V1 )
>>>> +    if ( zimage.magic != ZIMAGE64_MAGIC ) {
>>>> +        printk(XENLOG_ERR "No valid magic found in header! Kernel
>>>> too old?\n");
>>>
>>> I have found why there were no error messages here before. The
>>> function kernel_probe will try the different formats supported one by
>>> one.
>>>
>>> So this message will be printed if the kernel is an ARM32 image, which
>>> will confuse the user. So I would print this message only when
>>> zimage.magic0 is equal to ZIMAGE64_MAGIC_V0.
>>
>>
>> Which we don't have with the recent header format any more.
>
> Well, we control the structure in Xen. So we could re-introduce the
> field magic0 through an union.
>
>  > This does
>> mean I drop this message again, as it doesn't make sense if the
>> magic is
>> used for the format detection.
>
> I would still prefer to keep an error message when only MAGIC_V0 is
> present. This will avoid people to spend time understanding why it
> does not work anymore.


This way

if ( zimage.magic != ZIMAGE64_MAGIC ) {
       if ( zimage.magic0 == ZIMAGE64_MAGIC_V0 )
             printk(XENLOG_ERR "No valid magic found in header! Kernel 
too old?\n");
       return -EINVAL;
}

with magic0 being a union with code0?

Best regards

Dirk


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

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

* Re: [PATCH v3] xen: arm: Update arm64 image header
  2016-06-26  9:30       ` Dirk Behme
@ 2016-06-26 19:39         ` Julien Grall
  0 siblings, 0 replies; 6+ messages in thread
From: Julien Grall @ 2016-06-26 19:39 UTC (permalink / raw)
  To: Dirk Behme; +Cc: xen-devel, Dirk Behme, Stefano Stabellini



On 26/06/2016 10:30, Dirk Behme wrote:
> Hi Julien,

Hi Dirk,

> On 26.06.2016 10:29, Julien Grall wrote:
>> Hello Dirk,
>>
>> On 26/06/2016 06:47, Dirk Behme wrote:
>>> On 23.06.2016 17:18, Julien Grall wrote:
>>>> On 23/06/16 07:38, Dirk Behme wrote:
>>>>> +        uint64_t res2;
>>>>>           uint64_t res3;
>>>>>           uint64_t res4;
>>>>> -        uint64_t res5;
>>>>> -        uint32_t magic1;
>>>>> -        uint32_t res6;
>>>>> +        uint32_t magic;        /* Magic number, little endian,
>>>>> "ARM\x64" */
>>>>> +        uint32_t res5;
>>>>>       } zimage;
>>>>>       uint64_t start, end;
>>>>>
>>>>> @@ -354,20 +353,30 @@ static int kernel_zimage64_probe(struct
>>>>> kernel_info *info,
>>>>>
>>>>>       copy_from_paddr(&zimage, addr, sizeof(zimage));
>>>>>
>>>>> -    if ( zimage.magic0 != ZIMAGE64_MAGIC_V0 &&
>>>>> -         zimage.magic1 != ZIMAGE64_MAGIC_V1 )
>>>>> +    if ( zimage.magic != ZIMAGE64_MAGIC ) {
>>>>> +        printk(XENLOG_ERR "No valid magic found in header! Kernel
>>>>> too old?\n");
>>>>
>>>> I have found why there were no error messages here before. The
>>>> function kernel_probe will try the different formats supported one by
>>>> one.
>>>>
>>>> So this message will be printed if the kernel is an ARM32 image, which
>>>> will confuse the user. So I would print this message only when
>>>> zimage.magic0 is equal to ZIMAGE64_MAGIC_V0.
>>>
>>>
>>> Which we don't have with the recent header format any more.
>>
>> Well, we control the structure in Xen. So we could re-introduce the
>> field magic0 through an union.
>>
>>  > This does
>>> mean I drop this message again, as it doesn't make sense if the
>>> magic is
>>> used for the format detection.
>>
>> I would still prefer to keep an error message when only MAGIC_V0 is
>> present. This will avoid people to spend time understanding why it
>> does not work anymore.
>
>
> This way
>
> if ( zimage.magic != ZIMAGE64_MAGIC ) {
>       if ( zimage.magic0 == ZIMAGE64_MAGIC_V0 )
>             printk(XENLOG_ERR "No valid magic found in header! Kernel
> too old?\n");
>       return -EINVAL;
> }
>
> with magic0 being a union with code0?

Yes, although I would drop the question marks at the end of the second 
sentence. We know that the kernel is too old.

Thank you for doing this.

Regards,

-- 
Julien Grall

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

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

end of thread, other threads:[~2016-06-26 19:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-23  6:38 [PATCH v3] xen: arm: Update arm64 image header Dirk Behme
2016-06-23 15:18 ` Julien Grall
2016-06-26  5:47   ` Dirk Behme
2016-06-26  8:29     ` Julien Grall
2016-06-26  9:30       ` Dirk Behme
2016-06-26 19:39         ` Julien Grall

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).