From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1J9ktN-0003H8-LM for mharc-grub-devel@gnu.org; Tue, 01 Jan 2008 12:27:05 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1J9ktL-0003Ev-VE for grub-devel@gnu.org; Tue, 01 Jan 2008 12:27:04 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1J9ktL-0003Da-7s for grub-devel@gnu.org; Tue, 01 Jan 2008 12:27:03 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1J9ktL-0003DS-0P for grub-devel@gnu.org; Tue, 01 Jan 2008 12:27:03 -0500 Received: from mailout08.sul.t-online.de ([194.25.134.20] helo=mailout08.sul.t-online.com) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1J9ktK-0003gw-Gk for grub-devel@gnu.org; Tue, 01 Jan 2008 12:27:02 -0500 Received: from fwd35.aul.t-online.de by mailout08.sul.t-online.com with smtp id 1J9ktI-0002lG-00; Tue, 01 Jan 2008 18:27:00 +0100 Received: from [10.3.2.2] (VOzvFmZa8hN1CxyMNaAZHbAIR+BcCgruhD4fokI3xkePyYC4hiJEdinXJKQRkhZQD3@[217.235.229.48]) by fwd35.aul.t-online.de with esmtp id 1J9kt5-0jviU40; Tue, 1 Jan 2008 18:26:47 +0100 Message-ID: <477A77D9.9020607@t-online.de> Date: Tue, 01 Jan 2008 18:26:49 +0100 From: Christian Franke User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6) Gecko/20070802 SeaMonkey/1.1.4 MIME-Version: 1.0 To: The development of GRUB 2 References: <471E4628.9030706@t-online.de> <20071118112737.GB16674@thorin> <474202DA.4010001@t-online.de> <47790D50.4060000@t-online.de> <20080101111652.GA8608@thorin> In-Reply-To: <20080101111652.GA8608@thorin> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-ID: VOzvFmZa8hN1CxyMNaAZHbAIR+BcCgruhD4fokI3xkePyYC4hiJEdinXJKQRkhZQD3 X-TOI-MSGID: 2e3b4699-ee9b-40eb-88b6-35c2583dcfa8 X-detected-kernel: by monty-python.gnu.org: Linux 2.6 (newer, 3) Subject: Re: [PATCH] Fix eisa_mmap evaluation, add memory existence check X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Jan 2008 17:27:04 -0000 Robert Millan wrote: > On Mon, Dec 31, 2007 at 04:40:00PM +0100, Christian Franke wrote: > >> This version of the patch contains only the fix for the E801 EISA memory >> map. The memory existence check was helpful for testing but is not >> really necessary. >> > > I think the memory existence check might be good to have (I'd make sure it's > not cpu-specific and move it to kern/whatever.c, though). As for the rest.. > > Yes, good point. >> But this bug should be fixed, otherwise GRUB2 would crash if BIOS does >> not provide the E820 memory map. >> >> Christian >> >> >> 2007-12-31 Christian Franke >> >> * kern/i386/pc/init.c (grub_machine_init): Fix >> evaluation of eisa_mmap. >> > > I don't think I'm the most indicate to review this part of the patch, but > since nobody else does... > > Thanks! > Well you might find this funny, but try a google search for "e801 eisa" and > see what the first hit is. :-) > :-) > >> --- grub2.orig/kern/i386/pc/init.c 2007-10-22 22:22:51.359375000 +0200 >> +++ grub2/kern/i386/pc/init.c 2007-12-31 16:05:59.953125000 +0100 >> @@ -199,13 +199,8 @@ grub_machine_init (void) >> >> if (eisa_mmap) >> { >> - if ((eisa_mmap & 0xFFFF) == 0x3C00) >> - add_mem_region (0x100000, (eisa_mmap << 16) + 0x100000 * 15); >> - else >> - { >> - add_mem_region (0x100000, (eisa_mmap & 0xFFFF) << 10); >> - add_mem_region (0x1000000, eisa_mmap << 16); >> - } >> + add_mem_region (0x100000, (eisa_mmap & 0xFFFF) << 10); >> + add_mem_region (0x1000000, eisa_mmap & ~0xFFFF); >> } >> else >> add_mem_region (0x100000, grub_get_memsize (1) << 10); >> > > Ok, as it seems, this comes from: > > * grub_get_eisa_mmap() : return packed EISA memory map, lower 16 bits is > * memory between 1M and 16M in 1K parts, upper 16 bits is > * memory above 16M in 64K parts. If error, return zero. > > So the replacement of "eisa_mmap << 16" seems obviously correct, but the > "0x3C00" part you removed is completely misterious to me. Can you explain > what was it supposed to be doing or why you removed it? > > This part is intended to handle the (normal) case of one continuous region with not gap between 1M and 16M: (0x3C00 << 10) = 0x100000 * 15 = 15M But this part does not work due to the same bug. It is IMO not necessary to make this distinction. The function compact_mem_regions() called a few lines later joins the two regions anyway. Christian