From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NKqXP-0007ee-3G for qemu-devel@nongnu.org; Wed, 16 Dec 2009 04:51:19 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NKqXJ-0007Yi-Vq for qemu-devel@nongnu.org; Wed, 16 Dec 2009 04:51:18 -0500 Received: from [199.232.76.173] (port=53497 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NKqXJ-0007Yc-Pg for qemu-devel@nongnu.org; Wed, 16 Dec 2009 04:51:13 -0500 Received: from mx1.redhat.com ([209.132.183.28]:4695) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NKqXJ-00033M-7Y for qemu-devel@nongnu.org; Wed, 16 Dec 2009 04:51:13 -0500 From: Markus Armbruster Subject: Re: [Qemu-devel] [FOR 0.12][PATCH] Fix loading of ELF multiboot kernels References: <1259943565-10528-1-git-send-email-kwolf@redhat.com> Date: Wed, 16 Dec 2009 10:51:08 +0100 In-Reply-To: <1259943565-10528-1-git-send-email-kwolf@redhat.com> (Kevin Wolf's message of "Fri, 4 Dec 2009 17:19:25 +0100") Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, agraf@suse.de Kevin Wolf writes: > The multiboot implementation assumed that there is only one program header > (which contains the entry point) and that the entry point is at the start of > the code. This doesn't hold true generally and caused too little data to be > loaded. Out of curiosity: does this affect images people actually use? Examples? > Fix the loading code to pass the whole loaded data to the Multiboot Option ROM. > > Signed-off-by: Kevin Wolf > --- > hw/loader.c | 2 -- > hw/pc.c | 10 ++++++---- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/hw/loader.c b/hw/loader.c > index 2d7a2c4..4c6981f 100644 > --- a/hw/loader.c > +++ b/hw/loader.c > @@ -718,8 +718,6 @@ int rom_copy(uint8_t *dest, target_phys_addr_t addr, size_t size) > QTAILQ_FOREACH(rom, &roms, next) { > if (rom->max) > continue; > - if (rom->min > addr) > - continue; > if (rom->min + rom->romsize < addr) > continue; > if (rom->min > end) I don't understand this hunk. > diff --git a/hw/pc.c b/hw/pc.c > index 8c1b7ea..fcebe3d 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -560,19 +560,21 @@ static int load_multiboot(void *fw_cfg, > } > if (!(flags & 0x00010000)) { /* MULTIBOOT_HEADER_HAS_ADDR */ > uint64_t elf_entry; > + uint64_t elf_low, elf_high; > int kernel_size; > fclose(f); > - kernel_size = load_elf(kernel_filename, 0, &elf_entry, NULL, NULL, > + kernel_size = load_elf(kernel_filename, 0, &elf_entry, &elf_low, &elf_high, > 0, ELF_MACHINE, 0); > if (kernel_size < 0) { > fprintf(stderr, "Error while loading elf kernel\n"); > exit(1); > } > - mh_load_addr = mh_entry_addr = elf_entry; > - mb_kernel_size = kernel_size; > + mh_load_addr = elf_low; > + mb_kernel_size = elf_high - elf_low; > + mh_entry_addr = elf_entry; > > mb_kernel_data = qemu_malloc(mb_kernel_size); > - if (rom_copy(mb_kernel_data, elf_entry, kernel_size) != kernel_size) { > + if (rom_copy(mb_kernel_data, mh_load_addr, mb_kernel_size) != mb_kernel_size) { > fprintf(stderr, "Error while fetching elf kernel from rom\n"); > exit(1); > } I get this part, and it looks good.