[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