| Message ID | 20200415195422.19866-6-atish.patra@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add UEFI support for RISC-V | expand |
Hi Atish,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on efi/next]
[also build test WARNING on linus/master v5.7-rc1 next-20200415]
[cannot apply to arm/for-next arm64/for-next/core linux/master atish-riscv-linux/topo_v3]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Atish-Patra/Add-UEFI-support-for-RISC-V/20200416-065438
base: https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
config: riscv-rv32_defconfig (attached as .config)
compiler: riscv32-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from include/linux/list.h:9,
from include/linux/preempt.h:11,
from include/linux/spinlock.h:51,
from include/linux/seqlock.h:36,
from include/linux/time.h:6,
from include/linux/efi.h:17,
from drivers/firmware/efi/libstub/riscv-stub.c:10:
drivers/firmware/efi/libstub/riscv-stub.c: In function 'handle_kernel_image':
>> drivers/firmware/efi/libstub/riscv-stub.c:89:17: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
89 | if (IS_ALIGNED((u64)_start, MIN_KIMG_ALIGN)) {
| ^
include/linux/kernel.h:37:30: note: in definition of macro 'IS_ALIGNED'
37 | #define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
| ^
>> drivers/firmware/efi/libstub/riscv-stub.c:89:17: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
89 | if (IS_ALIGNED((u64)_start, MIN_KIMG_ALIGN)) {
| ^
include/linux/kernel.h:37:44: note: in definition of macro 'IS_ALIGNED'
37 | #define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
| ^
drivers/firmware/efi/libstub/riscv-stub.c:94:17: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
94 | *image_addr = (u64)_start;
| ^
vim +89 drivers/firmware/efi/libstub/riscv-stub.c
69
70 efi_status_t handle_kernel_image(unsigned long *image_addr,
71 unsigned long *image_size,
72 unsigned long *reserve_addr,
73 unsigned long *reserve_size,
74 unsigned long dram_base,
75 efi_loaded_image_t *image)
76 {
77 efi_status_t status;
78 unsigned long kernel_size, kernel_memsize = 0;
79 unsigned long max_alloc_address;
80
81 if (image->image_base != _start)
82 pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n");
83
84 kernel_size = _edata - _start;
85 kernel_memsize = kernel_size + (_end - _edata);
86 max_alloc_address = round_up(dram_base, MIN_KIMG_ALIGN) +
87 kernel_memsize;
88
> 89 if (IS_ALIGNED((u64)_start, MIN_KIMG_ALIGN)) {
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Wed, 15 Apr 2020 at 21:54, Atish Patra <atish.patra@wdc.com> wrote: > > Add a RISC-V architecture specific stub code that actually copies the > actual kernel image to a valid address and jump to it after boot services > are terminated. Enable UEFI related kernel configs as well for RISC-V. > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > --- > arch/riscv/Kconfig | 20 ++++ > arch/riscv/Makefile | 1 + > arch/riscv/configs/defconfig | 1 + > arch/riscv/include/asm/efi.h | 44 +++++++++ > drivers/firmware/efi/Kconfig | 2 +- > drivers/firmware/efi/libstub/Makefile | 7 ++ > drivers/firmware/efi/libstub/riscv-stub.c | 111 ++++++++++++++++++++++ > 7 files changed, 185 insertions(+), 1 deletion(-) > create mode 100644 arch/riscv/include/asm/efi.h > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index f39e326a7a42..eb4f41c8f3ce 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -379,10 +379,30 @@ config CMDLINE_FORCE > > endchoice > > +config EFI_STUB > + bool > + > +config EFI > + bool "UEFI runtime support" > + depends on OF > + select LIBFDT > + select UCS2_STRING > + select EFI_PARAMS_FROM_FDT > + select EFI_STUB > + select EFI_GENERIC_STUB > + default y > + help > + This option provides support for runtime services provided > + by UEFI firmware (such as non-volatile variables, realtime > + clock, and platform reset). A UEFI stub is also provided to > + allow the kernel to be booted as an EFI application. This > + is only useful on systems that have UEFI firmware. > + > endmenu > > menu "Power management options" > > source "kernel/power/Kconfig" > +source "drivers/firmware/Kconfig" > > endmenu > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > index fb6e37db836d..079435804d6d 100644 > --- a/arch/riscv/Makefile > +++ b/arch/riscv/Makefile > @@ -80,6 +80,7 @@ head-y := arch/riscv/kernel/head.o > core-y += arch/riscv/ > > libs-y += arch/riscv/lib/ > +core-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a > > PHONY += vdso_install > vdso_install: > diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig > index 4da4886246a4..ae69e12d306a 100644 > --- a/arch/riscv/configs/defconfig > +++ b/arch/riscv/configs/defconfig > @@ -129,3 +129,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > # CONFIG_RUNTIME_TESTING_MENU is not set > CONFIG_MEMTEST=y > # CONFIG_SYSFS_SYSCALL is not set > +CONFIG_EFI=y > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h > new file mode 100644 > index 000000000000..62d7d5eafed8 > --- /dev/null > +++ b/arch/riscv/include/asm/efi.h > @@ -0,0 +1,44 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2020 Western Digital Corporation or its affiliates. > + * Based on arch/arm64/include/asm/efi.h > + */ > +#ifndef _ASM_EFI_H > +#define _ASM_EFI_H > + > +#include <asm/io.h> > +#include <asm/mmu_context.h> > +#include <asm/ptrace.h> > +#include <asm/tlbflush.h> > + > +#define VA_BITS_MIN 39 > + > +/* on RISC-V, the FDT may be located anywhere in system RAM */ > +static inline unsigned long efi_get_max_fdt_addr(unsigned long dram_base) > +{ > + return ULONG_MAX; > +} > + > +/* Load initrd at enough distance from DRAM start */ > +static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base, > + unsigned long image_addr) > +{ > + return dram_base + SZ_256M; > +} > + > +#define efi_bs_call(func, ...) efi_system_table()->boottime->func(__VA_ARGS__) > +#define efi_rt_call(func, ...) efi_system_table()->runtime->func(__VA_ARGS__) > +#define efi_is_native() (true) > + > +#define efi_table_attr(inst, attr) (inst->attr) > + > +#define efi_call_proto(inst, func, ...) inst->func(inst, ##__VA_ARGS__) > + > +#define alloc_screen_info(x...) (&screen_info) > +extern char stext_offset[]; > + > +static inline void free_screen_info(struct screen_info *si) > +{ > +} > + > +#endif /* _ASM_EFI_H */ > diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig > index 2a2b2b96a1dc..fcdc789d3f87 100644 > --- a/drivers/firmware/efi/Kconfig > +++ b/drivers/firmware/efi/Kconfig > @@ -111,7 +111,7 @@ config EFI_GENERIC_STUB > > config EFI_ARMSTUB_DTB_LOADER > bool "Enable the DTB loader" > - depends on EFI_GENERIC_STUB > + depends on EFI_GENERIC_STUB && !RISCV > default y > help > Select this config option to add support for the dtb= command > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > index 2b4e09bf987c..7d46b70b51f2 100644 > --- a/drivers/firmware/efi/libstub/Makefile > +++ b/drivers/firmware/efi/libstub/Makefile > @@ -22,6 +22,8 @@ cflags-$(CONFIG_ARM64) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > cflags-$(CONFIG_ARM) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > -fno-builtin -fpic \ > $(call cc-option,-mno-single-pic-base) > +cflags-$(CONFIG_RISCV) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > + -fpic > > cflags-$(CONFIG_EFI_GENERIC_STUB) += -I$(srctree)/scripts/dtc/libfdt > > @@ -57,6 +59,7 @@ lib-$(CONFIG_EFI_GENERIC_STUB) += efi-stub.o fdt.o string.o \ > lib-$(CONFIG_ARM) += arm32-stub.o > lib-$(CONFIG_ARM64) += arm64-stub.o > lib-$(CONFIG_X86) += x86-stub.o > +lib-$(CONFIG_RISCV) += riscv-stub.o > CFLAGS_arm32-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > @@ -81,6 +84,10 @@ STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \ > --prefix-symbols=__efistub_ > STUBCOPY_RELOC-$(CONFIG_ARM64) := R_AARCH64_ABS > > +STUBCOPY_FLAGS-$(CONFIG_RISCV) += --prefix-alloc-sections=.init \ > + --prefix-symbols=__efistub_ > +STUBCOPY_RELOC-$(CONFIG_RISCV) := R_RISCV_HI20 > + > $(obj)/%.stub.o: $(obj)/%.o FORCE > $(call if_changed,stubcopy) > > diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c > new file mode 100644 > index 000000000000..69d13e0ebaea > --- /dev/null > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > @@ -0,0 +1,111 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2013, 2014 Linaro Ltd; <roy.franz@linaro.org> > + * Copyright (C) 2020 Western Digital Corporation or its affiliates. > + * > + * This file implements the EFI boot stub for the RISC-V kernel. > + * Adapted from ARM64 version at drivers/firmware/efi/libstub/arm64-stub.c. > + */ > + > +#include <linux/efi.h> > +#include <linux/libfdt.h> > +#include <linux/libfdt_env.h> > +#include <asm/efi.h> > +#include <asm/sections.h> > + > +#include "efistub.h" > +/* > + * RISCV requires the kernel image to placed TEXT_OFFSET bytes beyond a 2 MB > + * aligned base for 64 bit and 4MB for 32 bit. > + */ > +#ifdef CONFIG_64BIT > +#define MIN_KIMG_ALIGN SZ_2M > +#else > +#define MIN_KIMG_ALIGN SZ_4M > +#endif > + > +typedef __attribute__((noreturn)) void (*jump_kernel_func)(unsigned int, > + unsigned long); > +efi_status_t check_platform_features(void) > +{ > + return EFI_SUCCESS; > +} > + > +static u32 get_boot_hartid_from_fdt(unsigned long fdt) > +{ > + int chosen_node, len; > + const fdt32_t *prop; > + > + chosen_node = fdt_path_offset((void *)fdt, "/chosen"); > + if (chosen_node < 0) > + return U32_MAX; > + prop = fdt_getprop((void *)fdt, chosen_node, "boot-hartid", &len); > + if (!prop || len != sizeof(u32)) > + return U32_MAX; > + > + return fdt32_to_cpu(*prop); > +} > + > +/* > + * Jump to real kernel here with following constraints. > + * 1. MMU should be disabled. > + * 2. a0 should contain hartid > + * 3. a1 should DT address > + */ > +void __noreturn efi_enter_kernel(unsigned long entrypoint, unsigned long fdt, > + unsigned long fdt_size) > +{ > + unsigned long kernel_entry = entrypoint + (unsigned long)stext_offset; > + jump_kernel_func jump_kernel = (jump_kernel_func) kernel_entry; > + u32 hartid = get_boot_hartid_from_fdt(fdt); > + > + if (hartid == U32_MAX) > + /* We can not use panic or BUG at this point */ > + __asm__ __volatile__ ("ebreak"); > + /* Disable MMU */ > + csr_write(CSR_SATP, 0); > + jump_kernel(hartid, fdt); > +} > + > +efi_status_t handle_kernel_image(unsigned long *image_addr, > + unsigned long *image_size, > + unsigned long *reserve_addr, > + unsigned long *reserve_size, > + unsigned long dram_base, > + efi_loaded_image_t *image) > +{ > + efi_status_t status; > + unsigned long kernel_size, kernel_memsize = 0; > + unsigned long max_alloc_address; > + > + if (image->image_base != _start) > + pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n"); > + I don't think you need this. > + kernel_size = _edata - _start; > + kernel_memsize = kernel_size + (_end - _edata); > + max_alloc_address = round_up(dram_base, MIN_KIMG_ALIGN) + > + kernel_memsize; > + You said the kernel could be anywhere in memory, as long as it is aligned correctly, right? In that case, you don't need this, you can simply pass ULONG_MAX as the max address. > + if (IS_ALIGNED((u64)_start, MIN_KIMG_ALIGN)) { > + /* > + * Just execute from wherever we were loaded by the > + * UEFI PE/COFF loader if the alignment is suitable. > + */ > + *image_addr = (u64)_start; Change these casts to (unsigned long), as reported by the robot. So you no longer need the placement to be TEXT_OFFSET bytes past a MIN_KIMG_ALIGN aligned boundary, right? > + *reserve_size = 0; > + return EFI_SUCCESS; > + } > + status = efi_allocate_pages_aligned(*reserve_size, reserve_addr, > + max_alloc_address, MIN_KIMG_ALIGN); > + > + if (status != EFI_SUCCESS) { > + pr_efi_err("Failed to relocate kernel\n"); > + *reserve_size = 0; > + return status; > + } > + *image_addr = *reserve_addr; > + > + memcpy((void *)*image_addr, _start, kernel_size); > + > + return EFI_SUCCESS; > +} > -- > 2.24.0 >
On Thu, Apr 16, 2020 at 12:41 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Wed, 15 Apr 2020 at 21:54, Atish Patra <atish.patra@wdc.com> wrote: > > > > Add a RISC-V architecture specific stub code that actually copies the > > actual kernel image to a valid address and jump to it after boot services > > are terminated. Enable UEFI related kernel configs as well for RISC-V. > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > --- > > arch/riscv/Kconfig | 20 ++++ > > arch/riscv/Makefile | 1 + > > arch/riscv/configs/defconfig | 1 + > > arch/riscv/include/asm/efi.h | 44 +++++++++ > > drivers/firmware/efi/Kconfig | 2 +- > > drivers/firmware/efi/libstub/Makefile | 7 ++ > > drivers/firmware/efi/libstub/riscv-stub.c | 111 ++++++++++++++++++++++ > > 7 files changed, 185 insertions(+), 1 deletion(-) > > create mode 100644 arch/riscv/include/asm/efi.h > > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index f39e326a7a42..eb4f41c8f3ce 100644 > > --- a/arch/riscv/Kconfig > > +++ b/arch/riscv/Kconfig > > @@ -379,10 +379,30 @@ config CMDLINE_FORCE > > > > endchoice > > > > +config EFI_STUB > > + bool > > + > > +config EFI > > + bool "UEFI runtime support" > > + depends on OF > > + select LIBFDT > > + select UCS2_STRING > > + select EFI_PARAMS_FROM_FDT > > + select EFI_STUB > > + select EFI_GENERIC_STUB > > + default y > > + help > > + This option provides support for runtime services provided > > + by UEFI firmware (such as non-volatile variables, realtime > > + clock, and platform reset). A UEFI stub is also provided to > > + allow the kernel to be booted as an EFI application. This > > + is only useful on systems that have UEFI firmware. > > + > > endmenu > > > > menu "Power management options" > > > > source "kernel/power/Kconfig" > > +source "drivers/firmware/Kconfig" > > > > endmenu > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > index fb6e37db836d..079435804d6d 100644 > > --- a/arch/riscv/Makefile > > +++ b/arch/riscv/Makefile > > @@ -80,6 +80,7 @@ head-y := arch/riscv/kernel/head.o > > core-y += arch/riscv/ > > > > libs-y += arch/riscv/lib/ > > +core-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a > > > > PHONY += vdso_install > > vdso_install: > > diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig > > index 4da4886246a4..ae69e12d306a 100644 > > --- a/arch/riscv/configs/defconfig > > +++ b/arch/riscv/configs/defconfig > > @@ -129,3 +129,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > > # CONFIG_RUNTIME_TESTING_MENU is not set > > CONFIG_MEMTEST=y > > # CONFIG_SYSFS_SYSCALL is not set > > +CONFIG_EFI=y > > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h > > new file mode 100644 > > index 000000000000..62d7d5eafed8 > > --- /dev/null > > +++ b/arch/riscv/include/asm/efi.h > > @@ -0,0 +1,44 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Copyright (C) 2020 Western Digital Corporation or its affiliates. > > + * Based on arch/arm64/include/asm/efi.h > > + */ > > +#ifndef _ASM_EFI_H > > +#define _ASM_EFI_H > > + > > +#include <asm/io.h> > > +#include <asm/mmu_context.h> > > +#include <asm/ptrace.h> > > +#include <asm/tlbflush.h> > > + > > +#define VA_BITS_MIN 39 > > + > > +/* on RISC-V, the FDT may be located anywhere in system RAM */ > > +static inline unsigned long efi_get_max_fdt_addr(unsigned long dram_base) > > +{ > > + return ULONG_MAX; > > +} > > + > > +/* Load initrd at enough distance from DRAM start */ > > +static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base, > > + unsigned long image_addr) > > +{ > > + return dram_base + SZ_256M; > > +} > > + > > +#define efi_bs_call(func, ...) efi_system_table()->boottime->func(__VA_ARGS__) > > +#define efi_rt_call(func, ...) efi_system_table()->runtime->func(__VA_ARGS__) > > +#define efi_is_native() (true) > > + > > +#define efi_table_attr(inst, attr) (inst->attr) > > + > > +#define efi_call_proto(inst, func, ...) inst->func(inst, ##__VA_ARGS__) > > + > > +#define alloc_screen_info(x...) (&screen_info) > > +extern char stext_offset[]; > > + > > +static inline void free_screen_info(struct screen_info *si) > > +{ > > +} > > + > > +#endif /* _ASM_EFI_H */ > > diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig > > index 2a2b2b96a1dc..fcdc789d3f87 100644 > > --- a/drivers/firmware/efi/Kconfig > > +++ b/drivers/firmware/efi/Kconfig > > @@ -111,7 +111,7 @@ config EFI_GENERIC_STUB > > > > config EFI_ARMSTUB_DTB_LOADER > > bool "Enable the DTB loader" > > - depends on EFI_GENERIC_STUB > > + depends on EFI_GENERIC_STUB && !RISCV > > default y > > help > > Select this config option to add support for the dtb= command > > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > > index 2b4e09bf987c..7d46b70b51f2 100644 > > --- a/drivers/firmware/efi/libstub/Makefile > > +++ b/drivers/firmware/efi/libstub/Makefile > > @@ -22,6 +22,8 @@ cflags-$(CONFIG_ARM64) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > cflags-$(CONFIG_ARM) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > -fno-builtin -fpic \ > > $(call cc-option,-mno-single-pic-base) > > +cflags-$(CONFIG_RISCV) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > + -fpic > > > > cflags-$(CONFIG_EFI_GENERIC_STUB) += -I$(srctree)/scripts/dtc/libfdt > > > > @@ -57,6 +59,7 @@ lib-$(CONFIG_EFI_GENERIC_STUB) += efi-stub.o fdt.o string.o \ > > lib-$(CONFIG_ARM) += arm32-stub.o > > lib-$(CONFIG_ARM64) += arm64-stub.o > > lib-$(CONFIG_X86) += x86-stub.o > > +lib-$(CONFIG_RISCV) += riscv-stub.o > > CFLAGS_arm32-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > @@ -81,6 +84,10 @@ STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \ > > --prefix-symbols=__efistub_ > > STUBCOPY_RELOC-$(CONFIG_ARM64) := R_AARCH64_ABS > > > > +STUBCOPY_FLAGS-$(CONFIG_RISCV) += --prefix-alloc-sections=.init \ > > + --prefix-symbols=__efistub_ > > +STUBCOPY_RELOC-$(CONFIG_RISCV) := R_RISCV_HI20 > > + > > $(obj)/%.stub.o: $(obj)/%.o FORCE > > $(call if_changed,stubcopy) > > > > diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c > > new file mode 100644 > > index 000000000000..69d13e0ebaea > > --- /dev/null > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > > @@ -0,0 +1,111 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2013, 2014 Linaro Ltd; <roy.franz@linaro.org> > > + * Copyright (C) 2020 Western Digital Corporation or its affiliates. > > + * > > + * This file implements the EFI boot stub for the RISC-V kernel. > > + * Adapted from ARM64 version at drivers/firmware/efi/libstub/arm64-stub.c. > > + */ > > + > > +#include <linux/efi.h> > > +#include <linux/libfdt.h> > > +#include <linux/libfdt_env.h> > > +#include <asm/efi.h> > > +#include <asm/sections.h> > > + > > +#include "efistub.h" > > +/* > > + * RISCV requires the kernel image to placed TEXT_OFFSET bytes beyond a 2 MB > > + * aligned base for 64 bit and 4MB for 32 bit. > > + */ Fixed the comment. > > +#ifdef CONFIG_64BIT > > +#define MIN_KIMG_ALIGN SZ_2M > > +#else > > +#define MIN_KIMG_ALIGN SZ_4M > > +#endif > > + > > +typedef __attribute__((noreturn)) void (*jump_kernel_func)(unsigned int, > > + unsigned long); > > +efi_status_t check_platform_features(void) > > +{ > > + return EFI_SUCCESS; > > +} > > + > > +static u32 get_boot_hartid_from_fdt(unsigned long fdt) > > +{ > > + int chosen_node, len; > > + const fdt32_t *prop; > > + > > + chosen_node = fdt_path_offset((void *)fdt, "/chosen"); > > + if (chosen_node < 0) > > + return U32_MAX; > > + prop = fdt_getprop((void *)fdt, chosen_node, "boot-hartid", &len); > > + if (!prop || len != sizeof(u32)) > > + return U32_MAX; > > + > > + return fdt32_to_cpu(*prop); > > +} > > + > > +/* > > + * Jump to real kernel here with following constraints. > > + * 1. MMU should be disabled. > > + * 2. a0 should contain hartid > > + * 3. a1 should DT address > > + */ > > +void __noreturn efi_enter_kernel(unsigned long entrypoint, unsigned long fdt, > > + unsigned long fdt_size) > > +{ > > + unsigned long kernel_entry = entrypoint + (unsigned long)stext_offset; > > + jump_kernel_func jump_kernel = (jump_kernel_func) kernel_entry; > > + u32 hartid = get_boot_hartid_from_fdt(fdt); > > + > > + if (hartid == U32_MAX) > > + /* We can not use panic or BUG at this point */ > > + __asm__ __volatile__ ("ebreak"); > > + /* Disable MMU */ > > + csr_write(CSR_SATP, 0); > > + jump_kernel(hartid, fdt); > > +} > > + > > +efi_status_t handle_kernel_image(unsigned long *image_addr, > > + unsigned long *image_size, > > + unsigned long *reserve_addr, > > + unsigned long *reserve_size, > > + unsigned long dram_base, > > + efi_loaded_image_t *image) > > +{ > > + efi_status_t status; > > + unsigned long kernel_size, kernel_memsize = 0; > > + unsigned long max_alloc_address; > > + > > + if (image->image_base != _start) > > + pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n"); > > + > > I don't think you need this. > Sure. I will remove it. I guess ARM64 code has the error print for legacy loader code ? > > + kernel_size = _edata - _start; > > + kernel_memsize = kernel_size + (_end - _edata); > > + max_alloc_address = round_up(dram_base, MIN_KIMG_ALIGN) + > > + kernel_memsize; > > + > > You said the kernel could be anywhere in memory, as long as it is > aligned correctly, right? Sorry I was wrong about this. RISC-V kernel maps PAGE_OFFSET virtual address to the physical address <xyz> where it is booted. That means memory between dram start and and <xyz> address will be unusable. I also realized that the above computing max_address as above also won't work for the following reason. efi_allocate_pages_aligned actually ALIGN_DOWN the max_address. Thus, efi won't find enough free memory in this case if the max_address is computed from the dram_base. Is there an implicit requirement for efi_allocate_pages_aligned or efi_low_alloc_above should be tried in case of failure? > In that case, you don't need this, you can simply pass ULONG_MAX as > the max address. > As RISC-V should allocate memory as low as possible to avoid memory wastage, I think the following should work. efi_low_alloc_above(*reserve_size, MIN_KIMG_ALIGN, reserve_addr, dram_base); If this looks okay to you, efi_low_alloc_above should be moved back to mem.c from relocate.c. Should I do it in a separate patch or the original patch should be modified so that efi_low_alloc_above was never moved to relocate.c > > + if (IS_ALIGNED((u64)_start, MIN_KIMG_ALIGN)) { > > + /* > > + * Just execute from wherever we were loaded by the > > + * UEFI PE/COFF loader if the alignment is suitable. > > + */ > > + *image_addr = (u64)_start; > > Change these casts to (unsigned long), as reported by the robot. > Done. > So you no longer need the placement to be TEXT_OFFSET bytes past a > MIN_KIMG_ALIGN aligned boundary, right? > Nope. EFI memory marked the firmware area as reserved. So EFI memory allocator will make sure that it never allocates memory from that region. > > + *reserve_size = 0; > > + return EFI_SUCCESS; > > + } > > + status = efi_allocate_pages_aligned(*reserve_size, reserve_addr, > > + max_alloc_address, MIN_KIMG_ALIGN); > > + > > + if (status != EFI_SUCCESS) { > > + pr_efi_err("Failed to relocate kernel\n"); > > + *reserve_size = 0; > > + return status; > > + } > > + *image_addr = *reserve_addr; > > + > > + memcpy((void *)*image_addr, _start, kernel_size); > > + > > + return EFI_SUCCESS; > > +} > > -- > > 2.24.0 > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- Regards, Atish
On Sat, 18 Apr 2020 at 05:03, Atish Patra <atishp@atishpatra.org> wrote: > > On Thu, Apr 16, 2020 at 12:41 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Wed, 15 Apr 2020 at 21:54, Atish Patra <atish.patra@wdc.com> wrote: > > > > > > Add a RISC-V architecture specific stub code that actually copies the > > > actual kernel image to a valid address and jump to it after boot services > > > are terminated. Enable UEFI related kernel configs as well for RISC-V. > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > --- > > > arch/riscv/Kconfig | 20 ++++ > > > arch/riscv/Makefile | 1 + > > > arch/riscv/configs/defconfig | 1 + > > > arch/riscv/include/asm/efi.h | 44 +++++++++ > > > drivers/firmware/efi/Kconfig | 2 +- > > > drivers/firmware/efi/libstub/Makefile | 7 ++ > > > drivers/firmware/efi/libstub/riscv-stub.c | 111 ++++++++++++++++++++++ > > > 7 files changed, 185 insertions(+), 1 deletion(-) > > > create mode 100644 arch/riscv/include/asm/efi.h > > > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > index f39e326a7a42..eb4f41c8f3ce 100644 > > > --- a/arch/riscv/Kconfig > > > +++ b/arch/riscv/Kconfig > > > @@ -379,10 +379,30 @@ config CMDLINE_FORCE > > > > > > endchoice > > > > > > +config EFI_STUB > > > + bool > > > + > > > +config EFI > > > + bool "UEFI runtime support" > > > + depends on OF > > > + select LIBFDT > > > + select UCS2_STRING > > > + select EFI_PARAMS_FROM_FDT > > > + select EFI_STUB > > > + select EFI_GENERIC_STUB > > > + default y > > > + help > > > + This option provides support for runtime services provided > > > + by UEFI firmware (such as non-volatile variables, realtime > > > + clock, and platform reset). A UEFI stub is also provided to > > > + allow the kernel to be booted as an EFI application. This > > > + is only useful on systems that have UEFI firmware. > > > + > > > endmenu > > > > > > menu "Power management options" > > > > > > source "kernel/power/Kconfig" > > > +source "drivers/firmware/Kconfig" > > > > > > endmenu > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > > index fb6e37db836d..079435804d6d 100644 > > > --- a/arch/riscv/Makefile > > > +++ b/arch/riscv/Makefile > > > @@ -80,6 +80,7 @@ head-y := arch/riscv/kernel/head.o > > > core-y += arch/riscv/ > > > > > > libs-y += arch/riscv/lib/ > > > +core-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a > > > > > > PHONY += vdso_install > > > vdso_install: > > > diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig > > > index 4da4886246a4..ae69e12d306a 100644 > > > --- a/arch/riscv/configs/defconfig > > > +++ b/arch/riscv/configs/defconfig > > > @@ -129,3 +129,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > > > # CONFIG_RUNTIME_TESTING_MENU is not set > > > CONFIG_MEMTEST=y > > > # CONFIG_SYSFS_SYSCALL is not set > > > +CONFIG_EFI=y > > > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h > > > new file mode 100644 > > > index 000000000000..62d7d5eafed8 > > > --- /dev/null > > > +++ b/arch/riscv/include/asm/efi.h > > > @@ -0,0 +1,44 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > +/* > > > + * Copyright (C) 2020 Western Digital Corporation or its affiliates. > > > + * Based on arch/arm64/include/asm/efi.h > > > + */ > > > +#ifndef _ASM_EFI_H > > > +#define _ASM_EFI_H > > > + > > > +#include <asm/io.h> > > > +#include <asm/mmu_context.h> > > > +#include <asm/ptrace.h> > > > +#include <asm/tlbflush.h> > > > + > > > +#define VA_BITS_MIN 39 > > > + > > > +/* on RISC-V, the FDT may be located anywhere in system RAM */ > > > +static inline unsigned long efi_get_max_fdt_addr(unsigned long dram_base) > > > +{ > > > + return ULONG_MAX; > > > +} > > > + > > > +/* Load initrd at enough distance from DRAM start */ > > > +static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base, > > > + unsigned long image_addr) > > > +{ > > > + return dram_base + SZ_256M; > > > +} > > > + > > > +#define efi_bs_call(func, ...) efi_system_table()->boottime->func(__VA_ARGS__) > > > +#define efi_rt_call(func, ...) efi_system_table()->runtime->func(__VA_ARGS__) > > > +#define efi_is_native() (true) > > > + > > > +#define efi_table_attr(inst, attr) (inst->attr) > > > + > > > +#define efi_call_proto(inst, func, ...) inst->func(inst, ##__VA_ARGS__) > > > + > > > +#define alloc_screen_info(x...) (&screen_info) > > > +extern char stext_offset[]; > > > + > > > +static inline void free_screen_info(struct screen_info *si) > > > +{ > > > +} > > > + > > > +#endif /* _ASM_EFI_H */ > > > diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig > > > index 2a2b2b96a1dc..fcdc789d3f87 100644 > > > --- a/drivers/firmware/efi/Kconfig > > > +++ b/drivers/firmware/efi/Kconfig > > > @@ -111,7 +111,7 @@ config EFI_GENERIC_STUB > > > > > > config EFI_ARMSTUB_DTB_LOADER > > > bool "Enable the DTB loader" > > > - depends on EFI_GENERIC_STUB > > > + depends on EFI_GENERIC_STUB && !RISCV > > > default y > > > help > > > Select this config option to add support for the dtb= command > > > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > > > index 2b4e09bf987c..7d46b70b51f2 100644 > > > --- a/drivers/firmware/efi/libstub/Makefile > > > +++ b/drivers/firmware/efi/libstub/Makefile > > > @@ -22,6 +22,8 @@ cflags-$(CONFIG_ARM64) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > cflags-$(CONFIG_ARM) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > -fno-builtin -fpic \ > > > $(call cc-option,-mno-single-pic-base) > > > +cflags-$(CONFIG_RISCV) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > + -fpic > > > > > > cflags-$(CONFIG_EFI_GENERIC_STUB) += -I$(srctree)/scripts/dtc/libfdt > > > > > > @@ -57,6 +59,7 @@ lib-$(CONFIG_EFI_GENERIC_STUB) += efi-stub.o fdt.o string.o \ > > > lib-$(CONFIG_ARM) += arm32-stub.o > > > lib-$(CONFIG_ARM64) += arm64-stub.o > > > lib-$(CONFIG_X86) += x86-stub.o > > > +lib-$(CONFIG_RISCV) += riscv-stub.o > > > CFLAGS_arm32-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > > CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > > > @@ -81,6 +84,10 @@ STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \ > > > --prefix-symbols=__efistub_ > > > STUBCOPY_RELOC-$(CONFIG_ARM64) := R_AARCH64_ABS > > > > > > +STUBCOPY_FLAGS-$(CONFIG_RISCV) += --prefix-alloc-sections=.init \ > > > + --prefix-symbols=__efistub_ > > > +STUBCOPY_RELOC-$(CONFIG_RISCV) := R_RISCV_HI20 > > > + > > > $(obj)/%.stub.o: $(obj)/%.o FORCE > > > $(call if_changed,stubcopy) > > > > > > diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c > > > new file mode 100644 > > > index 000000000000..69d13e0ebaea > > > --- /dev/null > > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > > > @@ -0,0 +1,111 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Copyright (C) 2013, 2014 Linaro Ltd; <roy.franz@linaro.org> > > > + * Copyright (C) 2020 Western Digital Corporation or its affiliates. > > > + * > > > + * This file implements the EFI boot stub for the RISC-V kernel. > > > + * Adapted from ARM64 version at drivers/firmware/efi/libstub/arm64-stub.c. > > > + */ > > > + > > > +#include <linux/efi.h> > > > +#include <linux/libfdt.h> > > > +#include <linux/libfdt_env.h> > > > +#include <asm/efi.h> > > > +#include <asm/sections.h> > > > + > > > +#include "efistub.h" > > > +/* > > > + * RISCV requires the kernel image to placed TEXT_OFFSET bytes beyond a 2 MB > > > + * aligned base for 64 bit and 4MB for 32 bit. > > > + */ > > Fixed the comment. > > > > +#ifdef CONFIG_64BIT > > > +#define MIN_KIMG_ALIGN SZ_2M > > > +#else > > > +#define MIN_KIMG_ALIGN SZ_4M > > > +#endif > > > + > > > +typedef __attribute__((noreturn)) void (*jump_kernel_func)(unsigned int, > > > + unsigned long); > > > +efi_status_t check_platform_features(void) > > > +{ > > > + return EFI_SUCCESS; > > > +} > > > + > > > +static u32 get_boot_hartid_from_fdt(unsigned long fdt) > > > +{ > > > + int chosen_node, len; > > > + const fdt32_t *prop; > > > + > > > + chosen_node = fdt_path_offset((void *)fdt, "/chosen"); > > > + if (chosen_node < 0) > > > + return U32_MAX; > > > + prop = fdt_getprop((void *)fdt, chosen_node, "boot-hartid", &len); > > > + if (!prop || len != sizeof(u32)) > > > + return U32_MAX; > > > + > > > + return fdt32_to_cpu(*prop); > > > +} > > > + > > > +/* > > > + * Jump to real kernel here with following constraints. > > > + * 1. MMU should be disabled. > > > + * 2. a0 should contain hartid > > > + * 3. a1 should DT address > > > + */ > > > +void __noreturn efi_enter_kernel(unsigned long entrypoint, unsigned long fdt, > > > + unsigned long fdt_size) > > > +{ > > > + unsigned long kernel_entry = entrypoint + (unsigned long)stext_offset; > > > + jump_kernel_func jump_kernel = (jump_kernel_func) kernel_entry; > > > + u32 hartid = get_boot_hartid_from_fdt(fdt); > > > + > > > + if (hartid == U32_MAX) > > > + /* We can not use panic or BUG at this point */ > > > + __asm__ __volatile__ ("ebreak"); > > > + /* Disable MMU */ > > > + csr_write(CSR_SATP, 0); > > > + jump_kernel(hartid, fdt); > > > +} > > > + > > > +efi_status_t handle_kernel_image(unsigned long *image_addr, > > > + unsigned long *image_size, > > > + unsigned long *reserve_addr, > > > + unsigned long *reserve_size, > > > + unsigned long dram_base, > > > + efi_loaded_image_t *image) > > > +{ > > > + efi_status_t status; > > > + unsigned long kernel_size, kernel_memsize = 0; > > > + unsigned long max_alloc_address; > > > + > > > + if (image->image_base != _start) > > > + pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n"); > > > + > > > > I don't think you need this. > > > > Sure. I will remove it. I guess ARM64 code has the error print for > legacy loader code ? > No, for broken distro versions of GRUB > > > + kernel_size = _edata - _start; > > > + kernel_memsize = kernel_size + (_end - _edata); > > > + max_alloc_address = round_up(dram_base, MIN_KIMG_ALIGN) + > > > + kernel_memsize; > > > + > > > > You said the kernel could be anywhere in memory, as long as it is > > aligned correctly, right? > > Sorry I was wrong about this. RISC-V kernel maps PAGE_OFFSET virtual > address to the > physical address <xyz> where it is booted. That means memory between > dram start and and <xyz> address > will be unusable. > OK > I also realized that the above computing max_address as above also > won't work for the following reason. > efi_allocate_pages_aligned actually ALIGN_DOWN the max_address. Thus, > efi won't find enough > free memory in this case if the max_address is computed from the dram_base. > > Is there an implicit requirement for efi_allocate_pages_aligned or > efi_low_alloc_above should be tried in case of failure? > No not really. What ever works for your particular use case is acceptable to me. > > In that case, you don't need this, you can simply pass ULONG_MAX as > > the max address. > > > As RISC-V should allocate memory as low as possible to avoid memory > wastage, I think the following should work. > > efi_low_alloc_above(*reserve_size, MIN_KIMG_ALIGN, reserve_addr, dram_base); > > If this looks okay to you, efi_low_alloc_above should be moved back to > mem.c from relocate.c. > Should I do it in a separate patch or the original patch should be > modified so that efi_low_alloc_above was never moved to relocate.c > No, please keep efi_low_alloc_above() where it is, but drop the static, and put back the declaration in efistub.h > > > + if (IS_ALIGNED((u64)_start, MIN_KIMG_ALIGN)) { > > > + /* > > > + * Just execute from wherever we were loaded by the > > > + * UEFI PE/COFF loader if the alignment is suitable. > > > + */ > > > + *image_addr = (u64)_start; > > > > Change these casts to (unsigned long), as reported by the robot. > > > Done. > > > So you no longer need the placement to be TEXT_OFFSET bytes past a > > MIN_KIMG_ALIGN aligned boundary, right? > > > > Nope. EFI memory marked the firmware area as reserved. So EFI memory > allocator will make sure that > it never allocates memory from that region. > OK, good. > > > + *reserve_size = 0; > > > + return EFI_SUCCESS; > > > + } > > > + status = efi_allocate_pages_aligned(*reserve_size, reserve_addr, > > > + max_alloc_address, MIN_KIMG_ALIGN); > > > + > > > + if (status != EFI_SUCCESS) { > > > + pr_efi_err("Failed to relocate kernel\n"); > > > + *reserve_size = 0; > > > + return status; > > > + } > > > + *image_addr = *reserve_addr; > > > + > > > + memcpy((void *)*image_addr, _start, kernel_size); > > > + > > > + return EFI_SUCCESS; > > > +} > > > -- > > > 2.24.0 > > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > > -- > Regards, > Atish
On Sat, 18 Apr 2020 at 12:51, Ard Biesheuvel <ardb@kernel.org> wrote: > > On Sat, 18 Apr 2020 at 05:03, Atish Patra <atishp@atishpatra.org> wrote: > > > > On Thu, Apr 16, 2020 at 12:41 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > On Wed, 15 Apr 2020 at 21:54, Atish Patra <atish.patra@wdc.com> wrote: > > > > > > > > Add a RISC-V architecture specific stub code that actually copies the > > > > actual kernel image to a valid address and jump to it after boot services > > > > are terminated. Enable UEFI related kernel configs as well for RISC-V. > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > > --- > > > > arch/riscv/Kconfig | 20 ++++ > > > > arch/riscv/Makefile | 1 + > > > > arch/riscv/configs/defconfig | 1 + > > > > arch/riscv/include/asm/efi.h | 44 +++++++++ > > > > drivers/firmware/efi/Kconfig | 2 +- > > > > drivers/firmware/efi/libstub/Makefile | 7 ++ > > > > drivers/firmware/efi/libstub/riscv-stub.c | 111 ++++++++++++++++++++++ > > > > 7 files changed, 185 insertions(+), 1 deletion(-) > > > > create mode 100644 arch/riscv/include/asm/efi.h > > > > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > index f39e326a7a42..eb4f41c8f3ce 100644 > > > > --- a/arch/riscv/Kconfig > > > > +++ b/arch/riscv/Kconfig > > > > @@ -379,10 +379,30 @@ config CMDLINE_FORCE > > > > > > > > endchoice > > > > > > > > +config EFI_STUB > > > > + bool > > > > + > > > > +config EFI > > > > + bool "UEFI runtime support" > > > > + depends on OF > > > > + select LIBFDT > > > > + select UCS2_STRING > > > > + select EFI_PARAMS_FROM_FDT > > > > + select EFI_STUB > > > > + select EFI_GENERIC_STUB > > > > + default y > > > > + help > > > > + This option provides support for runtime services provided > > > > + by UEFI firmware (such as non-volatile variables, realtime > > > > + clock, and platform reset). A UEFI stub is also provided to > > > > + allow the kernel to be booted as an EFI application. This > > > > + is only useful on systems that have UEFI firmware. > > > > + > > > > endmenu > > > > > > > > menu "Power management options" > > > > > > > > source "kernel/power/Kconfig" > > > > +source "drivers/firmware/Kconfig" > > > > > > > > endmenu > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > > > index fb6e37db836d..079435804d6d 100644 > > > > --- a/arch/riscv/Makefile > > > > +++ b/arch/riscv/Makefile > > > > @@ -80,6 +80,7 @@ head-y := arch/riscv/kernel/head.o > > > > core-y += arch/riscv/ > > > > > > > > libs-y += arch/riscv/lib/ > > > > +core-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a > > > > > > > > PHONY += vdso_install > > > > vdso_install: > > > > diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig > > > > index 4da4886246a4..ae69e12d306a 100644 > > > > --- a/arch/riscv/configs/defconfig > > > > +++ b/arch/riscv/configs/defconfig > > > > @@ -129,3 +129,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > > > > # CONFIG_RUNTIME_TESTING_MENU is not set > > > > CONFIG_MEMTEST=y > > > > # CONFIG_SYSFS_SYSCALL is not set > > > > +CONFIG_EFI=y > > > > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h > > > > new file mode 100644 > > > > index 000000000000..62d7d5eafed8 > > > > --- /dev/null > > > > +++ b/arch/riscv/include/asm/efi.h > > > > @@ -0,0 +1,44 @@ > > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > +/* > > > > + * Copyright (C) 2020 Western Digital Corporation or its affiliates. > > > > + * Based on arch/arm64/include/asm/efi.h > > > > + */ > > > > +#ifndef _ASM_EFI_H > > > > +#define _ASM_EFI_H > > > > + > > > > +#include <asm/io.h> > > > > +#include <asm/mmu_context.h> > > > > +#include <asm/ptrace.h> > > > > +#include <asm/tlbflush.h> > > > > + > > > > +#define VA_BITS_MIN 39 > > > > + > > > > +/* on RISC-V, the FDT may be located anywhere in system RAM */ > > > > +static inline unsigned long efi_get_max_fdt_addr(unsigned long dram_base) > > > > +{ > > > > + return ULONG_MAX; > > > > +} > > > > + > > > > +/* Load initrd at enough distance from DRAM start */ > > > > +static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base, > > > > + unsigned long image_addr) > > > > +{ > > > > + return dram_base + SZ_256M; > > > > +} > > > > + > > > > +#define efi_bs_call(func, ...) efi_system_table()->boottime->func(__VA_ARGS__) > > > > +#define efi_rt_call(func, ...) efi_system_table()->runtime->func(__VA_ARGS__) > > > > +#define efi_is_native() (true) > > > > + > > > > +#define efi_table_attr(inst, attr) (inst->attr) > > > > + > > > > +#define efi_call_proto(inst, func, ...) inst->func(inst, ##__VA_ARGS__) > > > > + > > > > +#define alloc_screen_info(x...) (&screen_info) > > > > +extern char stext_offset[]; > > > > + > > > > +static inline void free_screen_info(struct screen_info *si) > > > > +{ > > > > +} > > > > + > > > > +#endif /* _ASM_EFI_H */ > > > > diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig > > > > index 2a2b2b96a1dc..fcdc789d3f87 100644 > > > > --- a/drivers/firmware/efi/Kconfig > > > > +++ b/drivers/firmware/efi/Kconfig > > > > @@ -111,7 +111,7 @@ config EFI_GENERIC_STUB > > > > > > > > config EFI_ARMSTUB_DTB_LOADER > > > > bool "Enable the DTB loader" > > > > - depends on EFI_GENERIC_STUB > > > > + depends on EFI_GENERIC_STUB && !RISCV > > > > default y > > > > help > > > > Select this config option to add support for the dtb= command > > > > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > > > > index 2b4e09bf987c..7d46b70b51f2 100644 > > > > --- a/drivers/firmware/efi/libstub/Makefile > > > > +++ b/drivers/firmware/efi/libstub/Makefile > > > > @@ -22,6 +22,8 @@ cflags-$(CONFIG_ARM64) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > cflags-$(CONFIG_ARM) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > -fno-builtin -fpic \ > > > > $(call cc-option,-mno-single-pic-base) > > > > +cflags-$(CONFIG_RISCV) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > + -fpic > > > > > > > > cflags-$(CONFIG_EFI_GENERIC_STUB) += -I$(srctree)/scripts/dtc/libfdt > > > > > > > > @@ -57,6 +59,7 @@ lib-$(CONFIG_EFI_GENERIC_STUB) += efi-stub.o fdt.o string.o \ > > > > lib-$(CONFIG_ARM) += arm32-stub.o > > > > lib-$(CONFIG_ARM64) += arm64-stub.o > > > > lib-$(CONFIG_X86) += x86-stub.o > > > > +lib-$(CONFIG_RISCV) += riscv-stub.o > > > > CFLAGS_arm32-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > > > > > @@ -81,6 +84,10 @@ STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \ > > > > --prefix-symbols=__efistub_ > > > > STUBCOPY_RELOC-$(CONFIG_ARM64) := R_AARCH64_ABS > > > > > > > > +STUBCOPY_FLAGS-$(CONFIG_RISCV) += --prefix-alloc-sections=.init \ > > > > + --prefix-symbols=__efistub_ > > > > +STUBCOPY_RELOC-$(CONFIG_RISCV) := R_RISCV_HI20 > > > > + > > > > $(obj)/%.stub.o: $(obj)/%.o FORCE > > > > $(call if_changed,stubcopy) > > > > > > > > diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c > > > > new file mode 100644 > > > > index 000000000000..69d13e0ebaea > > > > --- /dev/null > > > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > > > > @@ -0,0 +1,111 @@ > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > +/* > > > > + * Copyright (C) 2013, 2014 Linaro Ltd; <roy.franz@linaro.org> > > > > + * Copyright (C) 2020 Western Digital Corporation or its affiliates. > > > > + * > > > > + * This file implements the EFI boot stub for the RISC-V kernel. > > > > + * Adapted from ARM64 version at drivers/firmware/efi/libstub/arm64-stub.c. > > > > + */ > > > > + > > > > +#include <linux/efi.h> > > > > +#include <linux/libfdt.h> > > > > +#include <linux/libfdt_env.h> > > > > +#include <asm/efi.h> > > > > +#include <asm/sections.h> > > > > + > > > > +#include "efistub.h" > > > > +/* > > > > + * RISCV requires the kernel image to placed TEXT_OFFSET bytes beyond a 2 MB > > > > + * aligned base for 64 bit and 4MB for 32 bit. > > > > + */ > > > > Fixed the comment. > > > > > > +#ifdef CONFIG_64BIT > > > > +#define MIN_KIMG_ALIGN SZ_2M > > > > +#else > > > > +#define MIN_KIMG_ALIGN SZ_4M > > > > +#endif > > > > + > > > > +typedef __attribute__((noreturn)) void (*jump_kernel_func)(unsigned int, > > > > + unsigned long); > > > > +efi_status_t check_platform_features(void) > > > > +{ > > > > + return EFI_SUCCESS; > > > > +} > > > > + > > > > +static u32 get_boot_hartid_from_fdt(unsigned long fdt) > > > > +{ > > > > + int chosen_node, len; > > > > + const fdt32_t *prop; > > > > + > > > > + chosen_node = fdt_path_offset((void *)fdt, "/chosen"); > > > > + if (chosen_node < 0) > > > > + return U32_MAX; > > > > + prop = fdt_getprop((void *)fdt, chosen_node, "boot-hartid", &len); > > > > + if (!prop || len != sizeof(u32)) > > > > + return U32_MAX; > > > > + > > > > + return fdt32_to_cpu(*prop); > > > > +} > > > > + > > > > +/* > > > > + * Jump to real kernel here with following constraints. > > > > + * 1. MMU should be disabled. > > > > + * 2. a0 should contain hartid > > > > + * 3. a1 should DT address > > > > + */ > > > > +void __noreturn efi_enter_kernel(unsigned long entrypoint, unsigned long fdt, > > > > + unsigned long fdt_size) > > > > +{ > > > > + unsigned long kernel_entry = entrypoint + (unsigned long)stext_offset; > > > > + jump_kernel_func jump_kernel = (jump_kernel_func) kernel_entry; > > > > + u32 hartid = get_boot_hartid_from_fdt(fdt); > > > > + > > > > + if (hartid == U32_MAX) > > > > + /* We can not use panic or BUG at this point */ > > > > + __asm__ __volatile__ ("ebreak"); > > > > + /* Disable MMU */ > > > > + csr_write(CSR_SATP, 0); > > > > + jump_kernel(hartid, fdt); > > > > +} > > > > + > > > > +efi_status_t handle_kernel_image(unsigned long *image_addr, > > > > + unsigned long *image_size, > > > > + unsigned long *reserve_addr, > > > > + unsigned long *reserve_size, > > > > + unsigned long dram_base, > > > > + efi_loaded_image_t *image) > > > > +{ > > > > + efi_status_t status; > > > > + unsigned long kernel_size, kernel_memsize = 0; > > > > + unsigned long max_alloc_address; > > > > + > > > > + if (image->image_base != _start) > > > > + pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n"); > > > > + > > > > > > I don't think you need this. > > > > > > > Sure. I will remove it. I guess ARM64 code has the error print for > > legacy loader code ? > > > > No, for broken distro versions of GRUB > > > > > + kernel_size = _edata - _start; > > > > + kernel_memsize = kernel_size + (_end - _edata); > > > > + max_alloc_address = round_up(dram_base, MIN_KIMG_ALIGN) + > > > > + kernel_memsize; > > > > + > > > > > > You said the kernel could be anywhere in memory, as long as it is > > > aligned correctly, right? > > > > Sorry I was wrong about this. RISC-V kernel maps PAGE_OFFSET virtual > > address to the > > physical address <xyz> where it is booted. That means memory between > > dram start and and <xyz> address > > will be unusable. > > > > OK > > > I also realized that the above computing max_address as above also > > won't work for the following reason. > > efi_allocate_pages_aligned actually ALIGN_DOWN the max_address. Thus, > > efi won't find enough > > free memory in this case if the max_address is computed from the dram_base. > > > > Is there an implicit requirement for efi_allocate_pages_aligned or > > efi_low_alloc_above should be tried in case of failure? > > > > No not really. What ever works for your particular use case is acceptable to me. > > > > In that case, you don't need this, you can simply pass ULONG_MAX as > > > the max address. > > > > > As RISC-V should allocate memory as low as possible to avoid memory > > wastage, I think the following should work. > > > > efi_low_alloc_above(*reserve_size, MIN_KIMG_ALIGN, reserve_addr, dram_base); > > > > If this looks okay to you, efi_low_alloc_above should be moved back to > > mem.c from relocate.c. > > Should I do it in a separate patch or the original patch should be > > modified so that efi_low_alloc_above was never moved to relocate.c > > > > No, please keep efi_low_alloc_above() where it is, but drop the > static, and put back the declaration in efistub.h > Alternatively, can you check whether efi_relocate_kernel() already does what you need?
On Sat, Apr 18, 2020 at 5:39 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Sat, 18 Apr 2020 at 12:51, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Sat, 18 Apr 2020 at 05:03, Atish Patra <atishp@atishpatra.org> wrote: > > > > > > On Thu, Apr 16, 2020 at 12:41 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > > > On Wed, 15 Apr 2020 at 21:54, Atish Patra <atish.patra@wdc.com> wrote: > > > > > > > > > > Add a RISC-V architecture specific stub code that actually copies the > > > > > actual kernel image to a valid address and jump to it after boot services > > > > > are terminated. Enable UEFI related kernel configs as well for RISC-V. > > > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > > > --- > > > > > arch/riscv/Kconfig | 20 ++++ > > > > > arch/riscv/Makefile | 1 + > > > > > arch/riscv/configs/defconfig | 1 + > > > > > arch/riscv/include/asm/efi.h | 44 +++++++++ > > > > > drivers/firmware/efi/Kconfig | 2 +- > > > > > drivers/firmware/efi/libstub/Makefile | 7 ++ > > > > > drivers/firmware/efi/libstub/riscv-stub.c | 111 ++++++++++++++++++++++ > > > > > 7 files changed, 185 insertions(+), 1 deletion(-) > > > > > create mode 100644 arch/riscv/include/asm/efi.h > > > > > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > index f39e326a7a42..eb4f41c8f3ce 100644 > > > > > --- a/arch/riscv/Kconfig > > > > > +++ b/arch/riscv/Kconfig > > > > > @@ -379,10 +379,30 @@ config CMDLINE_FORCE > > > > > > > > > > endchoice > > > > > > > > > > +config EFI_STUB > > > > > + bool > > > > > + > > > > > +config EFI > > > > > + bool "UEFI runtime support" > > > > > + depends on OF > > > > > + select LIBFDT > > > > > + select UCS2_STRING > > > > > + select EFI_PARAMS_FROM_FDT > > > > > + select EFI_STUB > > > > > + select EFI_GENERIC_STUB > > > > > + default y > > > > > + help > > > > > + This option provides support for runtime services provided > > > > > + by UEFI firmware (such as non-volatile variables, realtime > > > > > + clock, and platform reset). A UEFI stub is also provided to > > > > > + allow the kernel to be booted as an EFI application. This > > > > > + is only useful on systems that have UEFI firmware. > > > > > + > > > > > endmenu > > > > > > > > > > menu "Power management options" > > > > > > > > > > source "kernel/power/Kconfig" > > > > > +source "drivers/firmware/Kconfig" > > > > > > > > > > endmenu > > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > > > > index fb6e37db836d..079435804d6d 100644 > > > > > --- a/arch/riscv/Makefile > > > > > +++ b/arch/riscv/Makefile > > > > > @@ -80,6 +80,7 @@ head-y := arch/riscv/kernel/head.o > > > > > core-y += arch/riscv/ > > > > > > > > > > libs-y += arch/riscv/lib/ > > > > > +core-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a > > > > > > > > > > PHONY += vdso_install > > > > > vdso_install: > > > > > diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig > > > > > index 4da4886246a4..ae69e12d306a 100644 > > > > > --- a/arch/riscv/configs/defconfig > > > > > +++ b/arch/riscv/configs/defconfig > > > > > @@ -129,3 +129,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > > > > > # CONFIG_RUNTIME_TESTING_MENU is not set > > > > > CONFIG_MEMTEST=y > > > > > # CONFIG_SYSFS_SYSCALL is not set > > > > > +CONFIG_EFI=y > > > > > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h > > > > > new file mode 100644 > > > > > index 000000000000..62d7d5eafed8 > > > > > --- /dev/null > > > > > +++ b/arch/riscv/include/asm/efi.h > > > > > @@ -0,0 +1,44 @@ > > > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > > +/* > > > > > + * Copyright (C) 2020 Western Digital Corporation or its affiliates. > > > > > + * Based on arch/arm64/include/asm/efi.h > > > > > + */ > > > > > +#ifndef _ASM_EFI_H > > > > > +#define _ASM_EFI_H > > > > > + > > > > > +#include <asm/io.h> > > > > > +#include <asm/mmu_context.h> > > > > > +#include <asm/ptrace.h> > > > > > +#include <asm/tlbflush.h> > > > > > + > > > > > +#define VA_BITS_MIN 39 > > > > > + > > > > > +/* on RISC-V, the FDT may be located anywhere in system RAM */ > > > > > +static inline unsigned long efi_get_max_fdt_addr(unsigned long dram_base) > > > > > +{ > > > > > + return ULONG_MAX; > > > > > +} > > > > > + > > > > > +/* Load initrd at enough distance from DRAM start */ > > > > > +static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base, > > > > > + unsigned long image_addr) > > > > > +{ > > > > > + return dram_base + SZ_256M; > > > > > +} > > > > > + > > > > > +#define efi_bs_call(func, ...) efi_system_table()->boottime->func(__VA_ARGS__) > > > > > +#define efi_rt_call(func, ...) efi_system_table()->runtime->func(__VA_ARGS__) > > > > > +#define efi_is_native() (true) > > > > > + > > > > > +#define efi_table_attr(inst, attr) (inst->attr) > > > > > + > > > > > +#define efi_call_proto(inst, func, ...) inst->func(inst, ##__VA_ARGS__) > > > > > + > > > > > +#define alloc_screen_info(x...) (&screen_info) > > > > > +extern char stext_offset[]; > > > > > + > > > > > +static inline void free_screen_info(struct screen_info *si) > > > > > +{ > > > > > +} > > > > > + > > > > > +#endif /* _ASM_EFI_H */ > > > > > diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig > > > > > index 2a2b2b96a1dc..fcdc789d3f87 100644 > > > > > --- a/drivers/firmware/efi/Kconfig > > > > > +++ b/drivers/firmware/efi/Kconfig > > > > > @@ -111,7 +111,7 @@ config EFI_GENERIC_STUB > > > > > > > > > > config EFI_ARMSTUB_DTB_LOADER > > > > > bool "Enable the DTB loader" > > > > > - depends on EFI_GENERIC_STUB > > > > > + depends on EFI_GENERIC_STUB && !RISCV > > > > > default y > > > > > help > > > > > Select this config option to add support for the dtb= command > > > > > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > > > > > index 2b4e09bf987c..7d46b70b51f2 100644 > > > > > --- a/drivers/firmware/efi/libstub/Makefile > > > > > +++ b/drivers/firmware/efi/libstub/Makefile > > > > > @@ -22,6 +22,8 @@ cflags-$(CONFIG_ARM64) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > cflags-$(CONFIG_ARM) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > -fno-builtin -fpic \ > > > > > $(call cc-option,-mno-single-pic-base) > > > > > +cflags-$(CONFIG_RISCV) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > + -fpic > > > > > > > > > > cflags-$(CONFIG_EFI_GENERIC_STUB) += -I$(srctree)/scripts/dtc/libfdt > > > > > > > > > > @@ -57,6 +59,7 @@ lib-$(CONFIG_EFI_GENERIC_STUB) += efi-stub.o fdt.o string.o \ > > > > > lib-$(CONFIG_ARM) += arm32-stub.o > > > > > lib-$(CONFIG_ARM64) += arm64-stub.o > > > > > lib-$(CONFIG_X86) += x86-stub.o > > > > > +lib-$(CONFIG_RISCV) += riscv-stub.o > > > > > CFLAGS_arm32-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > > CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > > > > > > > @@ -81,6 +84,10 @@ STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \ > > > > > --prefix-symbols=__efistub_ > > > > > STUBCOPY_RELOC-$(CONFIG_ARM64) := R_AARCH64_ABS > > > > > > > > > > +STUBCOPY_FLAGS-$(CONFIG_RISCV) += --prefix-alloc-sections=.init \ > > > > > + --prefix-symbols=__efistub_ > > > > > +STUBCOPY_RELOC-$(CONFIG_RISCV) := R_RISCV_HI20 > > > > > + > > > > > $(obj)/%.stub.o: $(obj)/%.o FORCE > > > > > $(call if_changed,stubcopy) > > > > > > > > > > diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c > > > > > new file mode 100644 > > > > > index 000000000000..69d13e0ebaea > > > > > --- /dev/null > > > > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > > > > > @@ -0,0 +1,111 @@ > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > +/* > > > > > + * Copyright (C) 2013, 2014 Linaro Ltd; <roy.franz@linaro.org> > > > > > + * Copyright (C) 2020 Western Digital Corporation or its affiliates. > > > > > + * > > > > > + * This file implements the EFI boot stub for the RISC-V kernel. > > > > > + * Adapted from ARM64 version at drivers/firmware/efi/libstub/arm64-stub.c. > > > > > + */ > > > > > + > > > > > +#include <linux/efi.h> > > > > > +#include <linux/libfdt.h> > > > > > +#include <linux/libfdt_env.h> > > > > > +#include <asm/efi.h> > > > > > +#include <asm/sections.h> > > > > > + > > > > > +#include "efistub.h" > > > > > +/* > > > > > + * RISCV requires the kernel image to placed TEXT_OFFSET bytes beyond a 2 MB > > > > > + * aligned base for 64 bit and 4MB for 32 bit. > > > > > + */ > > > > > > Fixed the comment. > > > > > > > > +#ifdef CONFIG_64BIT > > > > > +#define MIN_KIMG_ALIGN SZ_2M > > > > > +#else > > > > > +#define MIN_KIMG_ALIGN SZ_4M > > > > > +#endif > > > > > + > > > > > +typedef __attribute__((noreturn)) void (*jump_kernel_func)(unsigned int, > > > > > + unsigned long); > > > > > +efi_status_t check_platform_features(void) > > > > > +{ > > > > > + return EFI_SUCCESS; > > > > > +} > > > > > + > > > > > +static u32 get_boot_hartid_from_fdt(unsigned long fdt) > > > > > +{ > > > > > + int chosen_node, len; > > > > > + const fdt32_t *prop; > > > > > + > > > > > + chosen_node = fdt_path_offset((void *)fdt, "/chosen"); > > > > > + if (chosen_node < 0) > > > > > + return U32_MAX; > > > > > + prop = fdt_getprop((void *)fdt, chosen_node, "boot-hartid", &len); > > > > > + if (!prop || len != sizeof(u32)) > > > > > + return U32_MAX; > > > > > + > > > > > + return fdt32_to_cpu(*prop); > > > > > +} > > > > > + > > > > > +/* > > > > > + * Jump to real kernel here with following constraints. > > > > > + * 1. MMU should be disabled. > > > > > + * 2. a0 should contain hartid > > > > > + * 3. a1 should DT address > > > > > + */ > > > > > +void __noreturn efi_enter_kernel(unsigned long entrypoint, unsigned long fdt, > > > > > + unsigned long fdt_size) > > > > > +{ > > > > > + unsigned long kernel_entry = entrypoint + (unsigned long)stext_offset; > > > > > + jump_kernel_func jump_kernel = (jump_kernel_func) kernel_entry; > > > > > + u32 hartid = get_boot_hartid_from_fdt(fdt); > > > > > + > > > > > + if (hartid == U32_MAX) > > > > > + /* We can not use panic or BUG at this point */ > > > > > + __asm__ __volatile__ ("ebreak"); > > > > > + /* Disable MMU */ > > > > > + csr_write(CSR_SATP, 0); > > > > > + jump_kernel(hartid, fdt); > > > > > +} > > > > > + > > > > > +efi_status_t handle_kernel_image(unsigned long *image_addr, > > > > > + unsigned long *image_size, > > > > > + unsigned long *reserve_addr, > > > > > + unsigned long *reserve_size, > > > > > + unsigned long dram_base, > > > > > + efi_loaded_image_t *image) > > > > > +{ > > > > > + efi_status_t status; > > > > > + unsigned long kernel_size, kernel_memsize = 0; > > > > > + unsigned long max_alloc_address; > > > > > + > > > > > + if (image->image_base != _start) > > > > > + pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n"); > > > > > + > > > > > > > > I don't think you need this. > > > > > > > > > > Sure. I will remove it. I guess ARM64 code has the error print for > > > legacy loader code ? > > > > > > > No, for broken distro versions of GRUB > > > > > > > + kernel_size = _edata - _start; > > > > > + kernel_memsize = kernel_size + (_end - _edata); > > > > > + max_alloc_address = round_up(dram_base, MIN_KIMG_ALIGN) + > > > > > + kernel_memsize; > > > > > + > > > > > > > > You said the kernel could be anywhere in memory, as long as it is > > > > aligned correctly, right? > > > > > > Sorry I was wrong about this. RISC-V kernel maps PAGE_OFFSET virtual > > > address to the > > > physical address <xyz> where it is booted. That means memory between > > > dram start and and <xyz> address > > > will be unusable. > > > > > > > OK > > > > > I also realized that the above computing max_address as above also > > > won't work for the following reason. > > > efi_allocate_pages_aligned actually ALIGN_DOWN the max_address. Thus, > > > efi won't find enough > > > free memory in this case if the max_address is computed from the dram_base. > > > > > > Is there an implicit requirement for efi_allocate_pages_aligned or > > > efi_low_alloc_above should be tried in case of failure? > > > > > > > No not really. What ever works for your particular use case is acceptable to me. > > > > > > In that case, you don't need this, you can simply pass ULONG_MAX as > > > > the max address. > > > > > > > As RISC-V should allocate memory as low as possible to avoid memory > > > wastage, I think the following should work. > > > > > > efi_low_alloc_above(*reserve_size, MIN_KIMG_ALIGN, reserve_addr, dram_base); > > > > > > If this looks okay to you, efi_low_alloc_above should be moved back to > > > mem.c from relocate.c. > > > Should I do it in a separate patch or the original patch should be > > > modified so that efi_low_alloc_above was never moved to relocate.c > > > > > > > No, please keep efi_low_alloc_above() where it is, but drop the > > static, and put back the declaration in efistub.h > > > > Alternatively, can you check whether efi_relocate_kernel() already > does what you need? Yeah. efi_relocate_kernel works too. It's just that efi_relocate_kernel expects a preferred address which RISC-V doesn't care about. I can pass ULONG_MAX so that efi_bs_call will fail and efi_low_alloc_above will be invoked eventually. I am also thinking to put a check in relocate_kernel for preferred address != ULONG_MAX to avoid an extra efi_bs_call. The extra call doesn't actually create an issue, just a minor optimization. But if that is not acceptable for generic code, that is fine as well. Thanks for your suggestions and time.
On Sat, 18 Apr 2020 at 21:19, Atish Patra <atishp@atishpatra.org> wrote: > > On Sat, Apr 18, 2020 at 5:39 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Sat, 18 Apr 2020 at 12:51, Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > On Sat, 18 Apr 2020 at 05:03, Atish Patra <atishp@atishpatra.org> wrote: > > > > > > > > On Thu, Apr 16, 2020 at 12:41 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > > > > > On Wed, 15 Apr 2020 at 21:54, Atish Patra <atish.patra@wdc.com> wrote: > > > > > > > > > > > > Add a RISC-V architecture specific stub code that actually copies the > > > > > > actual kernel image to a valid address and jump to it after boot services > > > > > > are terminated. Enable UEFI related kernel configs as well for RISC-V. > > > > > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > > > > --- > > > > > > arch/riscv/Kconfig | 20 ++++ > > > > > > arch/riscv/Makefile | 1 + > > > > > > arch/riscv/configs/defconfig | 1 + > > > > > > arch/riscv/include/asm/efi.h | 44 +++++++++ > > > > > > drivers/firmware/efi/Kconfig | 2 +- > > > > > > drivers/firmware/efi/libstub/Makefile | 7 ++ > > > > > > drivers/firmware/efi/libstub/riscv-stub.c | 111 ++++++++++++++++++++++ > > > > > > 7 files changed, 185 insertions(+), 1 deletion(-) > > > > > > create mode 100644 arch/riscv/include/asm/efi.h > > > > > > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > index f39e326a7a42..eb4f41c8f3ce 100644 > > > > > > --- a/arch/riscv/Kconfig > > > > > > +++ b/arch/riscv/Kconfig > > > > > > @@ -379,10 +379,30 @@ config CMDLINE_FORCE > > > > > > > > > > > > endchoice > > > > > > > > > > > > +config EFI_STUB > > > > > > + bool > > > > > > + > > > > > > +config EFI > > > > > > + bool "UEFI runtime support" > > > > > > + depends on OF > > > > > > + select LIBFDT > > > > > > + select UCS2_STRING > > > > > > + select EFI_PARAMS_FROM_FDT > > > > > > + select EFI_STUB > > > > > > + select EFI_GENERIC_STUB > > > > > > + default y > > > > > > + help > > > > > > + This option provides support for runtime services provided > > > > > > + by UEFI firmware (such as non-volatile variables, realtime > > > > > > + clock, and platform reset). A UEFI stub is also provided to > > > > > > + allow the kernel to be booted as an EFI application. This > > > > > > + is only useful on systems that have UEFI firmware. > > > > > > + > > > > > > endmenu > > > > > > > > > > > > menu "Power management options" > > > > > > > > > > > > source "kernel/power/Kconfig" > > > > > > +source "drivers/firmware/Kconfig" > > > > > > > > > > > > endmenu > > > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > > > > > index fb6e37db836d..079435804d6d 100644 > > > > > > --- a/arch/riscv/Makefile > > > > > > +++ b/arch/riscv/Makefile > > > > > > @@ -80,6 +80,7 @@ head-y := arch/riscv/kernel/head.o > > > > > > core-y += arch/riscv/ > > > > > > > > > > > > libs-y += arch/riscv/lib/ > > > > > > +core-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a > > > > > > > > > > > > PHONY += vdso_install > > > > > > vdso_install: > > > > > > diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig > > > > > > index 4da4886246a4..ae69e12d306a 100644 > > > > > > --- a/arch/riscv/configs/defconfig > > > > > > +++ b/arch/riscv/configs/defconfig > > > > > > @@ -129,3 +129,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > > > > > > # CONFIG_RUNTIME_TESTING_MENU is not set > > > > > > CONFIG_MEMTEST=y > > > > > > # CONFIG_SYSFS_SYSCALL is not set > > > > > > +CONFIG_EFI=y > > > > > > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h > > > > > > new file mode 100644 > > > > > > index 000000000000..62d7d5eafed8 > > > > > > --- /dev/null > > > > > > +++ b/arch/riscv/include/asm/efi.h > > > > > > @@ -0,0 +1,44 @@ > > > > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > > > +/* > > > > > > + * Copyright (C) 2020 Western Digital Corporation or its affiliates. > > > > > > + * Based on arch/arm64/include/asm/efi.h > > > > > > + */ > > > > > > +#ifndef _ASM_EFI_H > > > > > > +#define _ASM_EFI_H > > > > > > + > > > > > > +#include <asm/io.h> > > > > > > +#include <asm/mmu_context.h> > > > > > > +#include <asm/ptrace.h> > > > > > > +#include <asm/tlbflush.h> > > > > > > + > > > > > > +#define VA_BITS_MIN 39 > > > > > > + > > > > > > +/* on RISC-V, the FDT may be located anywhere in system RAM */ > > > > > > +static inline unsigned long efi_get_max_fdt_addr(unsigned long dram_base) > > > > > > +{ > > > > > > + return ULONG_MAX; > > > > > > +} > > > > > > + > > > > > > +/* Load initrd at enough distance from DRAM start */ > > > > > > +static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base, > > > > > > + unsigned long image_addr) > > > > > > +{ > > > > > > + return dram_base + SZ_256M; > > > > > > +} > > > > > > + > > > > > > +#define efi_bs_call(func, ...) efi_system_table()->boottime->func(__VA_ARGS__) > > > > > > +#define efi_rt_call(func, ...) efi_system_table()->runtime->func(__VA_ARGS__) > > > > > > +#define efi_is_native() (true) > > > > > > + > > > > > > +#define efi_table_attr(inst, attr) (inst->attr) > > > > > > + > > > > > > +#define efi_call_proto(inst, func, ...) inst->func(inst, ##__VA_ARGS__) > > > > > > + > > > > > > +#define alloc_screen_info(x...) (&screen_info) > > > > > > +extern char stext_offset[]; > > > > > > + > > > > > > +static inline void free_screen_info(struct screen_info *si) > > > > > > +{ > > > > > > +} > > > > > > + > > > > > > +#endif /* _ASM_EFI_H */ > > > > > > diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig > > > > > > index 2a2b2b96a1dc..fcdc789d3f87 100644 > > > > > > --- a/drivers/firmware/efi/Kconfig > > > > > > +++ b/drivers/firmware/efi/Kconfig > > > > > > @@ -111,7 +111,7 @@ config EFI_GENERIC_STUB > > > > > > > > > > > > config EFI_ARMSTUB_DTB_LOADER > > > > > > bool "Enable the DTB loader" > > > > > > - depends on EFI_GENERIC_STUB > > > > > > + depends on EFI_GENERIC_STUB && !RISCV > > > > > > default y > > > > > > help > > > > > > Select this config option to add support for the dtb= command > > > > > > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > > > > > > index 2b4e09bf987c..7d46b70b51f2 100644 > > > > > > --- a/drivers/firmware/efi/libstub/Makefile > > > > > > +++ b/drivers/firmware/efi/libstub/Makefile > > > > > > @@ -22,6 +22,8 @@ cflags-$(CONFIG_ARM64) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > > cflags-$(CONFIG_ARM) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > > -fno-builtin -fpic \ > > > > > > $(call cc-option,-mno-single-pic-base) > > > > > > +cflags-$(CONFIG_RISCV) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > > + -fpic > > > > > > > > > > > > cflags-$(CONFIG_EFI_GENERIC_STUB) += -I$(srctree)/scripts/dtc/libfdt > > > > > > > > > > > > @@ -57,6 +59,7 @@ lib-$(CONFIG_EFI_GENERIC_STUB) += efi-stub.o fdt.o string.o \ > > > > > > lib-$(CONFIG_ARM) += arm32-stub.o > > > > > > lib-$(CONFIG_ARM64) += arm64-stub.o > > > > > > lib-$(CONFIG_X86) += x86-stub.o > > > > > > +lib-$(CONFIG_RISCV) += riscv-stub.o > > > > > > CFLAGS_arm32-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > > > CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > > > > > > > > > @@ -81,6 +84,10 @@ STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \ > > > > > > --prefix-symbols=__efistub_ > > > > > > STUBCOPY_RELOC-$(CONFIG_ARM64) := R_AARCH64_ABS > > > > > > > > > > > > +STUBCOPY_FLAGS-$(CONFIG_RISCV) += --prefix-alloc-sections=.init \ > > > > > > + --prefix-symbols=__efistub_ > > > > > > +STUBCOPY_RELOC-$(CONFIG_RISCV) := R_RISCV_HI20 > > > > > > + > > > > > > $(obj)/%.stub.o: $(obj)/%.o FORCE > > > > > > $(call if_changed,stubcopy) > > > > > > > > > > > > diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c > > > > > > new file mode 100644 > > > > > > index 000000000000..69d13e0ebaea > > > > > > --- /dev/null > > > > > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > > > > > > @@ -0,0 +1,111 @@ > > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > > +/* > > > > > > + * Copyright (C) 2013, 2014 Linaro Ltd; <roy.franz@linaro.org> > > > > > > + * Copyright (C) 2020 Western Digital Corporation or its affiliates. > > > > > > + * > > > > > > + * This file implements the EFI boot stub for the RISC-V kernel. > > > > > > + * Adapted from ARM64 version at drivers/firmware/efi/libstub/arm64-stub.c. > > > > > > + */ > > > > > > + > > > > > > +#include <linux/efi.h> > > > > > > +#include <linux/libfdt.h> > > > > > > +#include <linux/libfdt_env.h> > > > > > > +#include <asm/efi.h> > > > > > > +#include <asm/sections.h> > > > > > > + > > > > > > +#include "efistub.h" > > > > > > +/* > > > > > > + * RISCV requires the kernel image to placed TEXT_OFFSET bytes beyond a 2 MB > > > > > > + * aligned base for 64 bit and 4MB for 32 bit. > > > > > > + */ > > > > > > > > Fixed the comment. > > > > > > > > > > +#ifdef CONFIG_64BIT > > > > > > +#define MIN_KIMG_ALIGN SZ_2M > > > > > > +#else > > > > > > +#define MIN_KIMG_ALIGN SZ_4M > > > > > > +#endif > > > > > > + > > > > > > +typedef __attribute__((noreturn)) void (*jump_kernel_func)(unsigned int, > > > > > > + unsigned long); > > > > > > +efi_status_t check_platform_features(void) > > > > > > +{ > > > > > > + return EFI_SUCCESS; > > > > > > +} > > > > > > + > > > > > > +static u32 get_boot_hartid_from_fdt(unsigned long fdt) > > > > > > +{ > > > > > > + int chosen_node, len; > > > > > > + const fdt32_t *prop; > > > > > > + > > > > > > + chosen_node = fdt_path_offset((void *)fdt, "/chosen"); > > > > > > + if (chosen_node < 0) > > > > > > + return U32_MAX; > > > > > > + prop = fdt_getprop((void *)fdt, chosen_node, "boot-hartid", &len); > > > > > > + if (!prop || len != sizeof(u32)) > > > > > > + return U32_MAX; > > > > > > + > > > > > > + return fdt32_to_cpu(*prop); > > > > > > +} > > > > > > + > > > > > > +/* > > > > > > + * Jump to real kernel here with following constraints. > > > > > > + * 1. MMU should be disabled. > > > > > > + * 2. a0 should contain hartid > > > > > > + * 3. a1 should DT address > > > > > > + */ > > > > > > +void __noreturn efi_enter_kernel(unsigned long entrypoint, unsigned long fdt, > > > > > > + unsigned long fdt_size) > > > > > > +{ > > > > > > + unsigned long kernel_entry = entrypoint + (unsigned long)stext_offset; > > > > > > + jump_kernel_func jump_kernel = (jump_kernel_func) kernel_entry; > > > > > > + u32 hartid = get_boot_hartid_from_fdt(fdt); > > > > > > + > > > > > > + if (hartid == U32_MAX) > > > > > > + /* We can not use panic or BUG at this point */ > > > > > > + __asm__ __volatile__ ("ebreak"); > > > > > > + /* Disable MMU */ > > > > > > + csr_write(CSR_SATP, 0); > > > > > > + jump_kernel(hartid, fdt); > > > > > > +} > > > > > > + > > > > > > +efi_status_t handle_kernel_image(unsigned long *image_addr, > > > > > > + unsigned long *image_size, > > > > > > + unsigned long *reserve_addr, > > > > > > + unsigned long *reserve_size, > > > > > > + unsigned long dram_base, > > > > > > + efi_loaded_image_t *image) > > > > > > +{ > > > > > > + efi_status_t status; > > > > > > + unsigned long kernel_size, kernel_memsize = 0; > > > > > > + unsigned long max_alloc_address; > > > > > > + > > > > > > + if (image->image_base != _start) > > > > > > + pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n"); > > > > > > + > > > > > > > > > > I don't think you need this. > > > > > > > > > > > > > Sure. I will remove it. I guess ARM64 code has the error print for > > > > legacy loader code ? > > > > > > > > > > No, for broken distro versions of GRUB > > > > > > > > > + kernel_size = _edata - _start; > > > > > > + kernel_memsize = kernel_size + (_end - _edata); > > > > > > + max_alloc_address = round_up(dram_base, MIN_KIMG_ALIGN) + > > > > > > + kernel_memsize; > > > > > > + > > > > > > > > > > You said the kernel could be anywhere in memory, as long as it is > > > > > aligned correctly, right? > > > > > > > > Sorry I was wrong about this. RISC-V kernel maps PAGE_OFFSET virtual > > > > address to the > > > > physical address <xyz> where it is booted. That means memory between > > > > dram start and and <xyz> address > > > > will be unusable. > > > > > > > > > > OK > > > > > > > I also realized that the above computing max_address as above also > > > > won't work for the following reason. > > > > efi_allocate_pages_aligned actually ALIGN_DOWN the max_address. Thus, > > > > efi won't find enough > > > > free memory in this case if the max_address is computed from the dram_base. > > > > > > > > Is there an implicit requirement for efi_allocate_pages_aligned or > > > > efi_low_alloc_above should be tried in case of failure? > > > > > > > > > > No not really. What ever works for your particular use case is acceptable to me. > > > > > > > > In that case, you don't need this, you can simply pass ULONG_MAX as > > > > > the max address. > > > > > > > > > As RISC-V should allocate memory as low as possible to avoid memory > > > > wastage, I think the following should work. > > > > > > > > efi_low_alloc_above(*reserve_size, MIN_KIMG_ALIGN, reserve_addr, dram_base); > > > > > > > > If this looks okay to you, efi_low_alloc_above should be moved back to > > > > mem.c from relocate.c. > > > > Should I do it in a separate patch or the original patch should be > > > > modified so that efi_low_alloc_above was never moved to relocate.c > > > > > > > > > > No, please keep efi_low_alloc_above() where it is, but drop the > > > static, and put back the declaration in efistub.h > > > > > > > Alternatively, can you check whether efi_relocate_kernel() already > > does what you need? > > Yeah. efi_relocate_kernel works too. It's just that > efi_relocate_kernel expects a preferred address > which RISC-V doesn't care about. There is a preferred address, no? The base of DRAM? > I can pass ULONG_MAX so that > efi_bs_call will fail and > efi_low_alloc_above will be invoked eventually. > > I am also thinking to put a check in relocate_kernel for preferred > address != ULONG_MAX to avoid > an extra efi_bs_call. The extra call doesn't actually create an issue, > just a minor optimization. > But if that is not acceptable for generic code, that is fine as well. > > Thanks for your suggestions and time. > -- > Regards, > Atish
On Sat, 2020-04-18 at 21:24 +0200, Ard Biesheuvel wrote: > On Sat, 18 Apr 2020 at 21:19, Atish Patra <atishp@atishpatra.org> > wrote: > > On Sat, Apr 18, 2020 at 5:39 AM Ard Biesheuvel <ardb@kernel.org> > > wrote: > > > On Sat, 18 Apr 2020 at 12:51, Ard Biesheuvel <ardb@kernel.org> > > > wrote: > > > > On Sat, 18 Apr 2020 at 05:03, Atish Patra < > > > > atishp@atishpatra.org> wrote: > > > > > On Thu, Apr 16, 2020 at 12:41 AM Ard Biesheuvel < > > > > > ardb@kernel.org> wrote: > > > > > > On Wed, 15 Apr 2020 at 21:54, Atish Patra < > > > > > > atish.patra@wdc.com> wrote: > > > > > > > Add a RISC-V architecture specific stub code that > > > > > > > actually copies the > > > > > > > actual kernel image to a valid address and jump to it > > > > > > > after boot services > > > > > > > are terminated. Enable UEFI related kernel configs as > > > > > > > well for RISC-V. > > > > > > > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > > > > > --- > > > > > > > arch/riscv/Kconfig | 20 ++++ > > > > > > > arch/riscv/Makefile | 1 + > > > > > > > arch/riscv/configs/defconfig | 1 + > > > > > > > arch/riscv/include/asm/efi.h | 44 > > > > > > > +++++++++ > > > > > > > drivers/firmware/efi/Kconfig | 2 +- > > > > > > > drivers/firmware/efi/libstub/Makefile | 7 ++ > > > > > > > drivers/firmware/efi/libstub/riscv-stub.c | 111 > > > > > > > ++++++++++++++++++++++ > > > > > > > 7 files changed, 185 insertions(+), 1 deletion(-) > > > > > > > create mode 100644 arch/riscv/include/asm/efi.h > > > > > > > create mode 100644 drivers/firmware/efi/libstub/riscv- > > > > > > > stub.c > > > > > > > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > > index f39e326a7a42..eb4f41c8f3ce 100644 > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > @@ -379,10 +379,30 @@ config CMDLINE_FORCE > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > +config EFI_STUB > > > > > > > + bool > > > > > > > + > > > > > > > +config EFI > > > > > > > + bool "UEFI runtime support" > > > > > > > + depends on OF > > > > > > > + select LIBFDT > > > > > > > + select UCS2_STRING > > > > > > > + select EFI_PARAMS_FROM_FDT > > > > > > > + select EFI_STUB > > > > > > > + select EFI_GENERIC_STUB > > > > > > > + default y > > > > > > > + help > > > > > > > + This option provides support for runtime > > > > > > > services provided > > > > > > > + by UEFI firmware (such as non-volatile > > > > > > > variables, realtime > > > > > > > + clock, and platform reset). A UEFI stub is > > > > > > > also provided to > > > > > > > + allow the kernel to be booted as an EFI > > > > > > > application. This > > > > > > > + is only useful on systems that have UEFI > > > > > > > firmware. > > > > > > > + > > > > > > > endmenu > > > > > > > > > > > > > > menu "Power management options" > > > > > > > > > > > > > > source "kernel/power/Kconfig" > > > > > > > +source "drivers/firmware/Kconfig" > > > > > > > > > > > > > > endmenu > > > > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > > > > > > index fb6e37db836d..079435804d6d 100644 > > > > > > > --- a/arch/riscv/Makefile > > > > > > > +++ b/arch/riscv/Makefile > > > > > > > @@ -80,6 +80,7 @@ head-y := arch/riscv/kernel/head.o > > > > > > > core-y += arch/riscv/ > > > > > > > > > > > > > > libs-y += arch/riscv/lib/ > > > > > > > +core-$(CONFIG_EFI_STUB) += > > > > > > > $(objtree)/drivers/firmware/efi/libstub/lib.a > > > > > > > > > > > > > > PHONY += vdso_install > > > > > > > vdso_install: > > > > > > > diff --git a/arch/riscv/configs/defconfig > > > > > > > b/arch/riscv/configs/defconfig > > > > > > > index 4da4886246a4..ae69e12d306a 100644 > > > > > > > --- a/arch/riscv/configs/defconfig > > > > > > > +++ b/arch/riscv/configs/defconfig > > > > > > > @@ -129,3 +129,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > > > > > > > # CONFIG_RUNTIME_TESTING_MENU is not set > > > > > > > CONFIG_MEMTEST=y > > > > > > > # CONFIG_SYSFS_SYSCALL is not set > > > > > > > +CONFIG_EFI=y > > > > > > > diff --git a/arch/riscv/include/asm/efi.h > > > > > > > b/arch/riscv/include/asm/efi.h > > > > > > > new file mode 100644 > > > > > > > index 000000000000..62d7d5eafed8 > > > > > > > --- /dev/null > > > > > > > +++ b/arch/riscv/include/asm/efi.h > > > > > > > @@ -0,0 +1,44 @@ > > > > > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > > > > +/* > > > > > > > + * Copyright (C) 2020 Western Digital Corporation or its > > > > > > > affiliates. > > > > > > > + * Based on arch/arm64/include/asm/efi.h > > > > > > > + */ > > > > > > > +#ifndef _ASM_EFI_H > > > > > > > +#define _ASM_EFI_H > > > > > > > + > > > > > > > +#include <asm/io.h> > > > > > > > +#include <asm/mmu_context.h> > > > > > > > +#include <asm/ptrace.h> > > > > > > > +#include <asm/tlbflush.h> > > > > > > > + > > > > > > > +#define VA_BITS_MIN 39 > > > > > > > + > > > > > > > +/* on RISC-V, the FDT may be located anywhere in system > > > > > > > RAM */ > > > > > > > +static inline unsigned long > > > > > > > efi_get_max_fdt_addr(unsigned long dram_base) > > > > > > > +{ > > > > > > > + return ULONG_MAX; > > > > > > > +} > > > > > > > + > > > > > > > +/* Load initrd at enough distance from DRAM start */ > > > > > > > +static inline unsigned long > > > > > > > efi_get_max_initrd_addr(unsigned long dram_base, > > > > > > > + unsig > > > > > > > ned long image_addr) > > > > > > > +{ > > > > > > > + return dram_base + SZ_256M; > > > > > > > +} > > > > > > > + > > > > > > > +#define efi_bs_call(func, ...) efi_system_table()- > > > > > > > >boottime->func(__VA_ARGS__) > > > > > > > +#define efi_rt_call(func, ...) efi_system_table()- > > > > > > > >runtime->func(__VA_ARGS__) > > > > > > > +#define efi_is_native() (true) > > > > > > > + > > > > > > > +#define efi_table_attr(inst, attr) (inst->attr) > > > > > > > + > > > > > > > +#define efi_call_proto(inst, func, ...) inst->func(inst, > > > > > > > ##__VA_ARGS__) > > > > > > > + > > > > > > > +#define > > > > > > > alloc_screen_info(x...) (&screen_info) > > > > > > > +extern char stext_offset[]; > > > > > > > + > > > > > > > +static inline void free_screen_info(struct screen_info > > > > > > > *si) > > > > > > > +{ > > > > > > > +} > > > > > > > + > > > > > > > +#endif /* _ASM_EFI_H */ > > > > > > > diff --git a/drivers/firmware/efi/Kconfig > > > > > > > b/drivers/firmware/efi/Kconfig > > > > > > > index 2a2b2b96a1dc..fcdc789d3f87 100644 > > > > > > > --- a/drivers/firmware/efi/Kconfig > > > > > > > +++ b/drivers/firmware/efi/Kconfig > > > > > > > @@ -111,7 +111,7 @@ config EFI_GENERIC_STUB > > > > > > > > > > > > > > config EFI_ARMSTUB_DTB_LOADER > > > > > > > bool "Enable the DTB loader" > > > > > > > - depends on EFI_GENERIC_STUB > > > > > > > + depends on EFI_GENERIC_STUB && !RISCV > > > > > > > default y > > > > > > > help > > > > > > > Select this config option to add support for > > > > > > > the dtb= command > > > > > > > diff --git a/drivers/firmware/efi/libstub/Makefile > > > > > > > b/drivers/firmware/efi/libstub/Makefile > > > > > > > index 2b4e09bf987c..7d46b70b51f2 100644 > > > > > > > --- a/drivers/firmware/efi/libstub/Makefile > > > > > > > +++ b/drivers/firmware/efi/libstub/Makefile > > > > > > > @@ -22,6 +22,8 @@ cflags- > > > > > > > $(CONFIG_ARM64) := $(subst > > > > > > > $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > > > cflags-$(CONFIG_ARM) := $(subst > > > > > > > $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > > > -fno-builtin -fpic \ > > > > > > > $(call cc-option,-mno- > > > > > > > single-pic-base) > > > > > > > +cflags-$(CONFIG_RISCV) := $(subst > > > > > > > $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > > > + -fpic > > > > > > > > > > > > > > cflags-$(CONFIG_EFI_GENERIC_STUB) += > > > > > > > -I$(srctree)/scripts/dtc/libfdt > > > > > > > > > > > > > > @@ -57,6 +59,7 @@ lib- > > > > > > > $(CONFIG_EFI_GENERIC_STUB) += efi-stub.o > > > > > > > fdt.o string.o \ > > > > > > > lib-$(CONFIG_ARM) += arm32-stub.o > > > > > > > lib-$(CONFIG_ARM64) += arm64-stub.o > > > > > > > lib-$(CONFIG_X86) += x86-stub.o > > > > > > > +lib-$(CONFIG_RISCV) += riscv-stub.o > > > > > > > CFLAGS_arm32-stub.o := > > > > > > > -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > > > > CFLAGS_arm64-stub.o := > > > > > > > -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > > > > > > > > > > > @@ -81,6 +84,10 @@ STUBCOPY_FLAGS- > > > > > > > $(CONFIG_ARM64) += --prefix-alloc-sections=.init \ > > > > > > > --prefix- > > > > > > > symbols=__efistub_ > > > > > > > STUBCOPY_RELOC-$(CONFIG_ARM64) := R_AARCH64_ABS > > > > > > > > > > > > > > +STUBCOPY_FLAGS-$(CONFIG_RISCV) += --prefix-alloc- > > > > > > > sections=.init \ > > > > > > > + --prefix- > > > > > > > symbols=__efistub_ > > > > > > > +STUBCOPY_RELOC-$(CONFIG_RISCV) := R_RISCV_HI20 > > > > > > > + > > > > > > > $(obj)/%.stub.o: $(obj)/%.o FORCE > > > > > > > $(call if_changed,stubcopy) > > > > > > > > > > > > > > diff --git a/drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > b/drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > new file mode 100644 > > > > > > > index 000000000000..69d13e0ebaea > > > > > > > --- /dev/null > > > > > > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > @@ -0,0 +1,111 @@ > > > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > > > +/* > > > > > > > + * Copyright (C) 2013, 2014 Linaro Ltd; < > > > > > > > roy.franz@linaro.org> > > > > > > > + * Copyright (C) 2020 Western Digital Corporation or its > > > > > > > affiliates. > > > > > > > + * > > > > > > > + * This file implements the EFI boot stub for the RISC-V > > > > > > > kernel. > > > > > > > + * Adapted from ARM64 version at > > > > > > > drivers/firmware/efi/libstub/arm64-stub.c. > > > > > > > + */ > > > > > > > + > > > > > > > +#include <linux/efi.h> > > > > > > > +#include <linux/libfdt.h> > > > > > > > +#include <linux/libfdt_env.h> > > > > > > > +#include <asm/efi.h> > > > > > > > +#include <asm/sections.h> > > > > > > > + > > > > > > > +#include "efistub.h" > > > > > > > +/* > > > > > > > + * RISCV requires the kernel image to placed TEXT_OFFSET > > > > > > > bytes beyond a 2 MB > > > > > > > + * aligned base for 64 bit and 4MB for 32 bit. > > > > > > > + */ > > > > > > > > > > Fixed the comment. > > > > > > > > > > > > +#ifdef CONFIG_64BIT > > > > > > > +#define MIN_KIMG_ALIGN SZ_2M > > > > > > > +#else > > > > > > > +#define MIN_KIMG_ALIGN SZ_4M > > > > > > > +#endif > > > > > > > + > > > > > > > +typedef __attribute__((noreturn)) void > > > > > > > (*jump_kernel_func)(unsigned int, > > > > > > > + > > > > > > > unsigned long); > > > > > > > +efi_status_t check_platform_features(void) > > > > > > > +{ > > > > > > > + return EFI_SUCCESS; > > > > > > > +} > > > > > > > + > > > > > > > +static u32 get_boot_hartid_from_fdt(unsigned long fdt) > > > > > > > +{ > > > > > > > + int chosen_node, len; > > > > > > > + const fdt32_t *prop; > > > > > > > + > > > > > > > + chosen_node = fdt_path_offset((void *)fdt, > > > > > > > "/chosen"); > > > > > > > + if (chosen_node < 0) > > > > > > > + return U32_MAX; > > > > > > > + prop = fdt_getprop((void *)fdt, chosen_node, > > > > > > > "boot-hartid", &len); > > > > > > > + if (!prop || len != sizeof(u32)) > > > > > > > + return U32_MAX; > > > > > > > + > > > > > > > + return fdt32_to_cpu(*prop); > > > > > > > +} > > > > > > > + > > > > > > > +/* > > > > > > > + * Jump to real kernel here with following constraints. > > > > > > > + * 1. MMU should be disabled. > > > > > > > + * 2. a0 should contain hartid > > > > > > > + * 3. a1 should DT address > > > > > > > + */ > > > > > > > +void __noreturn efi_enter_kernel(unsigned long > > > > > > > entrypoint, unsigned long fdt, > > > > > > > + unsigned long fdt_size) > > > > > > > +{ > > > > > > > + unsigned long kernel_entry = entrypoint + > > > > > > > (unsigned long)stext_offset; > > > > > > > + jump_kernel_func jump_kernel = (jump_kernel_func) > > > > > > > kernel_entry; > > > > > > > + u32 hartid = get_boot_hartid_from_fdt(fdt); > > > > > > > + > > > > > > > + if (hartid == U32_MAX) > > > > > > > + /* We can not use panic or BUG at this > > > > > > > point */ > > > > > > > + __asm__ __volatile__ ("ebreak"); > > > > > > > + /* Disable MMU */ > > > > > > > + csr_write(CSR_SATP, 0); > > > > > > > + jump_kernel(hartid, fdt); > > > > > > > +} > > > > > > > + > > > > > > > +efi_status_t handle_kernel_image(unsigned long > > > > > > > *image_addr, > > > > > > > + unsigned long > > > > > > > *image_size, > > > > > > > + unsigned long > > > > > > > *reserve_addr, > > > > > > > + unsigned long > > > > > > > *reserve_size, > > > > > > > + unsigned long dram_base, > > > > > > > + efi_loaded_image_t > > > > > > > *image) > > > > > > > +{ > > > > > > > + efi_status_t status; > > > > > > > + unsigned long kernel_size, kernel_memsize = 0; > > > > > > > + unsigned long max_alloc_address; > > > > > > > + > > > > > > > + if (image->image_base != _start) > > > > > > > + pr_efi_err("FIRMWARE BUG: > > > > > > > efi_loaded_image_t::image_base has bogus value\n"); > > > > > > > + > > > > > > > > > > > > I don't think you need this. > > > > > > > > > > > > > > > > Sure. I will remove it. I guess ARM64 code has the error > > > > > print for > > > > > legacy loader code ? > > > > > > > > > > > > > No, for broken distro versions of GRUB > > > > > > > > > > > + kernel_size = _edata - _start; > > > > > > > + kernel_memsize = kernel_size + (_end - _edata); > > > > > > > + max_alloc_address = round_up(dram_base, > > > > > > > MIN_KIMG_ALIGN) + > > > > > > > + kernel_memsize; > > > > > > > + > > > > > > > > > > > > You said the kernel could be anywhere in memory, as long as > > > > > > it is > > > > > > aligned correctly, right? > > > > > > > > > > Sorry I was wrong about this. RISC-V kernel maps PAGE_OFFSET > > > > > virtual > > > > > address to the > > > > > physical address <xyz> where it is booted. That means memory > > > > > between > > > > > dram start and and <xyz> address > > > > > will be unusable. > > > > > > > > > > > > > OK > > > > > > > > > I also realized that the above computing max_address as above > > > > > also > > > > > won't work for the following reason. > > > > > efi_allocate_pages_aligned actually ALIGN_DOWN the > > > > > max_address. Thus, > > > > > efi won't find enough > > > > > free memory in this case if the max_address is computed from > > > > > the dram_base. > > > > > > > > > > Is there an implicit requirement for > > > > > efi_allocate_pages_aligned or > > > > > efi_low_alloc_above should be tried in case of failure? > > > > > > > > > > > > > No not really. What ever works for your particular use case is > > > > acceptable to me. > > > > > > > > > > In that case, you don't need this, you can simply pass > > > > > > ULONG_MAX as > > > > > > the max address. > > > > > > > > > > > As RISC-V should allocate memory as low as possible to avoid > > > > > memory > > > > > wastage, I think the following should work. > > > > > > > > > > efi_low_alloc_above(*reserve_size, MIN_KIMG_ALIGN, > > > > > reserve_addr, dram_base); > > > > > > > > > > If this looks okay to you, efi_low_alloc_above should be > > > > > moved back to > > > > > mem.c from relocate.c. > > > > > Should I do it in a separate patch or the original patch > > > > > should be > > > > > modified so that efi_low_alloc_above was never moved to > > > > > relocate.c > > > > > > > > > > > > > No, please keep efi_low_alloc_above() where it is, but drop the > > > > static, and put back the declaration in efistub.h > > > > > > > > > > Alternatively, can you check whether efi_relocate_kernel() > > > already > > > does what you need? > > > > Yeah. efi_relocate_kernel works too. It's just that > > efi_relocate_kernel expects a preferred address > > which RISC-V doesn't care about. > > There is a preferred address, no? The base of DRAM? > I just realized that my reply from my gmail never landed in any of the mailing lists. If you have already received it, sorry for the spam. "If the preferred address is set as the base of DRAM, efi_bs_call is bound to fail as well because the base of DRAM is reserved by the firmware. So the efi memory allocator can't allocate at that address. Technically, it will work but it is no different than passing ULONG_MAX. So I thought ULONG_MAX will avoid the confusion. We try to allocate as low as possible so I am passing dram_base as the minimum address anyways. As the firmware reserved the first few KBs, > > I can pass ULONG_MAX so that > > efi_bs_call will fail and > > efi_low_alloc_above will be invoked eventually. > > > > I am also thinking to put a check in relocate_kernel for preferred > > address != ULONG_MAX to avoid > > an extra efi_bs_call. The extra call doesn't actually create an > > issue, > > just a minor optimization. > > But if that is not acceptable for generic code, that is fine as > > well. > > > > Thanks for your suggestions and time. > > -- > > Regards, > > Atish
On Mon, 20 Apr 2020 at 06:20, Atish Patra <Atish.Patra@wdc.com> wrote: > > On Sat, 2020-04-18 at 21:24 +0200, Ard Biesheuvel wrote: > > On Sat, 18 Apr 2020 at 21:19, Atish Patra <atishp@atishpatra.org> > > wrote: > > > On Sat, Apr 18, 2020 at 5:39 AM Ard Biesheuvel <ardb@kernel.org> > > > wrote: > > > > On Sat, 18 Apr 2020 at 12:51, Ard Biesheuvel <ardb@kernel.org> > > > > wrote: > > > > > On Sat, 18 Apr 2020 at 05:03, Atish Patra < > > > > > atishp@atishpatra.org> wrote: > > > > > > On Thu, Apr 16, 2020 at 12:41 AM Ard Biesheuvel < > > > > > > ardb@kernel.org> wrote: > > > > > > > On Wed, 15 Apr 2020 at 21:54, Atish Patra < > > > > > > > atish.patra@wdc.com> wrote: > > > > > > > > Add a RISC-V architecture specific stub code that > > > > > > > > actually copies the > > > > > > > > actual kernel image to a valid address and jump to it > > > > > > > > after boot services > > > > > > > > are terminated. Enable UEFI related kernel configs as > > > > > > > > well for RISC-V. > > > > > > > > > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > > > > > > --- > > > > > > > > arch/riscv/Kconfig | 20 ++++ > > > > > > > > arch/riscv/Makefile | 1 + > > > > > > > > arch/riscv/configs/defconfig | 1 + > > > > > > > > arch/riscv/include/asm/efi.h | 44 > > > > > > > > +++++++++ > > > > > > > > drivers/firmware/efi/Kconfig | 2 +- > > > > > > > > drivers/firmware/efi/libstub/Makefile | 7 ++ > > > > > > > > drivers/firmware/efi/libstub/riscv-stub.c | 111 > > > > > > > > ++++++++++++++++++++++ > > > > > > > > 7 files changed, 185 insertions(+), 1 deletion(-) > > > > > > > > create mode 100644 arch/riscv/include/asm/efi.h > > > > > > > > create mode 100644 drivers/firmware/efi/libstub/riscv- > > > > > > > > stub.c > > > > > > > > > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > > > index f39e326a7a42..eb4f41c8f3ce 100644 > > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > > @@ -379,10 +379,30 @@ config CMDLINE_FORCE > > > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > > > +config EFI_STUB > > > > > > > > + bool > > > > > > > > + > > > > > > > > +config EFI > > > > > > > > + bool "UEFI runtime support" > > > > > > > > + depends on OF > > > > > > > > + select LIBFDT > > > > > > > > + select UCS2_STRING > > > > > > > > + select EFI_PARAMS_FROM_FDT > > > > > > > > + select EFI_STUB > > > > > > > > + select EFI_GENERIC_STUB > > > > > > > > + default y > > > > > > > > + help > > > > > > > > + This option provides support for runtime > > > > > > > > services provided > > > > > > > > + by UEFI firmware (such as non-volatile > > > > > > > > variables, realtime > > > > > > > > + clock, and platform reset). A UEFI stub is > > > > > > > > also provided to > > > > > > > > + allow the kernel to be booted as an EFI > > > > > > > > application. This > > > > > > > > + is only useful on systems that have UEFI > > > > > > > > firmware. > > > > > > > > + > > > > > > > > endmenu > > > > > > > > > > > > > > > > menu "Power management options" > > > > > > > > > > > > > > > > source "kernel/power/Kconfig" > > > > > > > > +source "drivers/firmware/Kconfig" > > > > > > > > > > > > > > > > endmenu > > > > > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > > > > > > > index fb6e37db836d..079435804d6d 100644 > > > > > > > > --- a/arch/riscv/Makefile > > > > > > > > +++ b/arch/riscv/Makefile > > > > > > > > @@ -80,6 +80,7 @@ head-y := arch/riscv/kernel/head.o > > > > > > > > core-y += arch/riscv/ > > > > > > > > > > > > > > > > libs-y += arch/riscv/lib/ > > > > > > > > +core-$(CONFIG_EFI_STUB) += > > > > > > > > $(objtree)/drivers/firmware/efi/libstub/lib.a > > > > > > > > > > > > > > > > PHONY += vdso_install > > > > > > > > vdso_install: > > > > > > > > diff --git a/arch/riscv/configs/defconfig > > > > > > > > b/arch/riscv/configs/defconfig > > > > > > > > index 4da4886246a4..ae69e12d306a 100644 > > > > > > > > --- a/arch/riscv/configs/defconfig > > > > > > > > +++ b/arch/riscv/configs/defconfig > > > > > > > > @@ -129,3 +129,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > > > > > > > > # CONFIG_RUNTIME_TESTING_MENU is not set > > > > > > > > CONFIG_MEMTEST=y > > > > > > > > # CONFIG_SYSFS_SYSCALL is not set > > > > > > > > +CONFIG_EFI=y > > > > > > > > diff --git a/arch/riscv/include/asm/efi.h > > > > > > > > b/arch/riscv/include/asm/efi.h > > > > > > > > new file mode 100644 > > > > > > > > index 000000000000..62d7d5eafed8 > > > > > > > > --- /dev/null > > > > > > > > +++ b/arch/riscv/include/asm/efi.h > > > > > > > > @@ -0,0 +1,44 @@ > > > > > > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > > > > > +/* > > > > > > > > + * Copyright (C) 2020 Western Digital Corporation or its > > > > > > > > affiliates. > > > > > > > > + * Based on arch/arm64/include/asm/efi.h > > > > > > > > + */ > > > > > > > > +#ifndef _ASM_EFI_H > > > > > > > > +#define _ASM_EFI_H > > > > > > > > + > > > > > > > > +#include <asm/io.h> > > > > > > > > +#include <asm/mmu_context.h> > > > > > > > > +#include <asm/ptrace.h> > > > > > > > > +#include <asm/tlbflush.h> > > > > > > > > + > > > > > > > > +#define VA_BITS_MIN 39 > > > > > > > > + > > > > > > > > +/* on RISC-V, the FDT may be located anywhere in system > > > > > > > > RAM */ > > > > > > > > +static inline unsigned long > > > > > > > > efi_get_max_fdt_addr(unsigned long dram_base) > > > > > > > > +{ > > > > > > > > + return ULONG_MAX; > > > > > > > > +} > > > > > > > > + > > > > > > > > +/* Load initrd at enough distance from DRAM start */ > > > > > > > > +static inline unsigned long > > > > > > > > efi_get_max_initrd_addr(unsigned long dram_base, > > > > > > > > + unsig > > > > > > > > ned long image_addr) > > > > > > > > +{ > > > > > > > > + return dram_base + SZ_256M; > > > > > > > > +} > > > > > > > > + > > > > > > > > +#define efi_bs_call(func, ...) efi_system_table()- > > > > > > > > >boottime->func(__VA_ARGS__) > > > > > > > > +#define efi_rt_call(func, ...) efi_system_table()- > > > > > > > > >runtime->func(__VA_ARGS__) > > > > > > > > +#define efi_is_native() (true) > > > > > > > > + > > > > > > > > +#define efi_table_attr(inst, attr) (inst->attr) > > > > > > > > + > > > > > > > > +#define efi_call_proto(inst, func, ...) inst->func(inst, > > > > > > > > ##__VA_ARGS__) > > > > > > > > + > > > > > > > > +#define > > > > > > > > alloc_screen_info(x...) (&screen_info) > > > > > > > > +extern char stext_offset[]; > > > > > > > > + > > > > > > > > +static inline void free_screen_info(struct screen_info > > > > > > > > *si) > > > > > > > > +{ > > > > > > > > +} > > > > > > > > + > > > > > > > > +#endif /* _ASM_EFI_H */ > > > > > > > > diff --git a/drivers/firmware/efi/Kconfig > > > > > > > > b/drivers/firmware/efi/Kconfig > > > > > > > > index 2a2b2b96a1dc..fcdc789d3f87 100644 > > > > > > > > --- a/drivers/firmware/efi/Kconfig > > > > > > > > +++ b/drivers/firmware/efi/Kconfig > > > > > > > > @@ -111,7 +111,7 @@ config EFI_GENERIC_STUB > > > > > > > > > > > > > > > > config EFI_ARMSTUB_DTB_LOADER > > > > > > > > bool "Enable the DTB loader" > > > > > > > > - depends on EFI_GENERIC_STUB > > > > > > > > + depends on EFI_GENERIC_STUB && !RISCV > > > > > > > > default y > > > > > > > > help > > > > > > > > Select this config option to add support for > > > > > > > > the dtb= command > > > > > > > > diff --git a/drivers/firmware/efi/libstub/Makefile > > > > > > > > b/drivers/firmware/efi/libstub/Makefile > > > > > > > > index 2b4e09bf987c..7d46b70b51f2 100644 > > > > > > > > --- a/drivers/firmware/efi/libstub/Makefile > > > > > > > > +++ b/drivers/firmware/efi/libstub/Makefile > > > > > > > > @@ -22,6 +22,8 @@ cflags- > > > > > > > > $(CONFIG_ARM64) := $(subst > > > > > > > > $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > > > > cflags-$(CONFIG_ARM) := $(subst > > > > > > > > $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > > > > -fno-builtin -fpic \ > > > > > > > > $(call cc-option,-mno- > > > > > > > > single-pic-base) > > > > > > > > +cflags-$(CONFIG_RISCV) := $(subst > > > > > > > > $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > > > > + -fpic > > > > > > > > > > > > > > > > cflags-$(CONFIG_EFI_GENERIC_STUB) += > > > > > > > > -I$(srctree)/scripts/dtc/libfdt > > > > > > > > > > > > > > > > @@ -57,6 +59,7 @@ lib- > > > > > > > > $(CONFIG_EFI_GENERIC_STUB) += efi-stub.o > > > > > > > > fdt.o string.o \ > > > > > > > > lib-$(CONFIG_ARM) += arm32-stub.o > > > > > > > > lib-$(CONFIG_ARM64) += arm64-stub.o > > > > > > > > lib-$(CONFIG_X86) += x86-stub.o > > > > > > > > +lib-$(CONFIG_RISCV) += riscv-stub.o > > > > > > > > CFLAGS_arm32-stub.o := > > > > > > > > -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > > > > > CFLAGS_arm64-stub.o := > > > > > > > > -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > > > > > > > > > > > > > @@ -81,6 +84,10 @@ STUBCOPY_FLAGS- > > > > > > > > $(CONFIG_ARM64) += --prefix-alloc-sections=.init \ > > > > > > > > --prefix- > > > > > > > > symbols=__efistub_ > > > > > > > > STUBCOPY_RELOC-$(CONFIG_ARM64) := R_AARCH64_ABS > > > > > > > > > > > > > > > > +STUBCOPY_FLAGS-$(CONFIG_RISCV) += --prefix-alloc- > > > > > > > > sections=.init \ > > > > > > > > + --prefix- > > > > > > > > symbols=__efistub_ > > > > > > > > +STUBCOPY_RELOC-$(CONFIG_RISCV) := R_RISCV_HI20 > > > > > > > > + > > > > > > > > $(obj)/%.stub.o: $(obj)/%.o FORCE > > > > > > > > $(call if_changed,stubcopy) > > > > > > > > > > > > > > > > diff --git a/drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > > b/drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > > new file mode 100644 > > > > > > > > index 000000000000..69d13e0ebaea > > > > > > > > --- /dev/null > > > > > > > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > > @@ -0,0 +1,111 @@ > > > > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > > > > +/* > > > > > > > > + * Copyright (C) 2013, 2014 Linaro Ltd; < > > > > > > > > roy.franz@linaro.org> > > > > > > > > + * Copyright (C) 2020 Western Digital Corporation or its > > > > > > > > affiliates. > > > > > > > > + * > > > > > > > > + * This file implements the EFI boot stub for the RISC-V > > > > > > > > kernel. > > > > > > > > + * Adapted from ARM64 version at > > > > > > > > drivers/firmware/efi/libstub/arm64-stub.c. > > > > > > > > + */ > > > > > > > > + > > > > > > > > +#include <linux/efi.h> > > > > > > > > +#include <linux/libfdt.h> > > > > > > > > +#include <linux/libfdt_env.h> > > > > > > > > +#include <asm/efi.h> > > > > > > > > +#include <asm/sections.h> > > > > > > > > + > > > > > > > > +#include "efistub.h" > > > > > > > > +/* > > > > > > > > + * RISCV requires the kernel image to placed TEXT_OFFSET > > > > > > > > bytes beyond a 2 MB > > > > > > > > + * aligned base for 64 bit and 4MB for 32 bit. > > > > > > > > + */ > > > > > > > > > > > > Fixed the comment. > > > > > > > > > > > > > > +#ifdef CONFIG_64BIT > > > > > > > > +#define MIN_KIMG_ALIGN SZ_2M > > > > > > > > +#else > > > > > > > > +#define MIN_KIMG_ALIGN SZ_4M > > > > > > > > +#endif > > > > > > > > + > > > > > > > > +typedef __attribute__((noreturn)) void > > > > > > > > (*jump_kernel_func)(unsigned int, > > > > > > > > + > > > > > > > > unsigned long); > > > > > > > > +efi_status_t check_platform_features(void) > > > > > > > > +{ > > > > > > > > + return EFI_SUCCESS; > > > > > > > > +} > > > > > > > > + > > > > > > > > +static u32 get_boot_hartid_from_fdt(unsigned long fdt) > > > > > > > > +{ > > > > > > > > + int chosen_node, len; > > > > > > > > + const fdt32_t *prop; > > > > > > > > + > > > > > > > > + chosen_node = fdt_path_offset((void *)fdt, > > > > > > > > "/chosen"); > > > > > > > > + if (chosen_node < 0) > > > > > > > > + return U32_MAX; > > > > > > > > + prop = fdt_getprop((void *)fdt, chosen_node, > > > > > > > > "boot-hartid", &len); > > > > > > > > + if (!prop || len != sizeof(u32)) > > > > > > > > + return U32_MAX; > > > > > > > > + > > > > > > > > + return fdt32_to_cpu(*prop); > > > > > > > > +} > > > > > > > > + > > > > > > > > +/* > > > > > > > > + * Jump to real kernel here with following constraints. > > > > > > > > + * 1. MMU should be disabled. > > > > > > > > + * 2. a0 should contain hartid > > > > > > > > + * 3. a1 should DT address > > > > > > > > + */ > > > > > > > > +void __noreturn efi_enter_kernel(unsigned long > > > > > > > > entrypoint, unsigned long fdt, > > > > > > > > + unsigned long fdt_size) > > > > > > > > +{ > > > > > > > > + unsigned long kernel_entry = entrypoint + > > > > > > > > (unsigned long)stext_offset; > > > > > > > > + jump_kernel_func jump_kernel = (jump_kernel_func) > > > > > > > > kernel_entry; > > > > > > > > + u32 hartid = get_boot_hartid_from_fdt(fdt); > > > > > > > > + > > > > > > > > + if (hartid == U32_MAX) > > > > > > > > + /* We can not use panic or BUG at this > > > > > > > > point */ > > > > > > > > + __asm__ __volatile__ ("ebreak"); > > > > > > > > + /* Disable MMU */ > > > > > > > > + csr_write(CSR_SATP, 0); > > > > > > > > + jump_kernel(hartid, fdt); > > > > > > > > +} > > > > > > > > + > > > > > > > > +efi_status_t handle_kernel_image(unsigned long > > > > > > > > *image_addr, > > > > > > > > + unsigned long > > > > > > > > *image_size, > > > > > > > > + unsigned long > > > > > > > > *reserve_addr, > > > > > > > > + unsigned long > > > > > > > > *reserve_size, > > > > > > > > + unsigned long dram_base, > > > > > > > > + efi_loaded_image_t > > > > > > > > *image) > > > > > > > > +{ > > > > > > > > + efi_status_t status; > > > > > > > > + unsigned long kernel_size, kernel_memsize = 0; > > > > > > > > + unsigned long max_alloc_address; > > > > > > > > + > > > > > > > > + if (image->image_base != _start) > > > > > > > > + pr_efi_err("FIRMWARE BUG: > > > > > > > > efi_loaded_image_t::image_base has bogus value\n"); > > > > > > > > + > > > > > > > > > > > > > > I don't think you need this. > > > > > > > > > > > > > > > > > > > Sure. I will remove it. I guess ARM64 code has the error > > > > > > print for > > > > > > legacy loader code ? > > > > > > > > > > > > > > > > No, for broken distro versions of GRUB > > > > > > > > > > > > > + kernel_size = _edata - _start; > > > > > > > > + kernel_memsize = kernel_size + (_end - _edata); > > > > > > > > + max_alloc_address = round_up(dram_base, > > > > > > > > MIN_KIMG_ALIGN) + > > > > > > > > + kernel_memsize; > > > > > > > > + > > > > > > > > > > > > > > You said the kernel could be anywhere in memory, as long as > > > > > > > it is > > > > > > > aligned correctly, right? > > > > > > > > > > > > Sorry I was wrong about this. RISC-V kernel maps PAGE_OFFSET > > > > > > virtual > > > > > > address to the > > > > > > physical address <xyz> where it is booted. That means memory > > > > > > between > > > > > > dram start and and <xyz> address > > > > > > will be unusable. > > > > > > > > > > > > > > > > OK > > > > > > > > > > > I also realized that the above computing max_address as above > > > > > > also > > > > > > won't work for the following reason. > > > > > > efi_allocate_pages_aligned actually ALIGN_DOWN the > > > > > > max_address. Thus, > > > > > > efi won't find enough > > > > > > free memory in this case if the max_address is computed from > > > > > > the dram_base. > > > > > > > > > > > > Is there an implicit requirement for > > > > > > efi_allocate_pages_aligned or > > > > > > efi_low_alloc_above should be tried in case of failure? > > > > > > > > > > > > > > > > No not really. What ever works for your particular use case is > > > > > acceptable to me. > > > > > > > > > > > > In that case, you don't need this, you can simply pass > > > > > > > ULONG_MAX as > > > > > > > the max address. > > > > > > > > > > > > > As RISC-V should allocate memory as low as possible to avoid > > > > > > memory > > > > > > wastage, I think the following should work. > > > > > > > > > > > > efi_low_alloc_above(*reserve_size, MIN_KIMG_ALIGN, > > > > > > reserve_addr, dram_base); > > > > > > > > > > > > If this looks okay to you, efi_low_alloc_above should be > > > > > > moved back to > > > > > > mem.c from relocate.c. > > > > > > Should I do it in a separate patch or the original patch > > > > > > should be > > > > > > modified so that efi_low_alloc_above was never moved to > > > > > > relocate.c > > > > > > > > > > > > > > > > No, please keep efi_low_alloc_above() where it is, but drop the > > > > > static, and put back the declaration in efistub.h > > > > > > > > > > > > > Alternatively, can you check whether efi_relocate_kernel() > > > > already > > > > does what you need? > > > > > > Yeah. efi_relocate_kernel works too. It's just that > > > efi_relocate_kernel expects a preferred address > > > which RISC-V doesn't care about. > > > > There is a preferred address, no? The base of DRAM? > > > > I just realized that my reply from my gmail never landed in any of the > mailing lists. If you have already received it, sorry for the spam. > > "If the preferred address is set as the base of DRAM, efi_bs_call is > bound to fail as well because the base of DRAM is reserved by the > firmware. So the efi memory allocator can't allocate at that address. > Technically, it will work but it is no different than passing > ULONG_MAX. So I thought ULONG_MAX will avoid the confusion. > > We try to allocate as low as possible so I am passing dram_base as the > minimum address anyways. As the firmware reserved the first few KBs, > OK, so the preferred address *is* the base of DRAM (assuming it is 2 MB aligned). However, in the general case, you keep some firmware state there (couple of KB) and so you typically end up at DRAM base plus 2 MB? So first question: why does the firmware put this stuff at the base of DRAM in the first place? Does it *have* to live there? Then, if the base of DRAM is guaranteed to be occupied, why not make the preferred address base of DRAM + 2 MB ? (or 4 MB for the 32-bit case)
On Mon, Apr 20, 2020 at 12:04 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Mon, 20 Apr 2020 at 06:20, Atish Patra <Atish.Patra@wdc.com> wrote: > > > > On Sat, 2020-04-18 at 21:24 +0200, Ard Biesheuvel wrote: > > > On Sat, 18 Apr 2020 at 21:19, Atish Patra <atishp@atishpatra.org> > > > wrote: > > > > On Sat, Apr 18, 2020 at 5:39 AM Ard Biesheuvel <ardb@kernel.org> > > > > wrote: > > > > > On Sat, 18 Apr 2020 at 12:51, Ard Biesheuvel <ardb@kernel.org> > > > > > wrote: > > > > > > On Sat, 18 Apr 2020 at 05:03, Atish Patra < > > > > > > atishp@atishpatra.org> wrote: > > > > > > > On Thu, Apr 16, 2020 at 12:41 AM Ard Biesheuvel < > > > > > > > ardb@kernel.org> wrote: > > > > > > > > On Wed, 15 Apr 2020 at 21:54, Atish Patra < > > > > > > > > atish.patra@wdc.com> wrote: > > > > > > > > > Add a RISC-V architecture specific stub code that > > > > > > > > > actually copies the > > > > > > > > > actual kernel image to a valid address and jump to it > > > > > > > > > after boot services > > > > > > > > > are terminated. Enable UEFI related kernel configs as > > > > > > > > > well for RISC-V. > > > > > > > > > > > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > > > > > > > > > --- > > > > > > > > > arch/riscv/Kconfig | 20 ++++ > > > > > > > > > arch/riscv/Makefile | 1 + > > > > > > > > > arch/riscv/configs/defconfig | 1 + > > > > > > > > > arch/riscv/include/asm/efi.h | 44 > > > > > > > > > +++++++++ > > > > > > > > > drivers/firmware/efi/Kconfig | 2 +- > > > > > > > > > drivers/firmware/efi/libstub/Makefile | 7 ++ > > > > > > > > > drivers/firmware/efi/libstub/riscv-stub.c | 111 > > > > > > > > > ++++++++++++++++++++++ > > > > > > > > > 7 files changed, 185 insertions(+), 1 deletion(-) > > > > > > > > > create mode 100644 arch/riscv/include/asm/efi.h > > > > > > > > > create mode 100644 drivers/firmware/efi/libstub/riscv- > > > > > > > > > stub.c > > > > > > > > > > > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > > > > index f39e326a7a42..eb4f41c8f3ce 100644 > > > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > > > @@ -379,10 +379,30 @@ config CMDLINE_FORCE > > > > > > > > > > > > > > > > > > endchoice > > > > > > > > > > > > > > > > > > +config EFI_STUB > > > > > > > > > + bool > > > > > > > > > + > > > > > > > > > +config EFI > > > > > > > > > + bool "UEFI runtime support" > > > > > > > > > + depends on OF > > > > > > > > > + select LIBFDT > > > > > > > > > + select UCS2_STRING > > > > > > > > > + select EFI_PARAMS_FROM_FDT > > > > > > > > > + select EFI_STUB > > > > > > > > > + select EFI_GENERIC_STUB > > > > > > > > > + default y > > > > > > > > > + help > > > > > > > > > + This option provides support for runtime > > > > > > > > > services provided > > > > > > > > > + by UEFI firmware (such as non-volatile > > > > > > > > > variables, realtime > > > > > > > > > + clock, and platform reset). A UEFI stub is > > > > > > > > > also provided to > > > > > > > > > + allow the kernel to be booted as an EFI > > > > > > > > > application. This > > > > > > > > > + is only useful on systems that have UEFI > > > > > > > > > firmware. > > > > > > > > > + > > > > > > > > > endmenu > > > > > > > > > > > > > > > > > > menu "Power management options" > > > > > > > > > > > > > > > > > > source "kernel/power/Kconfig" > > > > > > > > > +source "drivers/firmware/Kconfig" > > > > > > > > > > > > > > > > > > endmenu > > > > > > > > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > > > > > > > > > index fb6e37db836d..079435804d6d 100644 > > > > > > > > > --- a/arch/riscv/Makefile > > > > > > > > > +++ b/arch/riscv/Makefile > > > > > > > > > @@ -80,6 +80,7 @@ head-y := arch/riscv/kernel/head.o > > > > > > > > > core-y += arch/riscv/ > > > > > > > > > > > > > > > > > > libs-y += arch/riscv/lib/ > > > > > > > > > +core-$(CONFIG_EFI_STUB) += > > > > > > > > > $(objtree)/drivers/firmware/efi/libstub/lib.a > > > > > > > > > > > > > > > > > > PHONY += vdso_install > > > > > > > > > vdso_install: > > > > > > > > > diff --git a/arch/riscv/configs/defconfig > > > > > > > > > b/arch/riscv/configs/defconfig > > > > > > > > > index 4da4886246a4..ae69e12d306a 100644 > > > > > > > > > --- a/arch/riscv/configs/defconfig > > > > > > > > > +++ b/arch/riscv/configs/defconfig > > > > > > > > > @@ -129,3 +129,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > > > > > > > > > # CONFIG_RUNTIME_TESTING_MENU is not set > > > > > > > > > CONFIG_MEMTEST=y > > > > > > > > > # CONFIG_SYSFS_SYSCALL is not set > > > > > > > > > +CONFIG_EFI=y > > > > > > > > > diff --git a/arch/riscv/include/asm/efi.h > > > > > > > > > b/arch/riscv/include/asm/efi.h > > > > > > > > > new file mode 100644 > > > > > > > > > index 000000000000..62d7d5eafed8 > > > > > > > > > --- /dev/null > > > > > > > > > +++ b/arch/riscv/include/asm/efi.h > > > > > > > > > @@ -0,0 +1,44 @@ > > > > > > > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > > > > > > > +/* > > > > > > > > > + * Copyright (C) 2020 Western Digital Corporation or its > > > > > > > > > affiliates. > > > > > > > > > + * Based on arch/arm64/include/asm/efi.h > > > > > > > > > + */ > > > > > > > > > +#ifndef _ASM_EFI_H > > > > > > > > > +#define _ASM_EFI_H > > > > > > > > > + > > > > > > > > > +#include <asm/io.h> > > > > > > > > > +#include <asm/mmu_context.h> > > > > > > > > > +#include <asm/ptrace.h> > > > > > > > > > +#include <asm/tlbflush.h> > > > > > > > > > + > > > > > > > > > +#define VA_BITS_MIN 39 > > > > > > > > > + > > > > > > > > > +/* on RISC-V, the FDT may be located anywhere in system > > > > > > > > > RAM */ > > > > > > > > > +static inline unsigned long > > > > > > > > > efi_get_max_fdt_addr(unsigned long dram_base) > > > > > > > > > +{ > > > > > > > > > + return ULONG_MAX; > > > > > > > > > +} > > > > > > > > > + > > > > > > > > > +/* Load initrd at enough distance from DRAM start */ > > > > > > > > > +static inline unsigned long > > > > > > > > > efi_get_max_initrd_addr(unsigned long dram_base, > > > > > > > > > + unsig > > > > > > > > > ned long image_addr) > > > > > > > > > +{ > > > > > > > > > + return dram_base + SZ_256M; > > > > > > > > > +} > > > > > > > > > + > > > > > > > > > +#define efi_bs_call(func, ...) efi_system_table()- > > > > > > > > > >boottime->func(__VA_ARGS__) > > > > > > > > > +#define efi_rt_call(func, ...) efi_system_table()- > > > > > > > > > >runtime->func(__VA_ARGS__) > > > > > > > > > +#define efi_is_native() (true) > > > > > > > > > + > > > > > > > > > +#define efi_table_attr(inst, attr) (inst->attr) > > > > > > > > > + > > > > > > > > > +#define efi_call_proto(inst, func, ...) inst->func(inst, > > > > > > > > > ##__VA_ARGS__) > > > > > > > > > + > > > > > > > > > +#define > > > > > > > > > alloc_screen_info(x...) (&screen_info) > > > > > > > > > +extern char stext_offset[]; > > > > > > > > > + > > > > > > > > > +static inline void free_screen_info(struct screen_info > > > > > > > > > *si) > > > > > > > > > +{ > > > > > > > > > +} > > > > > > > > > + > > > > > > > > > +#endif /* _ASM_EFI_H */ > > > > > > > > > diff --git a/drivers/firmware/efi/Kconfig > > > > > > > > > b/drivers/firmware/efi/Kconfig > > > > > > > > > index 2a2b2b96a1dc..fcdc789d3f87 100644 > > > > > > > > > --- a/drivers/firmware/efi/Kconfig > > > > > > > > > +++ b/drivers/firmware/efi/Kconfig > > > > > > > > > @@ -111,7 +111,7 @@ config EFI_GENERIC_STUB > > > > > > > > > > > > > > > > > > config EFI_ARMSTUB_DTB_LOADER > > > > > > > > > bool "Enable the DTB loader" > > > > > > > > > - depends on EFI_GENERIC_STUB > > > > > > > > > + depends on EFI_GENERIC_STUB && !RISCV > > > > > > > > > default y > > > > > > > > > help > > > > > > > > > Select this config option to add support for > > > > > > > > > the dtb= command > > > > > > > > > diff --git a/drivers/firmware/efi/libstub/Makefile > > > > > > > > > b/drivers/firmware/efi/libstub/Makefile > > > > > > > > > index 2b4e09bf987c..7d46b70b51f2 100644 > > > > > > > > > --- a/drivers/firmware/efi/libstub/Makefile > > > > > > > > > +++ b/drivers/firmware/efi/libstub/Makefile > > > > > > > > > @@ -22,6 +22,8 @@ cflags- > > > > > > > > > $(CONFIG_ARM64) := $(subst > > > > > > > > > $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > > > > > cflags-$(CONFIG_ARM) := $(subst > > > > > > > > > $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > > > > > -fno-builtin -fpic \ > > > > > > > > > $(call cc-option,-mno- > > > > > > > > > single-pic-base) > > > > > > > > > +cflags-$(CONFIG_RISCV) := $(subst > > > > > > > > > $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ > > > > > > > > > + -fpic > > > > > > > > > > > > > > > > > > cflags-$(CONFIG_EFI_GENERIC_STUB) += > > > > > > > > > -I$(srctree)/scripts/dtc/libfdt > > > > > > > > > > > > > > > > > > @@ -57,6 +59,7 @@ lib- > > > > > > > > > $(CONFIG_EFI_GENERIC_STUB) += efi-stub.o > > > > > > > > > fdt.o string.o \ > > > > > > > > > lib-$(CONFIG_ARM) += arm32-stub.o > > > > > > > > > lib-$(CONFIG_ARM64) += arm64-stub.o > > > > > > > > > lib-$(CONFIG_X86) += x86-stub.o > > > > > > > > > +lib-$(CONFIG_RISCV) += riscv-stub.o > > > > > > > > > CFLAGS_arm32-stub.o := > > > > > > > > > -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > > > > > > CFLAGS_arm64-stub.o := > > > > > > > > > -DTEXT_OFFSET=$(TEXT_OFFSET) > > > > > > > > > > > > > > > > > > @@ -81,6 +84,10 @@ STUBCOPY_FLAGS- > > > > > > > > > $(CONFIG_ARM64) += --prefix-alloc-sections=.init \ > > > > > > > > > --prefix- > > > > > > > > > symbols=__efistub_ > > > > > > > > > STUBCOPY_RELOC-$(CONFIG_ARM64) := R_AARCH64_ABS > > > > > > > > > > > > > > > > > > +STUBCOPY_FLAGS-$(CONFIG_RISCV) += --prefix-alloc- > > > > > > > > > sections=.init \ > > > > > > > > > + --prefix- > > > > > > > > > symbols=__efistub_ > > > > > > > > > +STUBCOPY_RELOC-$(CONFIG_RISCV) := R_RISCV_HI20 > > > > > > > > > + > > > > > > > > > $(obj)/%.stub.o: $(obj)/%.o FORCE > > > > > > > > > $(call if_changed,stubcopy) > > > > > > > > > > > > > > > > > > diff --git a/drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > > > b/drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > > > new file mode 100644 > > > > > > > > > index 000000000000..69d13e0ebaea > > > > > > > > > --- /dev/null > > > > > > > > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > > > @@ -0,0 +1,111 @@ > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0 > > > > > > > > > +/* > > > > > > > > > + * Copyright (C) 2013, 2014 Linaro Ltd; < > > > > > > > > > roy.franz@linaro.org> > > > > > > > > > + * Copyright (C) 2020 Western Digital Corporation or its > > > > > > > > > affiliates. > > > > > > > > > + * > > > > > > > > > + * This file implements the EFI boot stub for the RISC-V > > > > > > > > > kernel. > > > > > > > > > + * Adapted from ARM64 version at > > > > > > > > > drivers/firmware/efi/libstub/arm64-stub.c. > > > > > > > > > + */ > > > > > > > > > + > > > > > > > > > +#include <linux/efi.h> > > > > > > > > > +#include <linux/libfdt.h> > > > > > > > > > +#include <linux/libfdt_env.h> > > > > > > > > > +#include <asm/efi.h> > > > > > > > > > +#include <asm/sections.h> > > > > > > > > > + > > > > > > > > > +#include "efistub.h" > > > > > > > > > +/* > > > > > > > > > + * RISCV requires the kernel image to placed TEXT_OFFSET > > > > > > > > > bytes beyond a 2 MB > > > > > > > > > + * aligned base for 64 bit and 4MB for 32 bit. > > > > > > > > > + */ > > > > > > > > > > > > > > Fixed the comment. > > > > > > > > > > > > > > > > +#ifdef CONFIG_64BIT > > > > > > > > > +#define MIN_KIMG_ALIGN SZ_2M > > > > > > > > > +#else > > > > > > > > > +#define MIN_KIMG_ALIGN SZ_4M > > > > > > > > > +#endif > > > > > > > > > + > > > > > > > > > +typedef __attribute__((noreturn)) void > > > > > > > > > (*jump_kernel_func)(unsigned int, > > > > > > > > > + > > > > > > > > > unsigned long); > > > > > > > > > +efi_status_t check_platform_features(void) > > > > > > > > > +{ > > > > > > > > > + return EFI_SUCCESS; > > > > > > > > > +} > > > > > > > > > + > > > > > > > > > +static u32 get_boot_hartid_from_fdt(unsigned long fdt) > > > > > > > > > +{ > > > > > > > > > + int chosen_node, len; > > > > > > > > > + const fdt32_t *prop; > > > > > > > > > + > > > > > > > > > + chosen_node = fdt_path_offset((void *)fdt, > > > > > > > > > "/chosen"); > > > > > > > > > + if (chosen_node < 0) > > > > > > > > > + return U32_MAX; > > > > > > > > > + prop = fdt_getprop((void *)fdt, chosen_node, > > > > > > > > > "boot-hartid", &len); > > > > > > > > > + if (!prop || len != sizeof(u32)) > > > > > > > > > + return U32_MAX; > > > > > > > > > + > > > > > > > > > + return fdt32_to_cpu(*prop); > > > > > > > > > +} > > > > > > > > > + > > > > > > > > > +/* > > > > > > > > > + * Jump to real kernel here with following constraints. > > > > > > > > > + * 1. MMU should be disabled. > > > > > > > > > + * 2. a0 should contain hartid > > > > > > > > > + * 3. a1 should DT address > > > > > > > > > + */ > > > > > > > > > +void __noreturn efi_enter_kernel(unsigned long > > > > > > > > > entrypoint, unsigned long fdt, > > > > > > > > > + unsigned long fdt_size) > > > > > > > > > +{ > > > > > > > > > + unsigned long kernel_entry = entrypoint + > > > > > > > > > (unsigned long)stext_offset; > > > > > > > > > + jump_kernel_func jump_kernel = (jump_kernel_func) > > > > > > > > > kernel_entry; > > > > > > > > > + u32 hartid = get_boot_hartid_from_fdt(fdt); > > > > > > > > > + > > > > > > > > > + if (hartid == U32_MAX) > > > > > > > > > + /* We can not use panic or BUG at this > > > > > > > > > point */ > > > > > > > > > + __asm__ __volatile__ ("ebreak"); > > > > > > > > > + /* Disable MMU */ > > > > > > > > > + csr_write(CSR_SATP, 0); > > > > > > > > > + jump_kernel(hartid, fdt); > > > > > > > > > +} > > > > > > > > > + > > > > > > > > > +efi_status_t handle_kernel_image(unsigned long > > > > > > > > > *image_addr, > > > > > > > > > + unsigned long > > > > > > > > > *image_size, > > > > > > > > > + unsigned long > > > > > > > > > *reserve_addr, > > > > > > > > > + unsigned long > > > > > > > > > *reserve_size, > > > > > > > > > + unsigned long dram_base, > > > > > > > > > + efi_loaded_image_t > > > > > > > > > *image) > > > > > > > > > +{ > > > > > > > > > + efi_status_t status; > > > > > > > > > + unsigned long kernel_size, kernel_memsize = 0; > > > > > > > > > + unsigned long max_alloc_address; > > > > > > > > > + > > > > > > > > > + if (image->image_base != _start) > > > > > > > > > + pr_efi_err("FIRMWARE BUG: > > > > > > > > > efi_loaded_image_t::image_base has bogus value\n"); > > > > > > > > > + > > > > > > > > > > > > > > > > I don't think you need this. > > > > > > > > > > > > > > > > > > > > > > Sure. I will remove it. I guess ARM64 code has the error > > > > > > > print for > > > > > > > legacy loader code ? > > > > > > > > > > > > > > > > > > > No, for broken distro versions of GRUB > > > > > > > > > > > > > > > + kernel_size = _edata - _start; > > > > > > > > > + kernel_memsize = kernel_size + (_end - _edata); > > > > > > > > > + max_alloc_address = round_up(dram_base, > > > > > > > > > MIN_KIMG_ALIGN) + > > > > > > > > > + kernel_memsize; > > > > > > > > > + > > > > > > > > > > > > > > > > You said the kernel could be anywhere in memory, as long as > > > > > > > > it is > > > > > > > > aligned correctly, right? > > > > > > > > > > > > > > Sorry I was wrong about this. RISC-V kernel maps PAGE_OFFSET > > > > > > > virtual > > > > > > > address to the > > > > > > > physical address <xyz> where it is booted. That means memory > > > > > > > between > > > > > > > dram start and and <xyz> address > > > > > > > will be unusable. > > > > > > > > > > > > > > > > > > > OK > > > > > > > > > > > > > I also realized that the above computing max_address as above > > > > > > > also > > > > > > > won't work for the following reason. > > > > > > > efi_allocate_pages_aligned actually ALIGN_DOWN the > > > > > > > max_address. Thus, > > > > > > > efi won't find enough > > > > > > > free memory in this case if the max_address is computed from > > > > > > > the dram_base. > > > > > > > > > > > > > > Is there an implicit requirement for > > > > > > > efi_allocate_pages_aligned or > > > > > > > efi_low_alloc_above should be tried in case of failure? > > > > > > > > > > > > > > > > > > > No not really. What ever works for your particular use case is > > > > > > acceptable to me. > > > > > > > > > > > > > > In that case, you don't need this, you can simply pass > > > > > > > > ULONG_MAX as > > > > > > > > the max address. > > > > > > > > > > > > > > > As RISC-V should allocate memory as low as possible to avoid > > > > > > > memory > > > > > > > wastage, I think the following should work. > > > > > > > > > > > > > > efi_low_alloc_above(*reserve_size, MIN_KIMG_ALIGN, > > > > > > > reserve_addr, dram_base); > > > > > > > > > > > > > > If this looks okay to you, efi_low_alloc_above should be > > > > > > > moved back to > > > > > > > mem.c from relocate.c. > > > > > > > Should I do it in a separate patch or the original patch > > > > > > > should be > > > > > > > modified so that efi_low_alloc_above was never moved to > > > > > > > relocate.c > > > > > > > > > > > > > > > > > > > No, please keep efi_low_alloc_above() where it is, but drop the > > > > > > static, and put back the declaration in efistub.h > > > > > > > > > > > > > > > > Alternatively, can you check whether efi_relocate_kernel() > > > > > already > > > > > does what you need? > > > > > > > > Yeah. efi_relocate_kernel works too. It's just that > > > > efi_relocate_kernel expects a preferred address > > > > which RISC-V doesn't care about. > > > > > > There is a preferred address, no? The base of DRAM? > > > > > > > I just realized that my reply from my gmail never landed in any of the > > mailing lists. If you have already received it, sorry for the spam. > > > > "If the preferred address is set as the base of DRAM, efi_bs_call is > > bound to fail as well because the base of DRAM is reserved by the > > firmware. So the efi memory allocator can't allocate at that address. > > Technically, it will work but it is no different than passing > > ULONG_MAX. So I thought ULONG_MAX will avoid the confusion. > > > > We try to allocate as low as possible so I am passing dram_base as the > > minimum address anyways. As the firmware reserved the first few KBs, > > > > > OK, so the preferred address *is* the base of DRAM (assuming it is 2 > MB aligned). However, in the general case, you keep some firmware > state there (couple of KB) and so you typically end up at DRAM base > plus 2 MB? > Yes. > So first question: why does the firmware put this stuff at the base of > DRAM in the first place? Does it *have* to live there? > The firmware includes the RISC-V specific supervisor binary interface (SBI)[[1] implementation. As it is a RISC-V specific run time service provider, it has to be resident in memory. The arm equivalent of SBI would be PSCI. [1] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc > Then, if the base of DRAM is guaranteed to be occupied, why not make > the preferred address base of DRAM + 2 MB ? (or 4 MB for the 32-bit > case) I guess that will work too. I was inclined to use low_alloc_above so that we ensure that the kernel boots from the lowest possible address given the alignment restriction. If the alignment restrictions are relaxed, in future, we just have to change the macro. However, I think calling relocate_kernel with a preferred offset (dram_base + KIMG_ALIGN) is better because it will always fall back to low_alloc_above anyways. I will send the patch.
On Mon, 20 Apr 2020 at 09:59, Atish Patra <atishp@atishpatra.org> wrote: > > On Mon, Apr 20, 2020 at 12:04 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Mon, 20 Apr 2020 at 06:20, Atish Patra <Atish.Patra@wdc.com> wrote: > > > ... > > > > > > "If the preferred address is set as the base of DRAM, efi_bs_call is > > > bound to fail as well because the base of DRAM is reserved by the > > > firmware. So the efi memory allocator can't allocate at that address. > > > Technically, it will work but it is no different than passing > > > ULONG_MAX. So I thought ULONG_MAX will avoid the confusion. > > > > > > We try to allocate as low as possible so I am passing dram_base as the > > > minimum address anyways. As the firmware reserved the first few KBs, > > > > > > > > > OK, so the preferred address *is* the base of DRAM (assuming it is 2 > > MB aligned). However, in the general case, you keep some firmware > > state there (couple of KB) and so you typically end up at DRAM base > > plus 2 MB? > > > > Yes. > > > So first question: why does the firmware put this stuff at the base of > > DRAM in the first place? Does it *have* to live there? > > > > The firmware includes the RISC-V specific supervisor binary interface (SBI)[[1] > implementation. As it is a RISC-V specific run time service provider, > it has to be resident in memory. > The arm equivalent of SBI would be PSCI. > > [1] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc > I am not asking why it has to be resident in memory. I am asking why it has to be at the *base* of memory. > > Then, if the base of DRAM is guaranteed to be occupied, why not make > > the preferred address base of DRAM + 2 MB ? (or 4 MB for the 32-bit > > case) > > I guess that will work too. I was inclined to use low_alloc_above so > that we ensure that the kernel > boots from the lowest possible address given the alignment > restriction. If the alignment restrictions are relaxed, > in future, we just have to change the macro. > > However, I think calling relocate_kernel with a preferred offset > (dram_base + KIMG_ALIGN) is > better because it will always fall back to low_alloc_above anyways. I > will send the patch. Indeed. In the common case, it will just do the allocate_pages() directly, which is preferred. It will fall back to efi_low_alloc_aboce() [and do the memory map parsing and traversal etc] only if needed.
On Mon, Apr 20, 2020 at 1:02 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Mon, 20 Apr 2020 at 09:59, Atish Patra <atishp@atishpatra.org> wrote: > > > > On Mon, Apr 20, 2020 at 12:04 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > > > On Mon, 20 Apr 2020 at 06:20, Atish Patra <Atish.Patra@wdc.com> wrote: > > > > > ... > > > > > > > > "If the preferred address is set as the base of DRAM, efi_bs_call is > > > > bound to fail as well because the base of DRAM is reserved by the > > > > firmware. So the efi memory allocator can't allocate at that address. > > > > Technically, it will work but it is no different than passing > > > > ULONG_MAX. So I thought ULONG_MAX will avoid the confusion. > > > > > > > > We try to allocate as low as possible so I am passing dram_base as the > > > > minimum address anyways. As the firmware reserved the first few KBs, > > > > > > > > > > > > > OK, so the preferred address *is* the base of DRAM (assuming it is 2 > > > MB aligned). However, in the general case, you keep some firmware > > > state there (couple of KB) and so you typically end up at DRAM base > > > plus 2 MB? > > > > > > > Yes. > > > > > So first question: why does the firmware put this stuff at the base of > > > DRAM in the first place? Does it *have* to live there? > > > > > > > The firmware includes the RISC-V specific supervisor binary interface (SBI)[[1] > > implementation. As it is a RISC-V specific run time service provider, > > it has to be resident in memory. > > The arm equivalent of SBI would be PSCI. > > > > [1] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc > > > > I am not asking why it has to be resident in memory. I am asking why > it has to be at the *base* of memory. > Sorry. I misunderstood the question. I think it is kept at the start of dram to accommodate embedded systems with smaller memory. It was also easier to manage at the beginning of the memory because all other next stages in the boot process just ignores first few KBs of the memory. We also did not have a memory reservation scheme back then. Having said that, this parameter is configurable for each platform in OpenSBI. In future, this restriction can be relaxed if a platform vendor wants. IIRC, linux kernel can't use the memory below the kernel start address (where PAGE_OFFSET is mapped) because of generic mm code limitations. Added Anup (In case he wants to add something to this) > > > Then, if the base of DRAM is guaranteed to be occupied, why not make > > > the preferred address base of DRAM + 2 MB ? (or 4 MB for the 32-bit > > > case) > > > > I guess that will work too. I was inclined to use low_alloc_above so > > that we ensure that the kernel > > boots from the lowest possible address given the alignment > > restriction. If the alignment restrictions are relaxed, > > in future, we just have to change the macro. > > > > However, I think calling relocate_kernel with a preferred offset > > (dram_base + KIMG_ALIGN) is > > better because it will always fall back to low_alloc_above anyways. I > > will send the patch. > > Indeed. In the common case, it will just do the allocate_pages() > directly, which is preferred. It will fall back to > efi_low_alloc_aboce() [and do the memory map parsing and traversal > etc] only if needed.
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index f39e326a7a42..eb4f41c8f3ce 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -379,10 +379,30 @@ config CMDLINE_FORCE endchoice +config EFI_STUB + bool + +config EFI + bool "UEFI runtime support" + depends on OF + select LIBFDT + select UCS2_STRING + select EFI_PARAMS_FROM_FDT + select EFI_STUB + select EFI_GENERIC_STUB + default y + help + This option provides support for runtime services provided + by UEFI firmware (such as non-volatile variables, realtime + clock, and platform reset). A UEFI stub is also provided to + allow the kernel to be booted as an EFI application. This + is only useful on systems that have UEFI firmware. + endmenu menu "Power management options" source "kernel/power/Kconfig" +source "drivers/firmware/Kconfig" endmenu diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index fb6e37db836d..079435804d6d 100644 --- a/arch/riscv/Makefile +++ b/arch/riscv/Makefile @@ -80,6 +80,7 @@ head-y := arch/riscv/kernel/head.o core-y += arch/riscv/ libs-y += arch/riscv/lib/ +core-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a PHONY += vdso_install vdso_install: diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig index 4da4886246a4..ae69e12d306a 100644 --- a/arch/riscv/configs/defconfig +++ b/arch/riscv/configs/defconfig @@ -129,3 +129,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y # CONFIG_RUNTIME_TESTING_MENU is not set CONFIG_MEMTEST=y # CONFIG_SYSFS_SYSCALL is not set +CONFIG_EFI=y diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h new file mode 100644 index 000000000000..62d7d5eafed8 --- /dev/null +++ b/arch/riscv/include/asm/efi.h @@ -0,0 +1,44 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2020 Western Digital Corporation or its affiliates. + * Based on arch/arm64/include/asm/efi.h + */ +#ifndef _ASM_EFI_H +#define _ASM_EFI_H + +#include <asm/io.h> +#include <asm/mmu_context.h> +#include <asm/ptrace.h> +#include <asm/tlbflush.h> + +#define VA_BITS_MIN 39 + +/* on RISC-V, the FDT may be located anywhere in system RAM */ +static inline unsigned long efi_get_max_fdt_addr(unsigned long dram_base) +{ + return ULONG_MAX; +} + +/* Load initrd at enough distance from DRAM start */ +static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base, + unsigned long image_addr) +{ + return dram_base + SZ_256M; +} + +#define efi_bs_call(func, ...) efi_system_table()->boottime->func(__VA_ARGS__) +#define efi_rt_call(func, ...) efi_system_table()->runtime->func(__VA_ARGS__) +#define efi_is_native() (true) + +#define efi_table_attr(inst, attr) (inst->attr) + +#define efi_call_proto(inst, func, ...) inst->func(inst, ##__VA_ARGS__) + +#define alloc_screen_info(x...) (&screen_info) +extern char stext_offset[]; + +static inline void free_screen_info(struct screen_info *si) +{ +} + +#endif /* _ASM_EFI_H */ diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig index 2a2b2b96a1dc..fcdc789d3f87 100644 --- a/drivers/firmware/efi/Kconfig +++ b/drivers/firmware/efi/Kconfig @@ -111,7 +111,7 @@ config EFI_GENERIC_STUB config EFI_ARMSTUB_DTB_LOADER bool "Enable the DTB loader" - depends on EFI_GENERIC_STUB + depends on EFI_GENERIC_STUB && !RISCV default y help Select this config option to add support for the dtb= command diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index 2b4e09bf987c..7d46b70b51f2 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -22,6 +22,8 @@ cflags-$(CONFIG_ARM64) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ cflags-$(CONFIG_ARM) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ -fno-builtin -fpic \ $(call cc-option,-mno-single-pic-base) +cflags-$(CONFIG_RISCV) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \ + -fpic cflags-$(CONFIG_EFI_GENERIC_STUB) += -I$(srctree)/scripts/dtc/libfdt @@ -57,6 +59,7 @@ lib-$(CONFIG_EFI_GENERIC_STUB) += efi-stub.o fdt.o string.o \ lib-$(CONFIG_ARM) += arm32-stub.o lib-$(CONFIG_ARM64) += arm64-stub.o lib-$(CONFIG_X86) += x86-stub.o +lib-$(CONFIG_RISCV) += riscv-stub.o CFLAGS_arm32-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET) @@ -81,6 +84,10 @@ STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \ --prefix-symbols=__efistub_ STUBCOPY_RELOC-$(CONFIG_ARM64) := R_AARCH64_ABS +STUBCOPY_FLAGS-$(CONFIG_RISCV) += --prefix-alloc-sections=.init \ + --prefix-symbols=__efistub_ +STUBCOPY_RELOC-$(CONFIG_RISCV) := R_RISCV_HI20 + $(obj)/%.stub.o: $(obj)/%.o FORCE $(call if_changed,stubcopy) diff --git a/drivers/firmware/efi/libstub/riscv-stub.c b/drivers/firmware/efi/libstub/riscv-stub.c new file mode 100644 index 000000000000..69d13e0ebaea --- /dev/null +++ b/drivers/firmware/efi/libstub/riscv-stub.c @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2013, 2014 Linaro Ltd; <roy.franz@linaro.org> + * Copyright (C) 2020 Western Digital Corporation or its affiliates. + * + * This file implements the EFI boot stub for the RISC-V kernel. + * Adapted from ARM64 version at drivers/firmware/efi/libstub/arm64-stub.c. + */ + +#include <linux/efi.h> +#include <linux/libfdt.h> +#include <linux/libfdt_env.h> +#include <asm/efi.h> +#include <asm/sections.h> + +#include "efistub.h" +/* + * RISCV requires the kernel image to placed TEXT_OFFSET bytes beyond a 2 MB + * aligned base for 64 bit and 4MB for 32 bit. + */ +#ifdef CONFIG_64BIT +#define MIN_KIMG_ALIGN SZ_2M +#else +#define MIN_KIMG_ALIGN SZ_4M +#endif + +typedef __attribute__((noreturn)) void (*jump_kernel_func)(unsigned int, + unsigned long); +efi_status_t check_platform_features(void) +{ + return EFI_SUCCESS; +} + +static u32 get_boot_hartid_from_fdt(unsigned long fdt) +{ + int chosen_node, len; + const fdt32_t *prop; + + chosen_node = fdt_path_offset((void *)fdt, "/chosen"); + if (chosen_node < 0) + return U32_MAX; + prop = fdt_getprop((void *)fdt, chosen_node, "boot-hartid", &len); + if (!prop || len != sizeof(u32)) + return U32_MAX; + + return fdt32_to_cpu(*prop); +} + +/* + * Jump to real kernel here with following constraints. + * 1. MMU should be disabled. + * 2. a0 should contain hartid + * 3. a1 should DT address + */ +void __noreturn efi_enter_kernel(unsigned long entrypoint, unsigned long fdt, + unsigned long fdt_size) +{ + unsigned long kernel_entry = entrypoint + (unsigned long)stext_offset; + jump_kernel_func jump_kernel = (jump_kernel_func) kernel_entry; + u32 hartid = get_boot_hartid_from_fdt(fdt); + + if (hartid == U32_MAX) + /* We can not use panic or BUG at this point */ + __asm__ __volatile__ ("ebreak"); + /* Disable MMU */ + csr_write(CSR_SATP, 0); + jump_kernel(hartid, fdt); +} + +efi_status_t handle_kernel_image(unsigned long *image_addr, + unsigned long *image_size, + unsigned long *reserve_addr, + unsigned long *reserve_size, + unsigned long dram_base, + efi_loaded_image_t *image) +{ + efi_status_t status; + unsigned long kernel_size, kernel_memsize = 0; + unsigned long max_alloc_address; + + if (image->image_base != _start) + pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus value\n"); + + kernel_size = _edata - _start; + kernel_memsize = kernel_size + (_end - _edata); + max_alloc_address = round_up(dram_base, MIN_KIMG_ALIGN) + + kernel_memsize; + + if (IS_ALIGNED((u64)_start, MIN_KIMG_ALIGN)) { + /* + * Just execute from wherever we were loaded by the + * UEFI PE/COFF loader if the alignment is suitable. + */ + *image_addr = (u64)_start; + *reserve_size = 0; + return EFI_SUCCESS; + } + status = efi_allocate_pages_aligned(*reserve_size, reserve_addr, + max_alloc_address, MIN_KIMG_ALIGN); + + if (status != EFI_SUCCESS) { + pr_efi_err("Failed to relocate kernel\n"); + *reserve_size = 0; + return status; + } + *image_addr = *reserve_addr; + + memcpy((void *)*image_addr, _start, kernel_size); + + return EFI_SUCCESS; +}
Add a RISC-V architecture specific stub code that actually copies the actual kernel image to a valid address and jump to it after boot services are terminated. Enable UEFI related kernel configs as well for RISC-V. Signed-off-by: Atish Patra <atish.patra@wdc.com> --- arch/riscv/Kconfig | 20 ++++ arch/riscv/Makefile | 1 + arch/riscv/configs/defconfig | 1 + arch/riscv/include/asm/efi.h | 44 +++++++++ drivers/firmware/efi/Kconfig | 2 +- drivers/firmware/efi/libstub/Makefile | 7 ++ drivers/firmware/efi/libstub/riscv-stub.c | 111 ++++++++++++++++++++++ 7 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 arch/riscv/include/asm/efi.h create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c