Re: [PATCH 11/12] efi/loongarch: libstub: remove dependency on flattened DT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Ard,

I think the parameters passed to the core kernel need to be discussed.
The old way (so-called old world):
a0=argc, a1=argv, a1=bpi

The current way (so-called new world):
a0=efi boot flag, a1=fdt pointer

The new way (in this patchset):
a0=efi boot flag, a1=systemtable, a2=cmdline

I prefer to use the current way, there are 3 reasons:
1, both acpi system and dt system can use the same parameters passing method;
2, arm64, riscv and loongarch can use similar logics (light FDT);
3, out-of-tree patches can make compatibility with the old world
easier by just judging whether a2 is zero.

Huacai

On Mon, Sep 19, 2022 at 5:36 AM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
>
> LoongArch does not use FDT or DT natively [yet], and the only reason it
> currently uses it is so that it can reuse the existing EFI stub code.
>
> Overloading the DT with data passed between the EFI stub and the core
> kernel has been a source of problems: there is the overlap between
> information provided by EFI which DT can also provide (initrd base/size,
> command line, memory descriptions), requiring us to reason about which
> is which and what to prioritize. It has also resulted in ABI leaks,
> i.e., internal ABI being promoted to external ABI inadvertently because
> the bootloader can set the EFI stub's DT properties as well (e.g.,
> "kaslr-seed"). This has become especially problematic with boot
> environments that want to pretend that EFI boot is being done (to access
> ACPI and SMBIOS tables, for instance) but have no ability to execute the
> EFI stub, and so the environment that the EFI stub creates is emulated
> [poorly, in some cases].
>
> Another downside of treating DT like this is that the DT binary that the
> kernel receives is different from the one created by the firmware, which
> is undesirable in the context of secure and measured boot.
>
> Given that LoongArch support in Linux is brand new, we can avoid these
> pitfalls, and treat the DT strictly as a hardware description, and use a
> separate handover method between the EFI stub and the kernel. Now that
> initrd loading and passing the EFI memory map have been refactored into
> pure EFI routines that use EFI configuration tables, the only thing we
> need to pass directly is the kernel command line (even if we could pass
> this via a config table as well, it is used extremely early, so passing
> it directly is preferred in this case.)
>
> Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> ---
>  arch/loongarch/Kconfig                        |  3 --
>  arch/loongarch/include/asm/bootinfo.h         |  2 +-
>  arch/loongarch/kernel/efi.c                   | 30 ++++++++++-
>  arch/loongarch/kernel/env.c                   | 22 ++++----
>  arch/loongarch/kernel/head.S                  |  2 +
>  arch/loongarch/kernel/setup.c                 |  4 +-
>  drivers/firmware/efi/libstub/Makefile         | 13 +++--
>  drivers/firmware/efi/libstub/loongarch-stub.c | 56 +++++++++++++++++---
>  8 files changed, 100 insertions(+), 32 deletions(-)
>
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index fca106a8b8af..14a2a1ec8561 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -104,8 +104,6 @@ config LOONGARCH
>         select MODULES_USE_ELF_RELA if MODULES
>         select NEED_PER_CPU_EMBED_FIRST_CHUNK
>         select NEED_PER_CPU_PAGE_FIRST_CHUNK
> -       select OF
> -       select OF_EARLY_FLATTREE
>         select PCI
>         select PCI_DOMAINS_GENERIC
>         select PCI_ECAM if ACPI
> @@ -311,7 +309,6 @@ config DMI
>  config EFI
>         bool "EFI runtime service support"
>         select UCS2_STRING
> -       select EFI_PARAMS_FROM_FDT
>         select EFI_RUNTIME_WRAPPERS
>         help
>           This enables the kernel to use EFI runtime services that are
> diff --git a/arch/loongarch/include/asm/bootinfo.h b/arch/loongarch/include/asm/bootinfo.h
> index e02ac4af7f6e..8e5881bc5ad1 100644
> --- a/arch/loongarch/include/asm/bootinfo.h
> +++ b/arch/loongarch/include/asm/bootinfo.h
> @@ -36,7 +36,7 @@ struct loongson_system_configuration {
>  };
>
>  extern u64 efi_system_table;
> -extern unsigned long fw_arg0, fw_arg1;
> +extern unsigned long fw_arg0, fw_arg1, fw_arg2;
>  extern struct loongson_board_info b_info;
>  extern struct loongson_system_configuration loongson_sysconf;
>
> diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
> index 1f1f755fb425..3b80675726ec 100644
> --- a/arch/loongarch/kernel/efi.c
> +++ b/arch/loongarch/kernel/efi.c
> @@ -27,8 +27,13 @@
>  static unsigned long efi_nr_tables;
>  static unsigned long efi_config_table;
>
> +static unsigned long __initdata boot_memmap = EFI_INVALID_TABLE_ADDR;
> +
>  static efi_system_table_t *efi_systab;
> -static efi_config_table_type_t arch_tables[] __initdata = {{},};
> +static efi_config_table_type_t arch_tables[] __initdata = {
> +       {LINUX_EFI_BOOT_MEMMAP_GUID,    &boot_memmap,   "MEMMAP" },
> +       {},
> +};
>
>  void __init efi_runtime_init(void)
>  {
> @@ -61,6 +66,8 @@ void __init efi_init(void)
>                 return;
>         }
>
> +       efi_systab_report_header(&efi_systab->hdr, efi_systab->fw_vendor);
> +
>         set_bit(EFI_64BIT, &efi.flags);
>         efi_nr_tables    = efi_systab->nr_tables;
>         efi_config_table = (unsigned long)efi_systab->tables;
> @@ -72,4 +79,25 @@ void __init efi_init(void)
>
>         if (screen_info.orig_video_isVGA == VIDEO_TYPE_EFI)
>                 memblock_reserve(screen_info.lfb_base, screen_info.lfb_size);
> +
> +       if (boot_memmap != EFI_INVALID_TABLE_ADDR) {
> +               struct efi_memory_map_data data;
> +               struct efi_boot_memmap *tbl;
> +
> +               tbl = early_memremap_ro(boot_memmap, sizeof(*tbl));
> +               if (tbl) {
> +                       data.phys_map           = boot_memmap + sizeof(*tbl);
> +                       data.size               = tbl->map_size;
> +                       data.desc_size          = tbl->desc_size;
> +                       data.desc_version       = tbl->desc_ver;
> +
> +                       if (efi_memmap_init_early(&data) < 0)
> +                               panic("Unable to map EFI memory map.\n");
> +
> +                       memblock_reserve(data.phys_map & PAGE_MASK,
> +                                        PAGE_ALIGN(data.size + (data.phys_map & ~PAGE_MASK)));
> +
> +                       early_memunmap(tbl, sizeof(*tbl));
> +               }
> +       }
>  }
> diff --git a/arch/loongarch/kernel/env.c b/arch/loongarch/kernel/env.c
> index 82b478a5c665..05c38d28476e 100644
> --- a/arch/loongarch/kernel/env.c
> +++ b/arch/loongarch/kernel/env.c
> @@ -8,7 +8,6 @@
>  #include <linux/efi.h>
>  #include <linux/export.h>
>  #include <linux/memblock.h>
> -#include <linux/of_fdt.h>
>  #include <asm/early_ioremap.h>
>  #include <asm/bootinfo.h>
>  #include <asm/loongson.h>
> @@ -20,21 +19,18 @@ EXPORT_SYMBOL(loongson_sysconf);
>  void __init init_environ(void)
>  {
>         int efi_boot = fw_arg0;
> -       struct efi_memory_map_data data;
> -       void *fdt_ptr = early_memremap_ro(fw_arg1, SZ_64K);
>
> -       if (efi_boot)
> -               set_bit(EFI_BOOT, &efi.flags);
> -       else
> -               clear_bit(EFI_BOOT, &efi.flags);
> +       if (efi_boot) {
> +               char *cmdline = early_memremap_ro(fw_arg2, COMMAND_LINE_SIZE);
>
> -       early_init_dt_scan(fdt_ptr);
> -       early_init_fdt_reserve_self();
> -       efi_system_table = efi_get_fdt_params(&data);
> +               strscpy(boot_command_line, cmdline, COMMAND_LINE_SIZE);
> +               early_memunmap(cmdline, COMMAND_LINE_SIZE);
>
> -       efi_memmap_init_early(&data);
> -       memblock_reserve(data.phys_map & PAGE_MASK,
> -                        PAGE_ALIGN(data.size + (data.phys_map & ~PAGE_MASK)));
> +               efi_system_table = fw_arg1;
> +               set_bit(EFI_BOOT, &efi.flags);
> +       } else {
> +               clear_bit(EFI_BOOT, &efi.flags);
> +       }
>  }
>
>  static int __init init_cpu_fullname(void)
> diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S
> index 01bac62a6442..8f89f39fd31b 100644
> --- a/arch/loongarch/kernel/head.S
> +++ b/arch/loongarch/kernel/head.S
> @@ -67,6 +67,8 @@ SYM_CODE_START(kernel_entry)                  # kernel entry point
>         st.d            a0, t0, 0               # firmware arguments
>         la              t0, fw_arg1
>         st.d            a1, t0, 0
> +       la              t0, fw_arg2
> +       st.d            a2, t0, 0
>
>         /* KSave3 used for percpu base, initialized as 0 */
>         csrwr           zero, PERCPU_BASE_KS
> diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
> index e8714b1d94c8..7fabf2306e80 100644
> --- a/arch/loongarch/kernel/setup.c
> +++ b/arch/loongarch/kernel/setup.c
> @@ -51,7 +51,7 @@
>
>  struct screen_info screen_info __section(".data");
>
> -unsigned long fw_arg0, fw_arg1;
> +unsigned long fw_arg0, fw_arg1, fw_arg2;
>  DEFINE_PER_CPU(unsigned long, kernelsp);
>  struct cpuinfo_loongarch cpu_data[NR_CPUS] __read_mostly;
>
> @@ -187,7 +187,6 @@ early_param("mem", early_parse_mem);
>
>  void __init platform_init(void)
>  {
> -       efi_init();
>  #ifdef CONFIG_ACPI_TABLE_UPGRADE
>         acpi_table_upgrade();
>  #endif
> @@ -347,6 +346,7 @@ void __init setup_arch(char **cmdline_p)
>         *cmdline_p = boot_command_line;
>
>         init_environ();
> +       efi_init();
>         memblock_init();
>         parse_early_param();
>
> diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
> index 0dbc6d93f2e6..d8d6657e6277 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -29,7 +29,7 @@ cflags-$(CONFIG_RISCV)                := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \
>  cflags-$(CONFIG_LOONGARCH)     := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \
>                                    -fpie
>
> -cflags-$(CONFIG_EFI_GENERIC_STUB) += -I$(srctree)/scripts/dtc/libfdt
> +cflags-$(CONFIG_EFI_PARAMS_FROM_FDT)   += -I$(srctree)/scripts/dtc/libfdt
>
>  KBUILD_CFLAGS                  := $(cflags-y) -Os -DDISABLE_BRANCH_PROFILING \
>                                    -include $(srctree)/include/linux/hidden.h \
> @@ -60,14 +60,17 @@ lib-y                               := efi-stub-helper.o gop.o secureboot.o tpm.o \
>                                    alignedmem.o relocate.o vsprintf.o \
>                                    systable.o
>
> -# include the stub's generic dependencies from lib/ when building for ARM/arm64
> -efi-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> +# include the stub's libfdt dependencies from lib/ when needed
> +libfdt-deps                    := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c \
> +                                  fdt_empty_tree.c fdt_sw.c
> +
> +lib-$(CONFIG_EFI_PARAMS_FROM_FDT) += fdt.o \
> +                                    $(patsubst %.c,lib-%.o,$(libfdt-deps))
>
>  $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
>         $(call if_changed_rule,cc_o_c)
>
> -lib-$(CONFIG_EFI_GENERIC_STUB) += efi-stub.o fdt.o string.o intrinsics.o \
> -                                  $(patsubst %.c,lib-%.o,$(efi-deps-y))
> +lib-$(CONFIG_EFI_GENERIC_STUB) += efi-stub.o string.o intrinsics.o
>
>  lib-$(CONFIG_ARM)              += arm32-stub.o
>  lib-$(CONFIG_ARM64)            += arm64-stub.o
> diff --git a/drivers/firmware/efi/libstub/loongarch-stub.c b/drivers/firmware/efi/libstub/loongarch-stub.c
> index b7ef8d2df59e..7c684d10f936 100644
> --- a/drivers/firmware/efi/libstub/loongarch-stub.c
> +++ b/drivers/firmware/efi/libstub/loongarch-stub.c
> @@ -9,7 +9,8 @@
>  #include <asm/addrspace.h>
>  #include "efistub.h"
>
> -typedef void __noreturn (*kernel_entry_t)(bool efi, unsigned long fdt);
> +typedef void __noreturn (*kernel_entry_t)(bool efi, unsigned long systab,
> +                                         unsigned long cmdline);
>
>  extern int kernel_asize;
>  extern int kernel_fsize;
> @@ -42,19 +43,60 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
>         return status;
>  }
>
> -void __noreturn efi_enter_kernel(unsigned long entrypoint, unsigned long fdt, unsigned long fdt_size)
> +struct exit_boot_struct {
> +       efi_memory_desc_t       *runtime_map;
> +       int                     runtime_entry_count;
> +};
> +
> +static efi_status_t exit_boot_func(struct efi_boot_memmap *map, void *priv)
> +{
> +       struct exit_boot_struct *p = priv;
> +
> +       /*
> +        * Update the memory map with virtual addresses. The function will also
> +        * populate @runtime_map with copies of just the EFI_MEMORY_RUNTIME
> +        * entries so that we can pass it straight to SetVirtualAddressMap()
> +        */
> +       efi_get_virtmap(map->map, map->map_size, map->desc_size,
> +                       p->runtime_map, &p->runtime_entry_count);
> +
> +       return EFI_SUCCESS;
> +}
> +
> +efi_status_t efi_boot_kernel(void *handle, efi_loaded_image_t *image,
> +                            unsigned long image_addr, char *cmdline_ptr)
>  {
>         kernel_entry_t real_kernel_entry;
> +       struct exit_boot_struct priv;
> +       unsigned long desc_size;
> +       efi_status_t status;
> +       u32 desc_ver;
> +
> +       status = efi_alloc_virtmap(&priv.runtime_map, &desc_size, &desc_ver);
> +       if (status != EFI_SUCCESS) {
> +               efi_err("Unable to retrieve UEFI memory map.\n");
> +               return status;
> +       }
> +
> +       efi_info("Exiting boot services\n");
> +
> +       efi_novamap = false;
> +       status = efi_exit_boot_services(handle, &priv, exit_boot_func);
> +       if (status != EFI_SUCCESS)
> +               return status;
> +
> +       /* Install the new virtual address map */
> +       efi_rt_call(set_virtual_address_map,
> +                   priv.runtime_entry_count * desc_size, desc_size,
> +                   desc_ver, priv.runtime_map);
>
>         /* Config Direct Mapping */
>         csr_write64(CSR_DMW0_INIT, LOONGARCH_CSR_DMWIN0);
>         csr_write64(CSR_DMW1_INIT, LOONGARCH_CSR_DMWIN1);
>
>         real_kernel_entry = (kernel_entry_t)
> -               ((unsigned long)&kernel_entry - entrypoint + VMLINUX_LOAD_ADDRESS);
> +               ((unsigned long)&kernel_entry - image_addr + VMLINUX_LOAD_ADDRESS);
>
> -       if (!efi_novamap)
> -               real_kernel_entry(true, fdt);
> -       else
> -               real_kernel_entry(false, fdt);
> +       real_kernel_entry(true, (unsigned long)efi_system_table,
> +                         (unsigned long)cmdline_ptr);
>  }
> --
> 2.35.1
>
>



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux