[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Fix eisa_mmap evaluation, add memory existence check

From: Christian Franke
Subject: Re: [PATCH] Fix eisa_mmap evaluation, add memory existence check
Date: Tue, 01 Jan 2008 18:26:49 +0100
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20070802 SeaMonkey/1.1.4

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.


2007-12-31  Christian Franke  <address@hidden>

        * 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...


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);
        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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]