From: Ard Biesheuvel <ardb@kernel.org>
To: Arvind Sankar <nivedita@alum.mit.edu>
Cc: linux-efi <linux-efi@vger.kernel.org>, X86 ML <x86@kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading
Date: Wed, 29 Apr 2020 21:05:51 +0200 [thread overview]
Message-ID: <CAMj1kXFe0eEHnijuB3OGM2moHw0xH5viFvCiQ6Y0cLG7s=iHfg@mail.gmail.com> (raw)
In-Reply-To: <20200429174120.1497212-10-nivedita@alum.mit.edu>
On Wed, 29 Apr 2020 at 19:41, Arvind Sankar <nivedita@alum.mit.edu> 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@alum.mit.edu>
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 prev parent reply other threads:[~2020-04-29 19:06 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-29 17:41 [PATCH 00/10] efi: some cleanups/refactoring for efi/next Arvind Sankar
2020-04-29 17:41 ` [PATCH 01/10] efi/x86: Use correct size for boot_params Arvind Sankar
2020-04-29 17:41 ` [PATCH 02/10] efi: Add a helper function to split 64-bit values Arvind Sankar
2020-04-29 23:51 ` kbuild test robot
2020-04-29 17:41 ` [PATCH 02/10] efi/libstub: " Arvind Sankar
2020-04-29 17:41 ` [PATCH 03/10] efi/x86: Use pr_efi_err for error messages Arvind Sankar
2020-04-29 18:47 ` Joe Perches
2020-04-29 18:49 ` Ard Biesheuvel
2020-04-29 18:57 ` Joe Perches
2020-04-29 18:59 ` Ard Biesheuvel
2020-04-29 19:47 ` Joe Perches
2020-04-29 19:48 ` Ard Biesheuvel
2020-04-29 21:43 ` Arvind Sankar
2020-04-29 21:45 ` Ard Biesheuvel
2020-04-29 21:51 ` Arvind Sankar
2020-04-29 21:53 ` Joe Perches
2020-04-29 21:55 ` Ard Biesheuvel
2020-04-29 22:20 ` Arvind Sankar
2020-04-30 15:14 ` Ard Biesheuvel
2020-04-29 17:41 ` [PATCH 04/10] efi/gop: " Arvind Sankar
2020-04-29 17:41 ` [PATCH 05/10] efi/tpm: " Arvind Sankar
2020-04-29 17:41 ` [PATCH 06/10] efi/x86: Move command-line initrd loading to efi_main Arvind Sankar
2020-04-29 17:41 ` [PATCH 07/10] efi/libstub: Unify initrd loading across architectures Arvind Sankar
2020-04-29 17:41 ` [PATCH 08/10] efi/x86: Drop soft_limit for x86 initrd loading Arvind Sankar
2020-04-29 19:05 ` Ard Biesheuvel [this message]
2020-04-29 21:33 ` Arvind Sankar
2020-04-29 17:41 ` [PATCH 09/10] efi/x86: Support builtin command line Arvind Sankar
2020-04-29 19:07 ` Ard Biesheuvel
2020-04-29 21:39 ` Arvind Sankar
2020-04-29 21:40 ` Ard Biesheuvel
2020-04-29 21:48 ` Arvind Sankar
2020-04-29 21:51 ` Ard Biesheuvel
2020-04-29 17:41 ` [PATCH 10/10] efi/libstub: Check return value of efi_parse_options Arvind Sankar
2020-04-30 4:31 ` kbuild test robot
2020-04-30 18:28 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Arvind Sankar
2020-04-30 18:28 ` [PATCH v2 01/11] efi/x86: Use correct size for boot_params Arvind Sankar
2020-04-30 18:28 ` [PATCH v2 02/11] efi/libstub: Add a helper function to split 64-bit values Arvind Sankar
2020-04-30 18:28 ` [PATCH v2 03/11] efi/libstub: Move pr_efi/pr_efi_err into efi namespace Arvind Sankar
2020-04-30 18:28 ` [PATCH v2 04/11] efi/x86: Use efi_err for error messages Arvind Sankar
2020-04-30 18:28 ` [PATCH v2 05/11] efi/gop: " Arvind Sankar
2020-04-30 18:28 ` [PATCH v2 06/11] efi/tpm: " Arvind Sankar
2020-04-30 18:28 ` [PATCH v2 07/11] efi/libstub: Upgrade ignored dtb= argument message to error Arvind Sankar
2020-04-30 18:28 ` [PATCH v2 08/11] efi/x86: Move command-line initrd loading to efi_main Arvind Sankar
2020-04-30 18:28 ` [PATCH v2 09/11] efi/libstub: Unify initrd loading across architectures Arvind Sankar
2020-04-30 18:28 ` [PATCH v2 10/11] efi/x86: Support builtin command line Arvind Sankar
2020-04-30 18:28 ` [PATCH v2 11/11] efi/libstub: Check return value of efi_parse_options Arvind Sankar
2020-04-30 19:12 ` [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening Joe Perches
2020-04-30 19:12 ` [PATCH 2/2] efi/libstub: Correct comment typos Joe Perches
2020-04-30 19:30 ` Ard Biesheuvel
2020-05-04 18:29 ` [trivial PATCH] efi/libstub: Reduce efi_printk object size Joe Perches
2020-05-05 7:50 ` Ard Biesheuvel
2020-05-05 8:01 ` Joe Perches
2020-04-30 19:29 ` [PATCH 1/2] efi/libstub: efi_info/efi_err message neatening Ard Biesheuvel
2020-04-30 19:38 ` Joe Perches
2020-04-30 20:40 ` Arvind Sankar
2020-04-30 20:42 ` Ard Biesheuvel
2020-05-04 8:17 ` [PATCH v2 00/11] efi: some cleanups/refactoring for efi/next Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMj1kXFe0eEHnijuB3OGM2moHw0xH5viFvCiQ6Y0cLG7s=iHfg@mail.gmail.com' \
--to=ardb@kernel.org \
--cc=linux-efi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nivedita@alum.mit.edu \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).