On Tue, Oct 19, 2021 at 04:37:03PM -0500, Glenn Washburn wrote:
> On Tue, 12 Oct 2021 18:30:01 +1100
> Daniel Axtens <d...@axtens.net> wrote:
>
> > From: Patrick Steinhardt <p...@pks.im>
> >
> > The function `add_memory_regions ()` is currently only called on system
> > initialization to allocate a fixed amount of pages. As such, it didn't
> > need to return any errors: in case it failed, we cannot proceed anyway.
> > This will change with the upcoming support for requesting more memory
> > from the firmware at runtime, where it doesn't make sense anymore to
> > fail hard.
> >
> > Refactor the function to return an error to prepare for this. Note that
> > this does not change the behaviour when initializing the memory system
> > because `grub_efi_mm_init ()` knows to call `grub_fatal ()` in case
> > `grub_efi_mm_add_regions ()` returns an error.
> >
> > Signed-off-by: Patrick Steinhardt <p...@pks.im>
> > ---
> > grub-core/kern/efi/mm.c | 20 +++++++++++++-------
> > 1 file changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
> > index cfc6a67fc0cc..ced3ee5e7537 100644
> > --- a/grub-core/kern/efi/mm.c
> > +++ b/grub-core/kern/efi/mm.c
> > @@ -478,7 +478,7 @@ filter_memory_map (grub_efi_memory_descriptor_t
> > *memory_map,
> > }
> >
> > /* Add memory regions. */
> > -static void
> > +static grub_err_t
> > add_memory_regions (grub_efi_memory_descriptor_t *memory_map,
> > grub_efi_uintn_t desc_size,
> > grub_efi_memory_descriptor_t *memory_map_end,
> > @@ -506,9 +506,9 @@ add_memory_regions (grub_efi_memory_descriptor_t
> > *memory_map,
> > GRUB_EFI_ALLOCATE_ADDRESS,
> > GRUB_EFI_LOADER_CODE);
> > if (! addr)
> > - grub_fatal ("cannot allocate conventional memory %p with %u pages",
> > - (void *) ((grub_addr_t) start),
> > - (unsigned) pages);
> > + return grub_error (GRUB_ERR_OUT_OF_MEMORY,
> > + "cannot allocate conventional memory %p with %u
> > pages",
> > + (void *) ((grub_addr_t) start), (unsigned) pages);
>
> I wonder if we shouldn't print a debug message here and try the next
> descriptor. Is it possible that grub_efi_allocate_pages_real fails for
> the specified range, but a range further on will succeed? The fact that
> it hasn't mattered so far seems to indicate that either this case is
> very rarely encountered or we should not continue on a failure for some
> reason.
You allocate number of pages of a given UEFI memory region or less. So,
the grub_efi_allocate_pages_real() may fail only due to bug somewhere.
Then I do not think we should try next UEFI memory region.
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
