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

[Robert Millan]

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

> 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  <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);
>       }
>        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?

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)