On Mon, Aug 02, 2010 at 10:59:11AM -0700, Hollis Blanchard wrote: > On Sun, Aug 1, 2010 at 5:36 AM, Edgar E. Iglesias > <edgar.igles...@gmail.com> wrote: > > 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 <hol...@penguinppc.org> > >> > --- > >> > 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? > > > > 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. > > Are you claiming that's the common case? FWIW, PowerPC and ARM don't > seem to. I wouldn't expect such logic except in reaction to a specific > bug (i.e. a qemu/firmware loader bug).
I'm not trying to claim it's the common case, but it exists. BTW, qemu-arm seems to follow a convention to place initrd 8Mb above RAM base, it doesn't look at the loaded uimage size when deciding where to place initrd. > The load_uimage() interface claims to report the size of the kernel it > loaded. If you argue that it shouldn't try to do that (and indeed you The way I understand it, it reports the size of what got loaded. It would be very difficult for load_uimage to figure out what memory areas are beeing touched prior to .bss init (or the point where the passed blob is used). > could argue it's not *possible* to do that accurately), that logic Right, its very hard for it to guess what memory areas are safe. > should be completely removed. The current behavior is worse than not > knowing at all: callers *think* they know, but it's guaranteed to be > wrong. > > Of course, if you do want to remove the size, then callers are left > with even less information than they had before. In that case, you I think returning the size of the loaded image has a value, for example for archs that move away the blobs before touching any memory outside their image. Bootloaders for those archs can put some blobs right after the loaded image. > tell me: where should I hardcode initrd loading? Not sure, but I'd guess somewhere close to where you are calling load_uimage from (it wasn't clear to me where that was). Take a look at how arm does it in hw/arm_boot.c. CRIS doesn't have uimage support now, but if it had It would probably do whatever magic was needed in it's dedicated boot loader file, hw/cris-boot.c. Microblaze has uimage support, look in hw/petalogix_s3adsp1800_mmu.c. Maybe we should consider adding a ppc-boot.c with boot-loader magics? Cheers, Edgar
