Re: [bug #36532] boot in EFI mode (x86_64) fails on some systems

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 29.06.2012 22:49, address@hidden wrote:

>>>>> Vladimir,
>>>>>
>>>>> The 2.00rc1 version of grub2 still doesn't fix the efi memory map
>>>> buffer size I've been working on (though I can see you are now
>>>> allocating the efi memory map buffer very shortly before you are
>>>> calling grub_efi_finish_boot_services()).
>>>>>
>>>>> Increasing the mmap_size in find_efi_mmap_size()--as in the patch
>>>> immediately above this text--does fix the problem.  Even adding (2
>> <<
>>>> 12) (instead of (1 << 12)) to the mmap_size will work on the system
>>> I'm
>>>> testing with.
>>>>>
>>>>
>>>> I've changed it to 3. Thanks. It's annoying that even such simple
>>>> operations as we have between find_efi_mmap and finish drastically
>>>> increase memory map size.
>>>>
>>>> --
>>>> Regards
>>>> Vladimir 'φ-coder/phcoder' Serbinenko
>>>
>>> Thanks!  I completely agree that it is annoying.
>>>
>>
>> FYI, while grub-2.00 works on my system now, someone else here at Dell
>> has tested it and found that it still doesn't work.  (I verified that
>> they were indeed using version 2.00, and saw the error myself).
>>
>> I have no idea how or why the memory map size is growing that much.  As
>> I get time, I'll try to figure that out...
> 
> FYI again:
> 
> I've found the problem.  The memory map is not growing wildly... it is more 
> of a firmware "quirk."  The firmware is thinking it needs a larger buffer 
> than it actually needs.  So calls to GetMemoryMap with a buffer that is 
> smaller than (say) 52368 bytes will return EFI_BUFFER_TOO_SMALL and say that 
> the buffer needs to be 52368 bytes, but then when you call it with a buffer 
> that's 52368 bytes, it will put the memory map into the buffer and tell you 
> that the memory map is in fact only (say) 24576 bytes.
> 
> Unfortunately, right now, find_efi_mmap_size() will then return 24576 bytes 
> (plus 3 pages, aligned to a page size), which isn't enough, but not because 
> the memory map size is growing...
> 
> I'll send in a patch for grub that will make it immune to this quirk... it 
> should be pretty low risk (I'm thinking modify find_efi_mmap_size() to use 
> the value of mmap_size that was passed to grub_efi_get_memory_map(), rather 
> than the value of mmap_size that was returned from that function.)  With 
> that, I don't believe it would even need to add 3 pages to the memory map 
> size (I watched the memory map size, and it is not growing significantly).
> 

What about this:
=== modified file 'grub-core/loader/i386/linux.c'
--- grub-core/loader/i386/linux.c       2012-06-27 20:55:09 +0000
+++ grub-core/loader/i386/linux.c       2012-06-29 21:12:53 +0000
@@ -118,12 +118,13 @@ find_efi_mmap_size (void)
       int ret;
       grub_efi_memory_descriptor_t *mmap;
       grub_efi_uintn_t desc_size;
+      grub_efi_uintn_t cur_mmap_size = mmap_size;
 
-      mmap = grub_malloc (mmap_size);
+      mmap = grub_malloc (cur_mmap_size);
       if (! mmap)
        return 0;
 
-      ret = grub_efi_get_memory_map (&mmap_size, mmap, 0, &desc_size, 0);
+      ret = grub_efi_get_memory_map (&cur_mmap_size, mmap, 0, &desc_size, 0);
       grub_free (mmap);
 
       if (ret < 0)
@@ -134,6 +135,8 @@ find_efi_mmap_size (void)
       else if (ret > 0)
        break;
 
+      if (mmap_size < cur_mmap_size)
+       mmap_size = cur_mmap_size;
       mmap_size += (1 << 12);
     }
 

It will take the largest size returned which should be safe.

-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

signature.asc
Description: OpenPGP digital signature