On Sun, Aug 08, 2021 at 03:31:49PM +0200, Patrick Steinhardt wrote:
> 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.
>
> Signed-off-by: Patrick Steinhardt <p...@pks.im>
> ---
> grub-core/kern/efi/mm.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c
> index ec64c08c0..376af10af 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");
> +
> + return GRUB_ERR_NONE;
> }
>
> void
> @@ -565,6 +567,7 @@ grub_efi_mm_add_regions (grub_efi_uint64_t required_bytes)
> grub_efi_memory_descriptor_t *filtered_memory_map_end;
> grub_efi_uintn_t map_size;
> grub_efi_uintn_t desc_size;
> + grub_err_t err;
> int mm_status;
>
> /* Prepare a memory region to store two memory maps. */
> @@ -609,9 +612,11 @@ grub_efi_mm_add_regions (grub_efi_uint64_t
> required_bytes)
> sort_memory_map (filtered_memory_map, desc_size, filtered_memory_map_end);
>
> /* Allocate memory regions for GRUB's memory management. */
> - add_memory_regions (filtered_memory_map, desc_size,
> - filtered_memory_map_end,
> - BYTES_TO_PAGES (required_bytes));
> + err = add_memory_regions (filtered_memory_map, desc_size,
> + filtered_memory_map_end,
> + BYTES_TO_PAGES (required_bytes));
> + if (err != GRUB_ERR_NONE)
> + return err;
Should not you call grub_fatal() here? Otherwise you change memory init
behavior in this patch. I suppose this is not what we want...
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
