grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 12/19] efi: mm: Pass up errors from `add_memory_regions ()`


From: Daniel Kiper
Subject: Re: [PATCH 12/19] efi: mm: Pass up errors from `add_memory_regions ()`
Date: Tue, 9 Nov 2021 17:10:02 +0100
User-agent: NeoMutt/20170113 (1.7.2)

On Tue, Oct 12, 2021 at 06:30:01PM +1100, Daniel Axtens wrote:
> From: Patrick Steinhardt <ps@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 <ps@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);
>
>        grub_mm_init_region (addr, PAGES_TO_BYTES (pages));
>
> @@ -518,7 +518,9 @@ add_memory_regions (grub_efi_memory_descriptor_t 
> *memory_map,
>      }
>
>    if (required_pages > 0)
> -    grub_fatal ("too little memory");
> +    return grub_error (GRUB_ERR_OUT_OF_MEMORY, "too little memory");

I agree with Glenn. The message is a bit imprecise here. Maybe we should
make more clear that the UEFI allocation somehow failed. I think I would
improve message above too.

Daniel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]