Re: [PATCH v2] xen: arm: Update arm64 image header

[Julien Grall <julien.grall@arm.com>]

Hello,

On 22/06/16 14:51, Konrad Rzeszutek Wilk wrote:
> On Wed, Jun 22, 2016 at 03:44:44PM +0200, Dirk Behme wrote:
>> On 22.06.2016 15:30, Konrad Rzeszutek Wilk wrote:
>>> On Wed, Jun 22, 2016 at 10:59:19AM +0200, 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
>>>
>>> Don't you want to still check for those kernels and use them?
>>
>>
>> Please check the _existing_ code: It's
>>
>>
>> if ( zimage.magic0 != ZIMAGE64_MAGIC_V0 &&
>>       zimage.magic1 != ZIMAGE64_MAGIC_V1 )
>>    return -EINVAL;

It is an "&&" not "||". So it will fail only if none of the 2 magics are 
present.

> Oh, indeed!
>>
>>
>> My patch doesn't change anything regarding the fact that if the magics are
>> not valid (due to quite old kernel version) the code does exit with an
>> error.
>
> /me nods
>>
>> While the review of v1 of this patch, Julien asked for an error message to
>> be added here. Fine.
>>
>> But if there is the request to change the behavior regarding which kernels
>> are supported I'd think that this is independent on this patch and should be
>> done in an additional patch.
>
> Right, and I think it is fine to skip that  - because as you say - it
> already ignores older kernels.

This is not right. The current version of Xen is not ignoring older kernels.

The current check allows kernels with only MAGIC_V0 (any Linux older 
than 3.12) to be detected by Xen.

However, this is impacting only one version because support for Xen on 
ARM64 was added in Linux 3.11.

Regards,

-- 
Julien Grall

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