On Mon, Aug 2, 2010 at 12:56 PM, Edgar E. Iglesias <edgar.igles...@gmail.com> wrote: > On Mon, Aug 02, 2010 at 12:33:54PM -0700, Hollis Blanchard wrote: >> >> You mean the one architecture, which by the way doesn't even use this >> API? That doesn't seem like a strong argument to me. Anyways, it's > > Are we looking at the same code?
I don't know. > Grep for load_uimage in qemu. 4 archs use it, PPC, ARM, m68k and MB. I see the following: 1 75 hw/an5206.c <<an5206_init>> kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL); 2 233 hw/arm_boot.c <<arm_load_kernel>> kernel_size = load_uimage(info->kernel_filename, &entry, NULL, 3 50 hw/dummy_m68k.c <<dummy_m68k_init>> kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL); 4 14 hw/loader.h <<uint64_t>> int load_uimage(const char *filename, target_phys_addr_t *ep, 5 277 hw/mcf5208.c <<mcf5208evb_init>> kernel_size = load_uimage(kernel_filename, &entry, NULL, NULL); 6 121 hw/ppc440_bamboo.c <<bamboo_init>> kernel_size = load_uimage(kernel...ename, &entry, &loadaddr, NULL); 7 235 hw/ppce500_mpc8544ds.c <<mpc8544ds_init>> kernel_size = load_uimage(kernel...ename, &entry, &loadaddr, NULL); That makes 2x ColdFire, ARM, M68K, 2x PowerPC. hw/petalogix_s3adsp1800_mmu.c is the only MicroBlaze I can see, and it only loads ELF and binary kernels, not uImages. > Of those archs, only 2 actually use the return value of load_uimage > to decide where to place blobs. PPC and MB. MB doesn't want any > magic applied to the return value. That leaves us with _ONE_ single > arch that needs magic that IMO is broken. You are trying to guess the > size of the loaded image's .bss area by adding a 16th of the uimage size? Accounting for BSS hardly counts as "magic", I think. :) >> just as much work to relocate an initrd from a padded address as it is >> from a closer address, so there is no downside. >> >> The fact remains that the current API is broken by design, or to be >> charitable "violates the principle of least surprise." With the >> exception of microblaze, everybody who calls load_uimage() must >> understand the nuances of the return value and adjust it (or ignore >> it) accordingly. Why wouldn't we consolidate that logic? > > I don't see how guessing that the .bss area is a 16th of the loaded > image makes this call any less confusing. I agree it's arbitrary, but it's only arbitrary in one place. It's also well-commented (IMHO), which is more than can be said for the current code. >> >> 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). >> >> Sorry, let me rephrase. At what address should I hardcode my initrd? >> What about my device tree? As a followup, why not lower, or higher? > > You should be putting them at the same addresses as uboot puts them. Fine. It's arbitrary in u-boot too, but at least it will be consistent. -Hollis -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html