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

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

On 29/06/2016 12:08, Dirk Behme wrote:
> On 29.06.2016 12:32, Julien Grall wrote:
>> Hi Dirk,
>>
>> On 27/06/2016 08:53, Dirk Behme wrote:
>>> +    if ( (end - start) > 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;
>>> +    }
>>
>> This patch is breaking boot on any ARM64 platform (UEFI and
>> bootwrapper).
>
>
> Well, I wonder if it breaks because the kernel is too large? As intended?

Please read my previous e-mail, I gave an explanation why it breaks on 
UEFI/bootwrapper.

To summarize, the field 'image_size' is the total size of the kernel in 
memory BSS included. The section BSS is not included in the blob because 
it is all zeros and it is a waste of disk space.

>
> You are the expert on this, so I just can give my (limited?) understanding:
>
> In my use case, starting with the Xen development and not really knowing
> the details, taking a random example from the net I had configured 10MB
> in the device tree:
>
> module@0x48200000 {
>        compatible = "xen,linux-zimage", "xen,multiboot-module";
>        reg = <0x48200000 0x00A00000>; /* Max Image size 10MB */
> };
>
> This failed silently. No error message.
>
> Without knowing any details, my first workaround was to make the kernel
> smaller. Having a kernel Image smaller than 10MB worked, then.
>
> While debugging it, I found that these 0x00A00000 are used for 'size'.
> And increasing it to 0x00F00000 (15MB) does work for me, now.
>
> I don't know anything aobut UEFI and bootwrapper, but could you check
> the size given for 'size' and the real size of the kernel Image? What's
> about if you increase 'size'?

(XEN) Loading kernel from boot module @ 0000000080080000
(XEN) Error: Kernel Image size: 16482304 bytes > bootmodule size: 
15925760bytes
(XEN) The field 'size' does not match the size of blob!

For both UEFI and Bootwrapper, the bootmodule is created automatically 
and the size of the Image on the disk (e.g BSS not included) is used.

>
> Yes, this check is there just to avoid the silent failing I observed. If
> we have the error message, as I have implemented it, it would have saved
> some debugging time for me ;) So it's not about using the size for real
> loading, its just used for checking.
>
> A short term workaround would be to convert the ERR into WARN and remove
> the return.

This warning will always be printed for all the platform where the size 
is retrieved from the firmware (e.g UEFI, GRUB).

As mentioned in my previous mail, we should not copy more than the size 
of the bootmodule. Otherwise we may copy sensitive data in DOM0.

> It's somehow my feeling that there might be an issue regarding the sizes
> if the warning is there.

No, there is no issue. We misinterpreted the meaning of the field 
'image_size'. In the case of Xen, the size should only be used for 
placing the module.

Regards,

-- 
Julien Grall

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