Re: [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading

From: Ard Biesheuvel
Date: Wed Apr 29 2020 - 15:06:06 EST

Next message: Matthias Kaehlcke: "Re: [PATCH v6 1/4] dt-bindings: phy: qcom,qmp: Convert QMP PHY bindings to yaml"
Previous message: Andrew F. Davis: "Re: [PATCH] misc: sram: Add dma-heap-export reserved SRAM area type"
In reply to: Arvind Sankar: "[PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading"
Next in thread: Arvind Sankar: "Re: [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, 29 Apr 2020 at 19:41, Arvind Sankar <nivedita@xxxxxxxxxxxx> wrote:
>
> Currently the EFI stub attempts to load initrd(s) specified on the
> command line below hdr->initrd_addr_max (2G) and if that fails, falls
> back to allocating at an unrestricted address.
>
> The only case when loading at a low address helps is for the 32-bit
> kernel, where the initrd must be copied by the kernel into lowmem if
> it's not there already. The limit specified in hdr->initrd_addr_max is
> insufficient to ensure this in any case, since lowmem by default will
> extend to about 0.9G rather than 2G, and we don't attempt to load the
> initrd in lowmem at all for the new device-path based initrd.
>
> Simplify the code by dropping this optimization for the command line
> initrd(s) as well.
>
> Signed-off-by: Arvind Sankar <nivedita@xxxxxxxxxxxx>

It is not really an optimization, unfortunately. Commit 47226ad4f4cfd
has the details, but in short, loading above 4 GB broke some
platforms, so below 4 GB had to remain the default.

This was 6 years ago, and so we might be able to revisit this, but
characterising it as a mere optimization is inaccurate.

> ---
> drivers/firmware/efi/libstub/efi-stub-helper.c | 14 +++++---------
> drivers/firmware/efi/libstub/efi-stub.c | 3 +--
> drivers/firmware/efi/libstub/efistub.h | 8 +++-----
> drivers/firmware/efi/libstub/file.c | 13 ++-----------
> drivers/firmware/efi/libstub/x86-stub.c | 3 +--
> 5 files changed, 12 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index 2c0c2c34b4cc..32768fa04b32 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -378,8 +378,7 @@ static
> efi_status_t efi_load_initrd_cmdline(efi_loaded_image_t *image,
> unsigned long *load_addr,
> unsigned long *load_size,
> - unsigned long soft_limit,
> - unsigned long hard_limit)
> + unsigned long max)
> {
> if (!IS_ENABLED(CONFIG_EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER) ||
> (IS_ENABLED(CONFIG_X86) && (!efi_is_native() || image == NULL))) {
> @@ -388,27 +387,24 @@ efi_status_t efi_load_initrd_cmdline(efi_loaded_image_t *image,
> }
>
> return handle_cmdline_files(image, L"initrd=", sizeof(L"initrd=") - 2,
> - soft_limit, hard_limit,
> - load_addr, load_size);
> + max, load_addr, load_size);
> }
>
> efi_status_t efi_load_initrd(efi_loaded_image_t *image,
> unsigned long *load_addr,
> unsigned long *load_size,
> - unsigned long soft_limit,
> - unsigned long hard_limit)
> + unsigned long max)
> {
> efi_status_t status;
>
> if (!load_addr || !load_size)
> return EFI_INVALID_PARAMETER;
>
> - status = efi_load_initrd_dev_path(load_addr, load_size, hard_limit);
> + status = efi_load_initrd_dev_path(load_addr, load_size, max);
> if (status == EFI_SUCCESS) {
> pr_efi("Loaded initrd from LINUX_EFI_INITRD_MEDIA_GUID device path\n");
> } else if (status == EFI_NOT_FOUND) {
> - status = efi_load_initrd_cmdline(image, load_addr, load_size,
> - soft_limit, hard_limit);
> + status = efi_load_initrd_cmdline(image, load_addr, load_size, max);
> if (status == EFI_SUCCESS && *load_size > 0)
> pr_efi("Loaded initrd from command line option\n");
> }
> diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
> index d8f24f5c91bd..930302d9415a 100644
> --- a/drivers/firmware/efi/libstub/efi-stub.c
> +++ b/drivers/firmware/efi/libstub/efi-stub.c
> @@ -265,8 +265,7 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
>
> if (!efi_noinitrd) {
> max_addr = efi_get_max_initrd_addr(dram_base, image_addr);
> - status = efi_load_initrd(image, &initrd_addr, &initrd_size,
> - ULONG_MAX, max_addr);
> + status = efi_load_initrd(image, &initrd_addr, &initrd_size, max_addr);
> if (status != EFI_SUCCESS)
> pr_efi_err("Failed to load initrd!\n");
> }
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index dfdd7954bf58..1ba0887818d9 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -663,8 +663,7 @@ efi_status_t efi_setup_gop(struct screen_info *si, efi_guid_t *proto,
> efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
> const efi_char16_t *optstr,
> int optstr_size,
> - unsigned long soft_limit,
> - unsigned long hard_limit,
> + unsigned long max,
> unsigned long *load_addr,
> unsigned long *load_size);
>
> @@ -674,13 +673,12 @@ static inline efi_status_t efi_load_dtb(efi_loaded_image_t *image,
> unsigned long *load_size)
> {
> return handle_cmdline_files(image, L"dtb=", sizeof(L"dtb=") - 2,
> - ULONG_MAX, ULONG_MAX, load_addr, load_size);
> + ULONG_MAX, load_addr, load_size);
> }
>
> efi_status_t efi_load_initrd(efi_loaded_image_t *image,
> unsigned long *load_addr,
> unsigned long *load_size,
> - unsigned long soft_limit,
> - unsigned long hard_limit);
> + unsigned long max);
>
> #endif
> diff --git a/drivers/firmware/efi/libstub/file.c b/drivers/firmware/efi/libstub/file.c
> index 50aaf15f9ad5..7dee3c5d81fb 100644
> --- a/drivers/firmware/efi/libstub/file.c
> +++ b/drivers/firmware/efi/libstub/file.c
> @@ -124,8 +124,7 @@ static int find_file_option(const efi_char16_t *cmdline, int cmdline_len,
> efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
> const efi_char16_t *optstr,
> int optstr_size,
> - unsigned long soft_limit,
> - unsigned long hard_limit,
> + unsigned long max,
> unsigned long *load_addr,
> unsigned long *load_size)
> {
> @@ -181,15 +180,7 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t *image,
> round_up(alloc_size, EFI_ALLOC_ALIGN)) {
> unsigned long old_addr = alloc_addr;
>
> - status = EFI_OUT_OF_RESOURCES;
> - if (soft_limit < hard_limit)
> - status = efi_allocate_pages(alloc_size + size,
> - &alloc_addr,
> - soft_limit);
> - if (status == EFI_OUT_OF_RESOURCES)
> - status = efi_allocate_pages(alloc_size + size,
> - &alloc_addr,
> - hard_limit);
> + status = efi_allocate_pages(alloc_size + size, &alloc_addr, max);
> if (status != EFI_SUCCESS) {
> pr_efi_err("Failed to allocate memory for files\n");
> goto err_close_file;
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index 1d3f94f1dafa..85a924fecc87 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -755,8 +755,7 @@ unsigned long efi_main(efi_handle_t handle,
> if (!efi_noinitrd) {
> unsigned long addr, size;
>
> - status = efi_load_initrd(image, &addr, &size,
> - hdr->initrd_addr_max, ULONG_MAX);
> + status = efi_load_initrd(image, &addr, &size, ULONG_MAX);
>
> if (status != EFI_SUCCESS) {
> pr_efi_err("Failed to load initrd!\n");
> --
> 2.26.2
>

Next message: Matthias Kaehlcke: "Re: [PATCH v6 1/4] dt-bindings: phy: qcom,qmp: Convert QMP PHY bindings to yaml"
Previous message: Andrew F. Davis: "Re: [PATCH] misc: sram: Add dma-heap-export reserved SRAM area type"
In reply to: Arvind Sankar: "[PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading"
Next in thread: Arvind Sankar: "Re: [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]