Re: [Qemu-devel] [PATCH] loader: pad kernel size when loaded from a uImage

[Edgar E. Iglesias]

On Sat, Jul 31, 2010 at 12:56:42AM +0200, Edgar E. Iglesias wrote:
> On Thu, Jul 29, 2010 at 06:48:24PM -0700, Hollis Blanchard wrote:
> > The kernel's BSS size is lost by mkimage, which only considers file
> > size. As a result, loading other blobs (e.g. device tree, initrd)
> > immediately after the kernel location can result in them being zeroed by
> > the kernel's BSS initialization code.
> > 
> > Signed-off-by: Hollis Blanchard <address@hidden>
> > ---
> >  hw/loader.c |    7 +++++++
> >  1 files changed, 7 insertions(+), 0 deletions(-)
> > 
> > diff --git a/hw/loader.c b/hw/loader.c
> > index 79a6f95..35bc25a 100644
> > --- a/hw/loader.c
> > +++ b/hw/loader.c
> > @@ -507,6 +507,13 @@ int load_uimage(const char *filename, 
> > target_phys_addr_t *ep,
> >  
> >      ret = hdr->ih_size;
> >  
> > +   /* The kernel's BSS size is lost by mkimage, which only considers file
> > +    * size. We don't know how big it is, but we do know we can't place
> > +    * anything immediately after the kernel. The padding seems like it 
> > should
> > +    * be proportional to overall file size, but we also make sure it's at
> > +    * least 4-byte aligned. */
> > +   ret += (hdr->ih_size / 16) & ~0x3;
> 
> Maybe it's only me, but it feels a bit akward to push down this kind of
> knowledge down the abstraction layers. Does it work for you to have your
> caller of load_uimage apply whatever resizing magic needed for your kernel
> and arch?


Hi Hollis,

Sorry I was a bit in a hurry and short last time. And sorry for the bad
wording, I thought awkward simply meant wrong (english is not my native
languauge).

Ayway, IMO the conventions of where to pass blobs from the bootloader to the
loaded image are an agreement between the bootloader and the loaded code. The
formats or mechanisms to load the image should need to be involved that much.

For example in this particular case, other archs (e.g, MicroBlaze) might not
need any magic. The MicroBlaze linux kernel moves cmdline and device tree blobs
into safe areas prior to .bss initialization.

That's why I think that these kind of decisions should be made higher up.

Thanks and sorry for my clumsy wording last time :)
Edgar