Re: [PATCH 7/7] efi: change heap allocation type to GRUB_EFI_LOADER_CODE

[Daniel Kiper]

On Thu, Jul 27, 2017 at 03:45:48PM +0000, Vladimir 'phcoder' Serbinenko wrote:
> On Thu, Jul 27, 2017, 17:44 Daniel Kiper <address@hidden> wrote:
> > On Thu, Jul 27, 2017 at 03:33:06PM +0000, Vladimir 'phcoder' Serbinenko
> > wrote:
> > > On Thu, Jul 27, 2017, 17:30 Daniel Kiper <address@hidden> wrote:
> > > > On Mon, Jun 12, 2017 at 03:53:41PM +0100, Leif Lindholm wrote:
> > > > > With upcoming changes to EDK2, allocations of type EFI_LOADER_DATA
> > may
> > > > > not return regions with execute ability. Since modules are loaded
> > onto
> > > > > the heap, change the heap allocation type to GRUB_EFI_LOADER_CODE in
> > > > > order to permit execution on systems with this feature enabled.
> > > > >
> > > > > Closes: 50420
> > > > >
> > > > > Signed-off-by: Leif Lindholm <address@hidden>
> > > > > ---
> > > > >  grub-core/kern/efi/mm.c | 4 +++-
> > > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
> > > > > index 7b1763bc5..f27a48e68 100644
> > > > > --- a/grub-core/kern/efi/mm.c
> > > > > +++ b/grub-core/kern/efi/mm.c
> > > > > @@ -404,7 +404,9 @@ add_memory_regions (grub_efi_memory_descriptor_t
> > > > *memory_map,
> > > > >         pages = required_pages;
> > > > >       }
> > > > >
> > > > > -      addr = grub_efi_allocate_pages (start, pages);
> > > > > +      addr = grub_efi_allocate_pages_real (start, pages,
> > > > > +                                        GRUB_EFI_ALLOCATE_ADDRESS,
> > > > > +                                        GRUB_EFI_LOADER_CODE);
> > > >
> > > > Is it really needed? Is any module exectued in place or does it contain
> > > > code ready to run out of the box?
> > > >
> > > All the modules are loaded to heap
> >
> > I do not see why modules have to be loaded into memory region with
> > GRUB_EFI_LOADER_CODE type.
>
> He means grub modules not initrd or multiboot modules

Ahhh... Right, then it should be correct. Though I would double
check it applies to GRUB2 modules only.

Daniel