Le 10/11/2023 à 02:38, Daniel Walker a écrit :
> This adds code to handle the generic command line changes.
> The efi code appears that it doesn't benefit as much from this design
> as it could.
So what can we do to improve that ?
>
> For example, if you had a prepend command line with "nokaslr" then
> you might be helpful to re-enable it in the boot loader or dts,
s/you/it
> but there appears to be no way to re-enable kaslr or some of the
> other options.
>
> The efi command line handling is incorrect. x86 and arm have an append
> system however the efi code prepends the command line.
>
> For example, you could have a non-upgradable bios which sends
>
> efi=disable_early_pci_dma
>
> This hypothetically could have been set because early pci dma caused
> issues on early versions of the product.
>
> Then later the early pci dma was made to work and the company desired
> to start using it. To override the bios you could set the CONFIG_CMDLINE
> to,
>
> efi=no_disable_early_pci_dma
>
> then parsing would normally start with the bios command line, then move
> to the CONFIG_CMDLINE and you would end up with early pci dma turned on.
>
> however, current efi code keeps early pci dma off because the bios
> arguments always override the built in.
>
> Per my reading this is different from the main body of x86, arm, and
> arm64.
>
> The generic command line provides both append and prepend, so it
> alleviates this issue if it's used. However not all architectures use
> it.
>
> It would be desirable to allow the efi stub to have it's builtin command
> line to be modified after compile, but I don't see a feasible way to do
> that currently.
Would be desirable or is desirable ? Your explanations are unclear.
>
> Cc: xe-linux-external@xxxxxxxxx
> Signed-off-by: Daniel Walker <danielwa@xxxxxxxxx>
> ---
> .../firmware/efi/libstub/efi-stub-helper.c | 29 +++++++++++++++++++
> drivers/firmware/efi/libstub/efi-stub.c | 9 ++++++
> drivers/firmware/efi/libstub/efistub.h | 1 +
> drivers/firmware/efi/libstub/x86-stub.c | 14 +++++++--
> 4 files changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index bfa30625f5d0..952fa2cdff51 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -11,6 +11,7 @@
>
> #include <linux/efi.h>
> #include <linux/kernel.h>
> +#include <linux/cmdline.h>
> #include <asm/efi.h>
> #include <asm/setup.h>
>
> @@ -29,6 +30,34 @@ bool __pure __efi_soft_reserve_enabled(void)
> return !efi_nosoftreserve;
> }
>
> +/**
> + * efi_handle_cmdline() - handle adding in built-in parts of the command line
> + * @cmdline: kernel command line
> + *
> + * Add in the generic parts of the commandline and start the parsing of the
> + * command line.
> + *
> + * Return: status code
> + */
> +efi_status_t efi_handle_builtin_cmdline(char const *cmdline)
> +{
> + efi_status_t status = EFI_SUCCESS;
> +
> + if (sizeof(CMDLINE_STATIC_PREPEND) > 1)
> + status |= efi_parse_options(CMDLINE_STATIC_PREPEND);
> +
> + if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE))
> + status |= efi_parse_options(cmdline);
> +
> + if (sizeof(CMDLINE_STATIC_APPEND) > 1)
> + status |= efi_parse_options(CMDLINE_STATIC_APPEND);
> +
> + if (status != EFI_SUCCESS)
> + efi_err("Failed to parse options\n");
> +
> + return status;
> +}
> +
> /**
> * efi_parse_options() - Parse EFI command line options
> * @cmdline: kernel command line
> diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
> index f9c1e8a2bd1d..770abe95c0ee 100644
> --- a/drivers/firmware/efi/libstub/efi-stub.c
> +++ b/drivers/firmware/efi/libstub/efi-stub.c
> @@ -127,6 +127,14 @@ efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr)
> return EFI_OUT_OF_RESOURCES;
> }
>
> +#ifdef CONFIG_GENERIC_CMDLINE
> + status = efi_handle_builtin_cmdline(cmdline);
> + if (status != EFI_SUCCESS) {
> + goto fail_free_cmdline;
> + }
> +#endif
> +
> +#ifdef CONFIG_CMDLINE
> if (IS_ENABLED(CONFIG_CMDLINE_EXTEND) ||
> IS_ENABLED(CONFIG_CMDLINE_FORCE) ||
> cmdline_size == 0) {
> @@ -144,6 +152,7 @@ efi_status_t efi_handle_cmdline(efi_loaded_image_t *image, char **cmdline_ptr)
> goto fail_free_cmdline;
> }
> }
> +#endif
>
> *cmdline_ptr = cmdline;
> return EFI_SUCCESS;
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index 212687c30d79..1ac6631905c5 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -996,6 +996,7 @@ efi_status_t efi_relocate_kernel(unsigned long *image_addr,
> unsigned long alignment,
> unsigned long min_addr);
>
> +efi_status_t efi_handle_builtin_cmdline(char const *cmdline);
> efi_status_t efi_parse_options(char const *cmdline);
>
> void efi_parse_option_graphics(char *option);
> diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
> index 9d5df683f882..273a8a9c8bbb 100644
> --- a/drivers/firmware/efi/libstub/x86-stub.c
> +++ b/drivers/firmware/efi/libstub/x86-stub.c
> @@ -847,6 +847,8 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> struct setup_header *hdr = &boot_params->hdr;
> const struct linux_efi_initrd *initrd = NULL;
> unsigned long kernel_entry;
> + unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> + ((u64)boot_params->ext_cmd_line_ptr << 32));
> efi_status_t status;
>
> boot_params_pointer = boot_params;
> @@ -877,6 +879,14 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> goto fail;
> }
>
> +#ifdef CONFIG_GENERIC_CMDLINE
> + status = efi_handle_builtin_cmdline((char *)cmdline_paddr);
> + if (status != EFI_SUCCESS) {
> + efi_err("Failed to parse options\n");
> + goto fail;
> + }
> +#else /* CONFIG_GENERIC_CMDLINE */
> +
> #ifdef CONFIG_CMDLINE_BOOL
> status = efi_parse_options(CONFIG_CMDLINE);
> if (status != EFI_SUCCESS) {
> @@ -885,8 +895,6 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> }
> #endif
> if (!IS_ENABLED(CONFIG_CMDLINE_OVERRIDE)) {
> - unsigned long cmdline_paddr = ((u64)hdr->cmd_line_ptr |
> - ((u64)boot_params->ext_cmd_line_ptr << 32));
> status = efi_parse_options((char *)cmdline_paddr);
> if (status != EFI_SUCCESS) {
> efi_err("Failed to parse options\n");
> @@ -894,6 +902,8 @@ void __noreturn efi_stub_entry(efi_handle_t handle,
> }
> }
>
> +#endif
> +
> status = efi_decompress_kernel(&kernel_entry);
> if (status != EFI_SUCCESS) {
> efi_err("Failed to decompress kernel\n");
