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

[Stuart_Hayes]

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

That works great.  Tested on a couple systems, including the one on which 
grub-2.00 failed.

Thanks!
Stuart