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

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

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