From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NKqsA-00084f-No for qemu-devel@nongnu.org; Wed, 16 Dec 2009 05:12:46 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NKqs6-00082L-NW for qemu-devel@nongnu.org; Wed, 16 Dec 2009 05:12:46 -0500 Received: from [199.232.76.173] (port=49897 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NKqs6-00082C-J7 for qemu-devel@nongnu.org; Wed, 16 Dec 2009 05:12:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50818) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NKqs5-00053G-Tn for qemu-devel@nongnu.org; Wed, 16 Dec 2009 05:12:42 -0500 Message-ID: <4B28B256.2020605@redhat.com> Date: Wed, 16 Dec 2009 11:11:34 +0100 From: Kevin Wolf MIME-Version: 1.0 Subject: Re: [Qemu-devel] [FOR 0.12][PATCH] Fix loading of ELF multiboot kernels References: <1259943565-10528-1-git-send-email-kwolf@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, agraf@suse.de Am 16.12.2009 10:51, schrieb Markus Armbruster: > 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? It hit me, so yes. Probably no widespread kernels as Alex' tests looked fine (not sure what he tests, probably Xen and his OS X bootloader?). In my case it was a simple test kernel. I'd expect the sum of unknown research or hobby kernels to be a major use case for Multiboot support, though. >> 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. The original code assumes that there is only one ROM that contains the entry point. In fact, each program header of an ELF file becomes it own ROM, though. So if you have a first PH which contains the entry point (or now the lowest loaded address) and a second PH at a higher address, this test would fail for the second one even though we need to load it. What we really need to test is if [addr, end] and [rom->min, rom->min + rom->romsize] have an intersection. This is what the following two ifs already check. Kevin