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

[Dirk Behme <dirk.behme@xxxxxxxxxxxx>]

Hi Julien,

On 29.06.2016 13:33, Julien Grall wrote:
> 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.


Ok, yes, that answers my question from the previous mail. Thanks!


> > 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.

Yes.

Then the device tree case I like to cover is special.

Would it be the solution to add an additional

if ( device_tree )

and maybe convert it to a warning?

Best regards

Dirk

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