| Message ID | 20200226011037.7179-6-atish.patra@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add UEFI support for RISC-V | expand |
On Wed, 26 Feb 2020 at 02:10, 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 + > drivers/firmware/efi/libstub/Makefile | 8 ++ > drivers/firmware/efi/libstub/riscv-stub.c | 135 ++++++++++++++++++++++ > 5 files changed, 165 insertions(+) > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 42c122170cfd..68b1d565e51d 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -372,10 +372,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_ARCH_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 b9009a2fbaf5..0afaa89ba9ad 100644 > --- a/arch/riscv/Makefile > +++ b/arch/riscv/Makefile > @@ -78,6 +78,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 e2ff95cb3390..0a5d3578f51e 100644 > --- a/arch/riscv/configs/defconfig > +++ b/arch/riscv/configs/defconfig > @@ -125,3 +125,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > # CONFIG_FTRACE is not set > # CONFIG_RUNTIME_TESTING_MENU is not set > CONFIG_MEMTEST=y > +CONFIG_EFI=y > diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile > index 2c5b76787126..38facb61745b 100644 > --- a/drivers/firmware/efi/libstub/Makefile > +++ b/drivers/firmware/efi/libstub/Makefile > @@ -21,6 +21,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_ARCH_STUB) += -I$(srctree)/scripts/dtc/libfdt > > @@ -55,6 +57,7 @@ lib-$(CONFIG_EFI_GENERIC_ARCH_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) > > @@ -79,6 +82,11 @@ 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..3935b29ea93a > --- /dev/null > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > @@ -0,0 +1,135 @@ > +// 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. > + */ > +#if IS_ENABLED(CONFIG_64BIT) You can use #ifdef here > +#define MIN_KIMG_ALIGN SZ_2M > +#else > +#define MIN_KIMG_ALIGN SZ_4M > +#endif > +/* > + * TEXT_OFFSET ensures that we don't overwrite the firmware that probably sits > + * at the beginning of the DRAM. > + */ Ugh. Really? On an EFI system, that memory should be reserved in some way, we shouldn't be able to stomp on it like that. > +#define TEXT_OFFSET MIN_KIMG_ALIGN > + > +typedef __attribute__((noreturn)) void (*jump_kernel_func)(unsigned int, > + unsigned long); > + > +efi_status_t check_platform_features(void) > +{ > + return EFI_SUCCESS; > +} > + > +u64 get_boot_hartid_from_fdt(unsigned long fdt) static > +{ > + int chosen_node, len; > + const fdt64_t *prop; > + uint64_t hartid = U64_MAX; > + > + chosen_node = fdt_path_offset((void *)fdt, "/chosen"); > + if (chosen_node < 0) > + return hartid; Just return U64_MAX here > + prop = fdt_getprop((void *)fdt, chosen_node, "efi-boot-hartid", &len); Please call this 'boot-hartid' not 'efi-boot-hartid' as the hartid value is independent of whether you boot via EFI or not. > + if (!prop || len != sizeof(u64)) > + return hartid; > + Return U64_MAX > + hartid = fdt64_to_cpu(*prop); > + and just return the swabbed value, so you can get rid of the local var. > + return hartid; > +} > + > +/* > + * 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) This prototype has changed, and now includes the size of the fdt in param 3. > +{ > + unsigned long kernel_entry = entrypoint + _start_kernel - _start; stext_offset ? It has a terrible name though, and I'll probably propose to change it at some point, for all arches. But you can still use it here. > + jump_kernel_func jump_kernel = (void (*)(unsigned int, unsigned long))kernel_entry; > + u64 hartid = get_boot_hartid_from_fdt(fdt); > + > + if (hartid == U64_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 preferred_offset; > + > + /* > + * The preferred offset of the kernel Image is TEXT_OFFSET bytes beyond > + * a KIMG_ALIGN aligned base. > + */ > + preferred_offset = round_up(dram_base, MIN_KIMG_ALIGN) + TEXT_OFFSET; > + > + kernel_size = _edata - _start; > + kernel_memsize = kernel_size + (_end - _edata); > + > + /* > + * Try a straight allocation at the preferred offset. > + * This will work around the issue where, if dram_base == 0x0, > + * efi_low_alloc() refuses to allocate at 0x0 (to prevent the > + * address of the allocation to be mistaken for a FAIL return > + * value or a NULL pointer). It will also ensure that, on > + * platforms where the [dram_base, dram_base + TEXT_OFFSET) > + * interval is partially occupied by the firmware (like on APM > + * Mustang), we can still place the kernel at the address > + * 'dram_base + TEXT_OFFSET'. Better drop this entire last sentence (unless it is relevant, but then rework it to drop the APM Mustang reference) > + */ > + if (*image_addr == preferred_offset) > + return EFI_SUCCESS; > + > + *image_addr = *reserve_addr = preferred_offset; > + *reserve_size = round_up(kernel_memsize, EFI_ALLOC_ALIGN); > + > + status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS, > + EFI_LOADER_DATA, > + *reserve_size / EFI_PAGE_SIZE, > + (efi_physical_addr_t *)reserve_addr); > + > + if (status != EFI_SUCCESS) { > + *reserve_size = kernel_memsize + TEXT_OFFSET; > + status = efi_low_alloc(*reserve_size, MIN_KIMG_ALIGN, > + reserve_addr); > + > + if (status != EFI_SUCCESS) { > + pr_efi_err("Failed to relocate kernel\n"); > + *reserve_size = 0; > + return status; > + } > + *image_addr = *reserve_addr + TEXT_OFFSET; > + } > + memcpy((void *)*image_addr, image->image_base, kernel_size); > + > + return EFI_SUCCESS; > +} > -- > 2.24.0 >
On Wed, 2020-02-26 at 08:28 +0100, Ard Biesheuvel wrote: > On Wed, 26 Feb 2020 at 02:10, 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 + > > drivers/firmware/efi/libstub/Makefile | 8 ++ > > drivers/firmware/efi/libstub/riscv-stub.c | 135 > > ++++++++++++++++++++++ > > 5 files changed, 165 insertions(+) > > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index 42c122170cfd..68b1d565e51d 100644 > > --- a/arch/riscv/Kconfig > > +++ b/arch/riscv/Kconfig > > @@ -372,10 +372,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_ARCH_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 b9009a2fbaf5..0afaa89ba9ad 100644 > > --- a/arch/riscv/Makefile > > +++ b/arch/riscv/Makefile > > @@ -78,6 +78,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 e2ff95cb3390..0a5d3578f51e 100644 > > --- a/arch/riscv/configs/defconfig > > +++ b/arch/riscv/configs/defconfig > > @@ -125,3 +125,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > > # CONFIG_FTRACE is not set > > # CONFIG_RUNTIME_TESTING_MENU is not set > > CONFIG_MEMTEST=y > > +CONFIG_EFI=y > > diff --git a/drivers/firmware/efi/libstub/Makefile > > b/drivers/firmware/efi/libstub/Makefile > > index 2c5b76787126..38facb61745b 100644 > > --- a/drivers/firmware/efi/libstub/Makefile > > +++ b/drivers/firmware/efi/libstub/Makefile > > @@ -21,6 +21,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_ARCH_STUB) += > > -I$(srctree)/scripts/dtc/libfdt > > > > @@ -55,6 +57,7 @@ lib-$(CONFIG_EFI_GENERIC_ARCH_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) > > > > @@ -79,6 +82,11 @@ 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..3935b29ea93a > > --- /dev/null > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > > @@ -0,0 +1,135 @@ > > +// 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. > > + */ > > +#if IS_ENABLED(CONFIG_64BIT) > > You can use #ifdef here > ok. > > +#define MIN_KIMG_ALIGN SZ_2M > > +#else > > +#define MIN_KIMG_ALIGN SZ_4M > > +#endif > > +/* > > + * TEXT_OFFSET ensures that we don't overwrite the firmware that > > probably sits > > + * at the beginning of the DRAM. > > + */ > > Ugh. Really? On an EFI system, that memory should be reserved in some > way, we shouldn't be able to stomp on it like that. > Currently, we reserve the initial 128KB for run time firmware(only openSBI for now, EDK2 later) by using PMP (physical memory protection). Any acess to that region from supervisor mode (i.e. U-Boot) will result in a fault. Is it mandatory for UEFI to reserve the beginning of the DRAM ? > > +#define TEXT_OFFSET MIN_KIMG_ALIGN > > + > > +typedef __attribute__((noreturn)) void > > (*jump_kernel_func)(unsigned int, > > + unsigned > > long); > > + > > +efi_status_t check_platform_features(void) > > +{ > > + return EFI_SUCCESS; > > +} > > + > > +u64 get_boot_hartid_from_fdt(unsigned long fdt) > > static > > > +{ > > + int chosen_node, len; > > + const fdt64_t *prop; > > + uint64_t hartid = U64_MAX; > > + > > + chosen_node = fdt_path_offset((void *)fdt, "/chosen"); > > + if (chosen_node < 0) > > + return hartid; > > Just return U64_MAX here > > > + prop = fdt_getprop((void *)fdt, chosen_node, "efi-boot- > > hartid", &len); > > Please call this 'boot-hartid' not 'efi-boot-hartid' as the hartid > value is independent of whether you boot via EFI or not. > > > + if (!prop || len != sizeof(u64)) > > + return hartid; > > + > > Return U64_MAX > > > + hartid = fdt64_to_cpu(*prop); > > + > > and just return the swabbed value, so you can get rid of the local > var. > Fixed all the above issues. I changed it to u32 as u64 won't work on 32 bit systems. > > + return hartid; > > +} > > + > > +/* > > + * 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) > > This prototype has changed, and now includes the size of the fdt in > param 3. > Ahh yes. Fixed. > > +{ > > + unsigned long kernel_entry = entrypoint + _start_kernel - > > _start; > > stext_offset ? It has a terrible name though, and I'll probably > propose to change it at some point, for all arches. But you can still > use it here. > Sure. I updated it with stext_offset. > > + jump_kernel_func jump_kernel = (void (*)(unsigned int, > > unsigned long))kernel_entry; > > + u64 hartid = get_boot_hartid_from_fdt(fdt); > > + > > + if (hartid == U64_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 preferred_offset; > > + > > + /* > > + * The preferred offset of the kernel Image is TEXT_OFFSET > > bytes beyond > > + * a KIMG_ALIGN aligned base. > > + */ > > + preferred_offset = round_up(dram_base, MIN_KIMG_ALIGN) + > > TEXT_OFFSET; > > + > > + kernel_size = _edata - _start; > > + kernel_memsize = kernel_size + (_end - _edata); > > + > > + /* > > + * Try a straight allocation at the preferred offset. > > + * This will work around the issue where, if dram_base == > > 0x0, > > + * efi_low_alloc() refuses to allocate at 0x0 (to prevent > > the > > + * address of the allocation to be mistaken for a FAIL > > return > > + * value or a NULL pointer). It will also ensure that, on > > + * platforms where the [dram_base, dram_base + TEXT_OFFSET) > > + * interval is partially occupied by the firmware (like on > > APM > > + * Mustang), we can still place the kernel at the address > > + * 'dram_base + TEXT_OFFSET'. > > Better drop this entire last sentence (unless it is relevant, but > then > rework it to drop the APM Mustang reference) > As stated above, RISC-V firmware occupies [dram_base, dram_base + 128K). That's why I thought this comment is useful. I should have removed the mustand reference. I will update it. > > + */ > > + if (*image_addr == preferred_offset) > > + return EFI_SUCCESS; > > + > > + *image_addr = *reserve_addr = preferred_offset; > > + *reserve_size = round_up(kernel_memsize, EFI_ALLOC_ALIGN); > > + > > + status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS, > > + EFI_LOADER_DATA, > > + *reserve_size / EFI_PAGE_SIZE, > > + (efi_physical_addr_t > > *)reserve_addr); > > + > > + if (status != EFI_SUCCESS) { > > + *reserve_size = kernel_memsize + TEXT_OFFSET; > > + status = efi_low_alloc(*reserve_size, > > MIN_KIMG_ALIGN, > > + reserve_addr); > > + > > + if (status != EFI_SUCCESS) { > > + pr_efi_err("Failed to relocate kernel\n"); > > + *reserve_size = 0; > > + return status; > > + } > > + *image_addr = *reserve_addr + TEXT_OFFSET; > > + } > > + memcpy((void *)*image_addr, image->image_base, > > kernel_size); > > + > > + return EFI_SUCCESS; > > +} > > -- > > 2.24.0 > >
On Thu, 27 Feb 2020 at 20:53, Atish Patra <Atish.Patra@wdc.com> wrote: > > On Wed, 2020-02-26 at 08:28 +0100, Ard Biesheuvel wrote: > > On Wed, 26 Feb 2020 at 02:10, 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 + > > > drivers/firmware/efi/libstub/Makefile | 8 ++ > > > drivers/firmware/efi/libstub/riscv-stub.c | 135 > > > ++++++++++++++++++++++ > > > 5 files changed, 165 insertions(+) > > > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > index 42c122170cfd..68b1d565e51d 100644 > > > --- a/arch/riscv/Kconfig > > > +++ b/arch/riscv/Kconfig > > > @@ -372,10 +372,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_ARCH_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 b9009a2fbaf5..0afaa89ba9ad 100644 > > > --- a/arch/riscv/Makefile > > > +++ b/arch/riscv/Makefile > > > @@ -78,6 +78,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 e2ff95cb3390..0a5d3578f51e 100644 > > > --- a/arch/riscv/configs/defconfig > > > +++ b/arch/riscv/configs/defconfig > > > @@ -125,3 +125,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > > > # CONFIG_FTRACE is not set > > > # CONFIG_RUNTIME_TESTING_MENU is not set > > > CONFIG_MEMTEST=y > > > +CONFIG_EFI=y > > > diff --git a/drivers/firmware/efi/libstub/Makefile > > > b/drivers/firmware/efi/libstub/Makefile > > > index 2c5b76787126..38facb61745b 100644 > > > --- a/drivers/firmware/efi/libstub/Makefile > > > +++ b/drivers/firmware/efi/libstub/Makefile > > > @@ -21,6 +21,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_ARCH_STUB) += > > > -I$(srctree)/scripts/dtc/libfdt > > > > > > @@ -55,6 +57,7 @@ lib-$(CONFIG_EFI_GENERIC_ARCH_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) > > > > > > @@ -79,6 +82,11 @@ 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..3935b29ea93a > > > --- /dev/null > > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > > > @@ -0,0 +1,135 @@ > > > +// 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. > > > + */ > > > +#if IS_ENABLED(CONFIG_64BIT) > > > > You can use #ifdef here > > > > ok. > > > > +#define MIN_KIMG_ALIGN SZ_2M > > > +#else > > > +#define MIN_KIMG_ALIGN SZ_4M > > > +#endif > > > +/* > > > + * TEXT_OFFSET ensures that we don't overwrite the firmware that > > > probably sits > > > + * at the beginning of the DRAM. > > > + */ > > > > Ugh. Really? On an EFI system, that memory should be reserved in some > > way, we shouldn't be able to stomp on it like that. > > > > Currently, we reserve the initial 128KB for run time firmware(only > openSBI for now, EDK2 later) by using PMP (physical memory protection). > Any acess to that region from supervisor mode (i.e. U-Boot) will result > in a fault. > > Is it mandatory for UEFI to reserve the beginning of the DRAM ? > It is mandatory to describe which memory is usable and which memory is reserved. If this memory is not usable, you either describe it as reserved, or not describe it at all. Describing it as usable memory, allocating it for the kernel but with a hidden agreement that it is reserved is highly likely to cause problems down the road. > > > +#define TEXT_OFFSET MIN_KIMG_ALIGN > > > + > > > +typedef __attribute__((noreturn)) void > > > (*jump_kernel_func)(unsigned int, > > > + unsigned > > > long); > > > + > > > +efi_status_t check_platform_features(void) > > > +{ > > > + return EFI_SUCCESS; > > > +} > > > + > > > +u64 get_boot_hartid_from_fdt(unsigned long fdt) > > > > static > > > > > +{ > > > + int chosen_node, len; > > > + const fdt64_t *prop; > > > + uint64_t hartid = U64_MAX; > > > + > > > + chosen_node = fdt_path_offset((void *)fdt, "/chosen"); > > > + if (chosen_node < 0) > > > + return hartid; > > > > Just return U64_MAX here > > > > > + prop = fdt_getprop((void *)fdt, chosen_node, "efi-boot- > > > hartid", &len); > > > > Please call this 'boot-hartid' not 'efi-boot-hartid' as the hartid > > value is independent of whether you boot via EFI or not. > > > > > + if (!prop || len != sizeof(u64)) > > > + return hartid; > > > + > > > > Return U64_MAX > > > > > + hartid = fdt64_to_cpu(*prop); > > > + > > > > and just return the swabbed value, so you can get rid of the local > > var. > > > > Fixed all the above issues. I changed it to u32 as u64 won't work on 32 > bit systems. > If the hart id is only 32 bits max then i guess that will work. > > > + return hartid; > > > +} > > > + > > > +/* > > > + * 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) > > > > This prototype has changed, and now includes the size of the fdt in > > param 3. > > > > Ahh yes. Fixed. > > > > +{ > > > + unsigned long kernel_entry = entrypoint + _start_kernel - > > > _start; > > > > stext_offset ? It has a terrible name though, and I'll probably > > propose to change it at some point, for all arches. But you can still > > use it here. > > > > Sure. I updated it with stext_offset. > > > > + jump_kernel_func jump_kernel = (void (*)(unsigned int, > > > unsigned long))kernel_entry; > > > + u64 hartid = get_boot_hartid_from_fdt(fdt); > > > + > > > + if (hartid == U64_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 preferred_offset; > > > + > > > + /* > > > + * The preferred offset of the kernel Image is TEXT_OFFSET > > > bytes beyond > > > + * a KIMG_ALIGN aligned base. > > > + */ > > > + preferred_offset = round_up(dram_base, MIN_KIMG_ALIGN) + > > > TEXT_OFFSET; > > > + > > > + kernel_size = _edata - _start; > > > + kernel_memsize = kernel_size + (_end - _edata); > > > + > > > + /* > > > + * Try a straight allocation at the preferred offset. > > > + * This will work around the issue where, if dram_base == > > > 0x0, > > > + * efi_low_alloc() refuses to allocate at 0x0 (to prevent > > > the > > > + * address of the allocation to be mistaken for a FAIL > > > return > > > + * value or a NULL pointer). It will also ensure that, on > > > + * platforms where the [dram_base, dram_base + TEXT_OFFSET) > > > + * interval is partially occupied by the firmware (like on > > > APM > > > + * Mustang), we can still place the kernel at the address > > > + * 'dram_base + TEXT_OFFSET'. > > > > Better drop this entire last sentence (unless it is relevant, but > > then > > rework it to drop the APM Mustang reference) > > > > As stated above, RISC-V firmware occupies [dram_base, dram_base + > 128K). That's why I thought this comment is useful. I should have > removed the mustand reference. I will update it. > > > > + */ > > > + if (*image_addr == preferred_offset) > > > + return EFI_SUCCESS; > > > + > > > + *image_addr = *reserve_addr = preferred_offset; > > > + *reserve_size = round_up(kernel_memsize, EFI_ALLOC_ALIGN); > > > + > > > + status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS, > > > + EFI_LOADER_DATA, > > > + *reserve_size / EFI_PAGE_SIZE, > > > + (efi_physical_addr_t > > > *)reserve_addr); > > > + > > > + if (status != EFI_SUCCESS) { > > > + *reserve_size = kernel_memsize + TEXT_OFFSET; > > > + status = efi_low_alloc(*reserve_size, > > > MIN_KIMG_ALIGN, > > > + reserve_addr); > > > + > > > + if (status != EFI_SUCCESS) { > > > + pr_efi_err("Failed to relocate kernel\n"); > > > + *reserve_size = 0; > > > + return status; > > > + } > > > + *image_addr = *reserve_addr + TEXT_OFFSET; > > > + } > > > + memcpy((void *)*image_addr, image->image_base, > > > kernel_size); > > > + > > > + return EFI_SUCCESS; > > > +} > > > -- > > > 2.24.0 > > > > > -- > Regards, > Atish
On Thu, 2020-02-27 at 20:59 +0100, Ard Biesheuvel wrote: > On Thu, 27 Feb 2020 at 20:53, Atish Patra <Atish.Patra@wdc.com> > wrote: > > On Wed, 2020-02-26 at 08:28 +0100, Ard Biesheuvel wrote: > > > On Wed, 26 Feb 2020 at 02:10, 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 + > > > > drivers/firmware/efi/libstub/Makefile | 8 ++ > > > > drivers/firmware/efi/libstub/riscv-stub.c | 135 > > > > ++++++++++++++++++++++ > > > > 5 files changed, 165 insertions(+) > > > > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > index 42c122170cfd..68b1d565e51d 100644 > > > > --- a/arch/riscv/Kconfig > > > > +++ b/arch/riscv/Kconfig > > > > @@ -372,10 +372,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_ARCH_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 b9009a2fbaf5..0afaa89ba9ad 100644 > > > > --- a/arch/riscv/Makefile > > > > +++ b/arch/riscv/Makefile > > > > @@ -78,6 +78,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 e2ff95cb3390..0a5d3578f51e 100644 > > > > --- a/arch/riscv/configs/defconfig > > > > +++ b/arch/riscv/configs/defconfig > > > > @@ -125,3 +125,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > > > > # CONFIG_FTRACE is not set > > > > # CONFIG_RUNTIME_TESTING_MENU is not set > > > > CONFIG_MEMTEST=y > > > > +CONFIG_EFI=y > > > > diff --git a/drivers/firmware/efi/libstub/Makefile > > > > b/drivers/firmware/efi/libstub/Makefile > > > > index 2c5b76787126..38facb61745b 100644 > > > > --- a/drivers/firmware/efi/libstub/Makefile > > > > +++ b/drivers/firmware/efi/libstub/Makefile > > > > @@ -21,6 +21,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_ARCH_STUB) += > > > > -I$(srctree)/scripts/dtc/libfdt > > > > > > > > @@ -55,6 +57,7 @@ lib- > > > > $(CONFIG_EFI_GENERIC_ARCH_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) > > > > > > > > @@ -79,6 +82,11 @@ 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..3935b29ea93a > > > > --- /dev/null > > > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > > > > @@ -0,0 +1,135 @@ > > > > +// 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. > > > > + */ > > > > +#if IS_ENABLED(CONFIG_64BIT) > > > > > > You can use #ifdef here > > > > > > > ok. > > > > > > +#define MIN_KIMG_ALIGN SZ_2M > > > > +#else > > > > +#define MIN_KIMG_ALIGN SZ_4M > > > > +#endif > > > > +/* > > > > + * TEXT_OFFSET ensures that we don't overwrite the firmware > > > > that > > > > probably sits > > > > + * at the beginning of the DRAM. > > > > + */ > > > > > > Ugh. Really? On an EFI system, that memory should be reserved in > > > some > > > way, we shouldn't be able to stomp on it like that. > > > > > > > Currently, we reserve the initial 128KB for run time firmware(only > > openSBI for now, EDK2 later) by using PMP (physical memory > > protection). > > Any acess to that region from supervisor mode (i.e. U-Boot) will > > result > > in a fault. > > > > Is it mandatory for UEFI to reserve the beginning of the DRAM ? > > > > It is mandatory to describe which memory is usable and which memory > is > reserved. If this memory is not usable, you either describe it as > reserved, or not describe it at all. Describing it as usable memory, > allocating it for the kernel but with a hidden agreement that it is > reserved is highly likely to cause problems down the road. > I completely agree with you on this. We have been talking to have a booting guide and memory map document for RISC-V Linux to document all the idiosyncries of RISC-V. But that has not happend until now. Once, the ordered booting patches are merged, I will try to take a stab at it. Other than that, do we need to describe it somewhere in U-boot wrt to UEFI so that it doesn't allocate memory from that region ? > > > > > > +#define TEXT_OFFSET MIN_KIMG_ALIGN > > > > + > > > > +typedef __attribute__((noreturn)) void > > > > (*jump_kernel_func)(unsigned int, > > > > + unsi > > > > gned > > > > long); > > > > + > > > > +efi_status_t check_platform_features(void) > > > > +{ > > > > + return EFI_SUCCESS; > > > > +} > > > > + > > > > +u64 get_boot_hartid_from_fdt(unsigned long fdt) > > > > > > static > > > > > > > +{ > > > > + int chosen_node, len; > > > > + const fdt64_t *prop; > > > > + uint64_t hartid = U64_MAX; > > > > + > > > > + chosen_node = fdt_path_offset((void *)fdt, "/chosen"); > > > > + if (chosen_node < 0) > > > > + return hartid; > > > > > > Just return U64_MAX here > > > > > > > + prop = fdt_getprop((void *)fdt, chosen_node, "efi-boot- > > > > hartid", &len); > > > > > > Please call this 'boot-hartid' not 'efi-boot-hartid' as the > > > hartid > > > value is independent of whether you boot via EFI or not. > > > > > > > + if (!prop || len != sizeof(u64)) > > > > + return hartid; > > > > + > > > > > > Return U64_MAX > > > > > > > + hartid = fdt64_to_cpu(*prop); > > > > + > > > > > > and just return the swabbed value, so you can get rid of the > > > local > > > var. > > > > > > > Fixed all the above issues. I changed it to u32 as u64 won't work > > on 32 > > bit systems. > > > > If the hart id is only 32 bits max then i guess that will work. > > > > > + return hartid; > > > > +} > > > > + > > > > +/* > > > > + * 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) > > > > > > This prototype has changed, and now includes the size of the fdt > > > in > > > param 3. > > > > > > > Ahh yes. Fixed. > > > > > > +{ > > > > + unsigned long kernel_entry = entrypoint + _start_kernel > > > > - > > > > _start; > > > > > > stext_offset ? It has a terrible name though, and I'll probably > > > propose to change it at some point, for all arches. But you can > > > still > > > use it here. > > > > > > > Sure. I updated it with stext_offset. > > > > > > + jump_kernel_func jump_kernel = (void (*)(unsigned int, > > > > unsigned long))kernel_entry; > > > > + u64 hartid = get_boot_hartid_from_fdt(fdt); > > > > + > > > > + if (hartid == U64_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 preferred_offset; > > > > + > > > > + /* > > > > + * The preferred offset of the kernel Image is > > > > TEXT_OFFSET > > > > bytes beyond > > > > + * a KIMG_ALIGN aligned base. > > > > + */ > > > > + preferred_offset = round_up(dram_base, MIN_KIMG_ALIGN) > > > > + > > > > TEXT_OFFSET; > > > > + > > > > + kernel_size = _edata - _start; > > > > + kernel_memsize = kernel_size + (_end - _edata); > > > > + > > > > + /* > > > > + * Try a straight allocation at the preferred offset. > > > > + * This will work around the issue where, if dram_base > > > > == > > > > 0x0, > > > > + * efi_low_alloc() refuses to allocate at 0x0 (to > > > > prevent > > > > the > > > > + * address of the allocation to be mistaken for a FAIL > > > > return > > > > + * value or a NULL pointer). It will also ensure that, > > > > on > > > > + * platforms where the [dram_base, dram_base + > > > > TEXT_OFFSET) > > > > + * interval is partially occupied by the firmware (like > > > > on > > > > APM > > > > + * Mustang), we can still place the kernel at the > > > > address > > > > + * 'dram_base + TEXT_OFFSET'. > > > > > > Better drop this entire last sentence (unless it is relevant, but > > > then > > > rework it to drop the APM Mustang reference) > > > > > > > As stated above, RISC-V firmware occupies [dram_base, dram_base + > > 128K). That's why I thought this comment is useful. I should have > > removed the mustand reference. I will update it. > > > > > > + */ > > > > + if (*image_addr == preferred_offset) > > > > + return EFI_SUCCESS; > > > > + > > > > + *image_addr = *reserve_addr = preferred_offset; > > > > + *reserve_size = round_up(kernel_memsize, > > > > EFI_ALLOC_ALIGN); > > > > + > > > > + status = efi_bs_call(allocate_pages, > > > > EFI_ALLOCATE_ADDRESS, > > > > + EFI_LOADER_DATA, > > > > + *reserve_size / EFI_PAGE_SIZE, > > > > + (efi_physical_addr_t > > > > *)reserve_addr); > > > > + > > > > + if (status != EFI_SUCCESS) { > > > > + *reserve_size = kernel_memsize + TEXT_OFFSET; > > > > + status = efi_low_alloc(*reserve_size, > > > > MIN_KIMG_ALIGN, > > > > + reserve_addr); > > > > + > > > > + if (status != EFI_SUCCESS) { > > > > + pr_efi_err("Failed to relocate > > > > kernel\n"); > > > > + *reserve_size = 0; > > > > + return status; > > > > + } > > > > + *image_addr = *reserve_addr + TEXT_OFFSET; > > > > + } > > > > + memcpy((void *)*image_addr, image->image_base, > > > > kernel_size); > > > > + > > > > + return EFI_SUCCESS; > > > > +} > > > > -- > > > > 2.24.0 > > > > > > > > -- > > Regards, > > Atish
On Fri, 28 Feb 2020 at 02:05, Atish Patra <Atish.Patra@wdc.com> wrote: > > On Thu, 2020-02-27 at 20:59 +0100, Ard Biesheuvel wrote: > > On Thu, 27 Feb 2020 at 20:53, Atish Patra <Atish.Patra@wdc.com> > > wrote: > > > On Wed, 2020-02-26 at 08:28 +0100, Ard Biesheuvel wrote: > > > > On Wed, 26 Feb 2020 at 02:10, 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 + > > > > > drivers/firmware/efi/libstub/Makefile | 8 ++ > > > > > drivers/firmware/efi/libstub/riscv-stub.c | 135 > > > > > ++++++++++++++++++++++ > > > > > 5 files changed, 165 insertions(+) > > > > > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > index 42c122170cfd..68b1d565e51d 100644 > > > > > --- a/arch/riscv/Kconfig > > > > > +++ b/arch/riscv/Kconfig > > > > > @@ -372,10 +372,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_ARCH_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 b9009a2fbaf5..0afaa89ba9ad 100644 > > > > > --- a/arch/riscv/Makefile > > > > > +++ b/arch/riscv/Makefile > > > > > @@ -78,6 +78,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 e2ff95cb3390..0a5d3578f51e 100644 > > > > > --- a/arch/riscv/configs/defconfig > > > > > +++ b/arch/riscv/configs/defconfig > > > > > @@ -125,3 +125,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > > > > > # CONFIG_FTRACE is not set > > > > > # CONFIG_RUNTIME_TESTING_MENU is not set > > > > > CONFIG_MEMTEST=y > > > > > +CONFIG_EFI=y > > > > > diff --git a/drivers/firmware/efi/libstub/Makefile > > > > > b/drivers/firmware/efi/libstub/Makefile > > > > > index 2c5b76787126..38facb61745b 100644 > > > > > --- a/drivers/firmware/efi/libstub/Makefile > > > > > +++ b/drivers/firmware/efi/libstub/Makefile > > > > > @@ -21,6 +21,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_ARCH_STUB) += > > > > > -I$(srctree)/scripts/dtc/libfdt > > > > > > > > > > @@ -55,6 +57,7 @@ lib- > > > > > $(CONFIG_EFI_GENERIC_ARCH_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) > > > > > > > > > > @@ -79,6 +82,11 @@ 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..3935b29ea93a > > > > > --- /dev/null > > > > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > > > > > @@ -0,0 +1,135 @@ > > > > > +// 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. > > > > > + */ > > > > > +#if IS_ENABLED(CONFIG_64BIT) > > > > > > > > You can use #ifdef here > > > > > > > > > > ok. > > > > > > > > +#define MIN_KIMG_ALIGN SZ_2M > > > > > +#else > > > > > +#define MIN_KIMG_ALIGN SZ_4M > > > > > +#endif > > > > > +/* > > > > > + * TEXT_OFFSET ensures that we don't overwrite the firmware > > > > > that > > > > > probably sits > > > > > + * at the beginning of the DRAM. > > > > > + */ > > > > > > > > Ugh. Really? On an EFI system, that memory should be reserved in > > > > some > > > > way, we shouldn't be able to stomp on it like that. > > > > > > > > > > Currently, we reserve the initial 128KB for run time firmware(only > > > openSBI for now, EDK2 later) by using PMP (physical memory > > > protection). > > > Any acess to that region from supervisor mode (i.e. U-Boot) will > > > result > > > in a fault. > > > > > > Is it mandatory for UEFI to reserve the beginning of the DRAM ? > > > > > > > It is mandatory to describe which memory is usable and which memory > > is > > reserved. If this memory is not usable, you either describe it as > > reserved, or not describe it at all. Describing it as usable memory, > > allocating it for the kernel but with a hidden agreement that it is > > reserved is highly likely to cause problems down the road. > > > > I completely agree with you on this. We have been talking to have a > booting guide and memory map document for RISC-V Linux to document all > the idiosyncries of RISC-V. But that has not happend until now. > Once, the ordered booting patches are merged, I will try to take a stab > at it. > > Other than that, do we need to describe it somewhere in U-boot wrt to > UEFI so that it doesn't allocate memory from that region ? > It is an idiosyncrasy that the firmware should hide from the OS. What if GRUB comes along and attempts to allocate that memory? Do we also have to teach it that the first 128 KB memory of free memory are magic and should not be touched? So the answer is to mark it as reserved. This way, no UEFI tools, bootloaders etc will ever try to use it. Then, in the stub, you can tweak the existing code to cheat a bit, and make the TEXT_OFFSET window overlap the 128 KB reserved window at the bottom of memory. Doing that in the stub is fine - this is part of the kernel so it can know about crazy RISC-V rules.
On Thu, Feb 27, 2020 at 10:57 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Fri, 28 Feb 2020 at 02:05, Atish Patra <Atish.Patra@wdc.com> wrote: > > > > On Thu, 2020-02-27 at 20:59 +0100, Ard Biesheuvel wrote: > > > On Thu, 27 Feb 2020 at 20:53, Atish Patra <Atish.Patra@wdc.com> > > > wrote: > > > > On Wed, 2020-02-26 at 08:28 +0100, Ard Biesheuvel wrote: > > > > > On Wed, 26 Feb 2020 at 02:10, 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 + > > > > > > drivers/firmware/efi/libstub/Makefile | 8 ++ > > > > > > drivers/firmware/efi/libstub/riscv-stub.c | 135 > > > > > > ++++++++++++++++++++++ > > > > > > 5 files changed, 165 insertions(+) > > > > > > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > index 42c122170cfd..68b1d565e51d 100644 > > > > > > --- a/arch/riscv/Kconfig > > > > > > +++ b/arch/riscv/Kconfig > > > > > > @@ -372,10 +372,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_ARCH_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 b9009a2fbaf5..0afaa89ba9ad 100644 > > > > > > --- a/arch/riscv/Makefile > > > > > > +++ b/arch/riscv/Makefile > > > > > > @@ -78,6 +78,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 e2ff95cb3390..0a5d3578f51e 100644 > > > > > > --- a/arch/riscv/configs/defconfig > > > > > > +++ b/arch/riscv/configs/defconfig > > > > > > @@ -125,3 +125,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > > > > > > # CONFIG_FTRACE is not set > > > > > > # CONFIG_RUNTIME_TESTING_MENU is not set > > > > > > CONFIG_MEMTEST=y > > > > > > +CONFIG_EFI=y > > > > > > diff --git a/drivers/firmware/efi/libstub/Makefile > > > > > > b/drivers/firmware/efi/libstub/Makefile > > > > > > index 2c5b76787126..38facb61745b 100644 > > > > > > --- a/drivers/firmware/efi/libstub/Makefile > > > > > > +++ b/drivers/firmware/efi/libstub/Makefile > > > > > > @@ -21,6 +21,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_ARCH_STUB) += > > > > > > -I$(srctree)/scripts/dtc/libfdt > > > > > > > > > > > > @@ -55,6 +57,7 @@ lib- > > > > > > $(CONFIG_EFI_GENERIC_ARCH_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) > > > > > > > > > > > > @@ -79,6 +82,11 @@ 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..3935b29ea93a > > > > > > --- /dev/null > > > > > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > > > > > > @@ -0,0 +1,135 @@ > > > > > > +// 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. > > > > > > + */ > > > > > > +#if IS_ENABLED(CONFIG_64BIT) > > > > > > > > > > You can use #ifdef here > > > > > > > > > > > > > ok. > > > > > > > > > > +#define MIN_KIMG_ALIGN SZ_2M > > > > > > +#else > > > > > > +#define MIN_KIMG_ALIGN SZ_4M > > > > > > +#endif > > > > > > +/* > > > > > > + * TEXT_OFFSET ensures that we don't overwrite the firmware > > > > > > that > > > > > > probably sits > > > > > > + * at the beginning of the DRAM. > > > > > > + */ > > > > > > > > > > Ugh. Really? On an EFI system, that memory should be reserved in > > > > > some > > > > > way, we shouldn't be able to stomp on it like that. > > > > > > > > > > > > > Currently, we reserve the initial 128KB for run time firmware(only > > > > openSBI for now, EDK2 later) by using PMP (physical memory > > > > protection). > > > > Any acess to that region from supervisor mode (i.e. U-Boot) will > > > > result > > > > in a fault. > > > > > > > > Is it mandatory for UEFI to reserve the beginning of the DRAM ? > > > > > > > > > > It is mandatory to describe which memory is usable and which memory > > > is > > > reserved. If this memory is not usable, you either describe it as > > > reserved, or not describe it at all. Describing it as usable memory, > > > allocating it for the kernel but with a hidden agreement that it is > > > reserved is highly likely to cause problems down the road. > > > > > > > I completely agree with you on this. We have been talking to have a > > booting guide and memory map document for RISC-V Linux to document all > > the idiosyncries of RISC-V. But that has not happend until now. > > Once, the ordered booting patches are merged, I will try to take a stab > > at it. > > > > Other than that, do we need to describe it somewhere in U-boot wrt to > > UEFI so that it doesn't allocate memory from that region ? > > > > It is an idiosyncrasy that the firmware should hide from the OS. > > What if GRUB comes along and attempts to allocate that memory? Do we > also have to teach it that the first 128 KB memory of free memory are > magic and should not be touched? > > So the answer is to mark it as reserved. This way, no UEFI tools, > bootloaders etc will ever try to use it. Sounds good to me. We are currently discussing the best approach to provide reserved memory information to U-Boot/EDK2. The idea is to U-Boot/EDK2 may have to update the DT with reserved-memory node so that Linux is aware of the reservation as well. Then, in the stub, you can > tweak the existing code to cheat a bit, and make the TEXT_OFFSET > window overlap the 128 KB reserved window at the bottom of memory. > Doing that in the stub is fine - this is part of the kernel so it can > know about crazy RISC-V rules. I am bit confused here. Why does EFI stub need to overlap the reserved memory. I thought EFI stub just needs to parse the DT to find out the reserved memory region to make sure that it doesn't try to access/overwrite that region.
On Tue, Mar 10, 2020 at 12:39 PM Atish Patra <atishp@atishpatra.org> wrote: > > On Thu, Feb 27, 2020 at 10:57 PM Ard Biesheuvel <ardb@kernel.org> wrote: > > > > On Fri, 28 Feb 2020 at 02:05, Atish Patra <Atish.Patra@wdc.com> wrote: > > > > > > On Thu, 2020-02-27 at 20:59 +0100, Ard Biesheuvel wrote: > > > > On Thu, 27 Feb 2020 at 20:53, Atish Patra <Atish.Patra@wdc.com> > > > > wrote: > > > > > On Wed, 2020-02-26 at 08:28 +0100, Ard Biesheuvel wrote: > > > > > > On Wed, 26 Feb 2020 at 02:10, 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 + > > > > > > > drivers/firmware/efi/libstub/Makefile | 8 ++ > > > > > > > drivers/firmware/efi/libstub/riscv-stub.c | 135 > > > > > > > ++++++++++++++++++++++ > > > > > > > 5 files changed, 165 insertions(+) > > > > > > > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > > > > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > > > > > > index 42c122170cfd..68b1d565e51d 100644 > > > > > > > --- a/arch/riscv/Kconfig > > > > > > > +++ b/arch/riscv/Kconfig > > > > > > > @@ -372,10 +372,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_ARCH_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 b9009a2fbaf5..0afaa89ba9ad 100644 > > > > > > > --- a/arch/riscv/Makefile > > > > > > > +++ b/arch/riscv/Makefile > > > > > > > @@ -78,6 +78,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 e2ff95cb3390..0a5d3578f51e 100644 > > > > > > > --- a/arch/riscv/configs/defconfig > > > > > > > +++ b/arch/riscv/configs/defconfig > > > > > > > @@ -125,3 +125,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y > > > > > > > # CONFIG_FTRACE is not set > > > > > > > # CONFIG_RUNTIME_TESTING_MENU is not set > > > > > > > CONFIG_MEMTEST=y > > > > > > > +CONFIG_EFI=y > > > > > > > diff --git a/drivers/firmware/efi/libstub/Makefile > > > > > > > b/drivers/firmware/efi/libstub/Makefile > > > > > > > index 2c5b76787126..38facb61745b 100644 > > > > > > > --- a/drivers/firmware/efi/libstub/Makefile > > > > > > > +++ b/drivers/firmware/efi/libstub/Makefile > > > > > > > @@ -21,6 +21,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_ARCH_STUB) += > > > > > > > -I$(srctree)/scripts/dtc/libfdt > > > > > > > > > > > > > > @@ -55,6 +57,7 @@ lib- > > > > > > > $(CONFIG_EFI_GENERIC_ARCH_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) > > > > > > > > > > > > > > @@ -79,6 +82,11 @@ 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..3935b29ea93a > > > > > > > --- /dev/null > > > > > > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c > > > > > > > @@ -0,0 +1,135 @@ > > > > > > > +// 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. > > > > > > > + */ > > > > > > > +#if IS_ENABLED(CONFIG_64BIT) > > > > > > > > > > > > You can use #ifdef here > > > > > > > > > > > > > > > > ok. > > > > > > > > > > > > +#define MIN_KIMG_ALIGN SZ_2M > > > > > > > +#else > > > > > > > +#define MIN_KIMG_ALIGN SZ_4M > > > > > > > +#endif > > > > > > > +/* > > > > > > > + * TEXT_OFFSET ensures that we don't overwrite the firmware > > > > > > > that > > > > > > > probably sits > > > > > > > + * at the beginning of the DRAM. > > > > > > > + */ > > > > > > > > > > > > Ugh. Really? On an EFI system, that memory should be reserved in > > > > > > some > > > > > > way, we shouldn't be able to stomp on it like that. > > > > > > > > > > > > > > > > Currently, we reserve the initial 128KB for run time firmware(only > > > > > openSBI for now, EDK2 later) by using PMP (physical memory > > > > > protection). > > > > > Any acess to that region from supervisor mode (i.e. U-Boot) will > > > > > result > > > > > in a fault. > > > > > > > > > > Is it mandatory for UEFI to reserve the beginning of the DRAM ? > > > > > > > > > > > > > It is mandatory to describe which memory is usable and which memory > > > > is > > > > reserved. If this memory is not usable, you either describe it as > > > > reserved, or not describe it at all. Describing it as usable memory, > > > > allocating it for the kernel but with a hidden agreement that it is > > > > reserved is highly likely to cause problems down the road. > > > > > > > > > > I completely agree with you on this. We have been talking to have a > > > booting guide and memory map document for RISC-V Linux to document all > > > the idiosyncries of RISC-V. But that has not happend until now. > > > Once, the ordered booting patches are merged, I will try to take a stab > > > at it. > > > > > > Other than that, do we need to describe it somewhere in U-boot wrt to > > > UEFI so that it doesn't allocate memory from that region ? > > > > > > > It is an idiosyncrasy that the firmware should hide from the OS. > > > > What if GRUB comes along and attempts to allocate that memory? Do we > > also have to teach it that the first 128 KB memory of free memory are > > magic and should not be touched? > > > > So the answer is to mark it as reserved. This way, no UEFI tools, > > bootloaders etc will ever try to use it. > > Sounds good to me. We are currently discussing the best approach to > provide reserved memory > information to U-Boot/EDK2. The idea is to U-Boot/EDK2 may have to > update the DT with > reserved-memory node so that Linux is aware of the reservation as well. The discussion is happening on Github at: https://github.com/riscv/riscv-sbi-doc/pull/37 Feel free to share your views in above mentioned Github link. Regards, Anup
On 10/03/2020, Atish Patra <atishp@atishpatra.org> wrote: > On Thu, Feb 27, 2020 at 10:57 PM Ard Biesheuvel <ardb@kernel.org> wrote: >> >> On Fri, 28 Feb 2020 at 02:05, Atish Patra <Atish.Patra@wdc.com> wrote: >> > >> > On Thu, 2020-02-27 at 20:59 +0100, Ard Biesheuvel wrote: >> > > On Thu, 27 Feb 2020 at 20:53, Atish Patra <Atish.Patra@wdc.com> >> > > wrote: >> > > > On Wed, 2020-02-26 at 08:28 +0100, Ard Biesheuvel wrote: >> > > > > On Wed, 26 Feb 2020 at 02:10, 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 + >> > > > > > drivers/firmware/efi/libstub/Makefile | 8 ++ >> > > > > > drivers/firmware/efi/libstub/riscv-stub.c | 135 >> > > > > > ++++++++++++++++++++++ >> > > > > > 5 files changed, 165 insertions(+) >> > > > > > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c >> > > > > > >> > > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >> > > > > > index 42c122170cfd..68b1d565e51d 100644 >> > > > > > --- a/arch/riscv/Kconfig >> > > > > > +++ b/arch/riscv/Kconfig >> > > > > > @@ -372,10 +372,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_ARCH_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 b9009a2fbaf5..0afaa89ba9ad 100644 >> > > > > > --- a/arch/riscv/Makefile >> > > > > > +++ b/arch/riscv/Makefile >> > > > > > @@ -78,6 +78,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 e2ff95cb3390..0a5d3578f51e 100644 >> > > > > > --- a/arch/riscv/configs/defconfig >> > > > > > +++ b/arch/riscv/configs/defconfig >> > > > > > @@ -125,3 +125,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y >> > > > > > # CONFIG_FTRACE is not set >> > > > > > # CONFIG_RUNTIME_TESTING_MENU is not set >> > > > > > CONFIG_MEMTEST=y >> > > > > > +CONFIG_EFI=y >> > > > > > diff --git a/drivers/firmware/efi/libstub/Makefile >> > > > > > b/drivers/firmware/efi/libstub/Makefile >> > > > > > index 2c5b76787126..38facb61745b 100644 >> > > > > > --- a/drivers/firmware/efi/libstub/Makefile >> > > > > > +++ b/drivers/firmware/efi/libstub/Makefile >> > > > > > @@ -21,6 +21,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_ARCH_STUB) += >> > > > > > -I$(srctree)/scripts/dtc/libfdt >> > > > > > >> > > > > > @@ -55,6 +57,7 @@ lib- >> > > > > > $(CONFIG_EFI_GENERIC_ARCH_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) >> > > > > > >> > > > > > @@ -79,6 +82,11 @@ 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..3935b29ea93a >> > > > > > --- /dev/null >> > > > > > +++ b/drivers/firmware/efi/libstub/riscv-stub.c >> > > > > > @@ -0,0 +1,135 @@ >> > > > > > +// 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. >> > > > > > + */ >> > > > > > +#if IS_ENABLED(CONFIG_64BIT) >> > > > > >> > > > > You can use #ifdef here >> > > > > >> > > > >> > > > ok. >> > > > >> > > > > > +#define MIN_KIMG_ALIGN SZ_2M >> > > > > > +#else >> > > > > > +#define MIN_KIMG_ALIGN SZ_4M >> > > > > > +#endif >> > > > > > +/* >> > > > > > + * TEXT_OFFSET ensures that we don't overwrite the firmware >> > > > > > that >> > > > > > probably sits >> > > > > > + * at the beginning of the DRAM. >> > > > > > + */ >> > > > > >> > > > > Ugh. Really? On an EFI system, that memory should be reserved in >> > > > > some >> > > > > way, we shouldn't be able to stomp on it like that. >> > > > > >> > > > >> > > > Currently, we reserve the initial 128KB for run time firmware(only >> > > > openSBI for now, EDK2 later) by using PMP (physical memory >> > > > protection). >> > > > Any acess to that region from supervisor mode (i.e. U-Boot) will >> > > > result >> > > > in a fault. >> > > > >> > > > Is it mandatory for UEFI to reserve the beginning of the DRAM ? >> > > > >> > > >> > > It is mandatory to describe which memory is usable and which memory >> > > is >> > > reserved. If this memory is not usable, you either describe it as >> > > reserved, or not describe it at all. Describing it as usable memory, >> > > allocating it for the kernel but with a hidden agreement that it is >> > > reserved is highly likely to cause problems down the road. >> > > >> > >> > I completely agree with you on this. We have been talking to have a >> > booting guide and memory map document for RISC-V Linux to document all >> > the idiosyncries of RISC-V. But that has not happend until now. >> > Once, the ordered booting patches are merged, I will try to take a stab >> > at it. >> > >> > Other than that, do we need to describe it somewhere in U-boot wrt to >> > UEFI so that it doesn't allocate memory from that region ? >> > >> >> It is an idiosyncrasy that the firmware should hide from the OS. >> >> What if GRUB comes along and attempts to allocate that memory? Do we >> also have to teach it that the first 128 KB memory of free memory are >> magic and should not be touched? >> >> So the answer is to mark it as reserved. This way, no UEFI tools, >> bootloaders etc will ever try to use it. > > Sounds good to me. We are currently discussing the best approach to > provide reserved memory > information to U-Boot/EDK2. The idea is to U-Boot/EDK2 may have to > update the DT with > reserved-memory node so that Linux is aware of the reservation as well. > The efi stub disregards dt memory information entirely, so the reservation should be made in the efi memory map. For edk2, I don’t think there is any point in adding these reservations to the dt at all. > Then, in the stub, you can >> tweak the existing code to cheat a bit, and make the TEXT_OFFSET >> window overlap the 128 KB reserved window at the bottom of memory. >> Doing that in the stub is fine - this is part of the kernel so it can >> know about crazy RISC-V rules. > > I am bit confused here. Why does EFI stub need to overlap the reserved > memory. > I thought EFI stub just needs to parse the DT to find out the reserved > memory region to make sure > that it doesn't try to access/overwrite that region. The efi stub does not use dt memory information, since it has its own memory map. If some memory exists that may not be used for general allocation, you have to reserve it. Then, as an optimization, you may combine the stub’s knowledge about riscv in particular, and decide to use an allocation at the bottom of memory that partially overlaps in a way that is known to be safe
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 42c122170cfd..68b1d565e51d 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -372,10 +372,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_ARCH_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 b9009a2fbaf5..0afaa89ba9ad 100644 --- a/arch/riscv/Makefile +++ b/arch/riscv/Makefile @@ -78,6 +78,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 e2ff95cb3390..0a5d3578f51e 100644 --- a/arch/riscv/configs/defconfig +++ b/arch/riscv/configs/defconfig @@ -125,3 +125,4 @@ CONFIG_DEBUG_BLOCK_EXT_DEVT=y # CONFIG_FTRACE is not set # CONFIG_RUNTIME_TESTING_MENU is not set CONFIG_MEMTEST=y +CONFIG_EFI=y diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index 2c5b76787126..38facb61745b 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -21,6 +21,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_ARCH_STUB) += -I$(srctree)/scripts/dtc/libfdt @@ -55,6 +57,7 @@ lib-$(CONFIG_EFI_GENERIC_ARCH_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) @@ -79,6 +82,11 @@ 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..3935b29ea93a --- /dev/null +++ b/drivers/firmware/efi/libstub/riscv-stub.c @@ -0,0 +1,135 @@ +// 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. + */ +#if IS_ENABLED(CONFIG_64BIT) +#define MIN_KIMG_ALIGN SZ_2M +#else +#define MIN_KIMG_ALIGN SZ_4M +#endif +/* + * TEXT_OFFSET ensures that we don't overwrite the firmware that probably sits + * at the beginning of the DRAM. + */ +#define TEXT_OFFSET MIN_KIMG_ALIGN + +typedef __attribute__((noreturn)) void (*jump_kernel_func)(unsigned int, + unsigned long); + +efi_status_t check_platform_features(void) +{ + return EFI_SUCCESS; +} + +u64 get_boot_hartid_from_fdt(unsigned long fdt) +{ + int chosen_node, len; + const fdt64_t *prop; + uint64_t hartid = U64_MAX; + + chosen_node = fdt_path_offset((void *)fdt, "/chosen"); + if (chosen_node < 0) + return hartid; + prop = fdt_getprop((void *)fdt, chosen_node, "efi-boot-hartid", &len); + if (!prop || len != sizeof(u64)) + return hartid; + + hartid = fdt64_to_cpu(*prop); + + return hartid; +} + +/* + * 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 kernel_entry = entrypoint + _start_kernel - _start; + jump_kernel_func jump_kernel = (void (*)(unsigned int, unsigned long))kernel_entry; + u64 hartid = get_boot_hartid_from_fdt(fdt); + + if (hartid == U64_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 preferred_offset; + + /* + * The preferred offset of the kernel Image is TEXT_OFFSET bytes beyond + * a KIMG_ALIGN aligned base. + */ + preferred_offset = round_up(dram_base, MIN_KIMG_ALIGN) + TEXT_OFFSET; + + kernel_size = _edata - _start; + kernel_memsize = kernel_size + (_end - _edata); + + /* + * Try a straight allocation at the preferred offset. + * This will work around the issue where, if dram_base == 0x0, + * efi_low_alloc() refuses to allocate at 0x0 (to prevent the + * address of the allocation to be mistaken for a FAIL return + * value or a NULL pointer). It will also ensure that, on + * platforms where the [dram_base, dram_base + TEXT_OFFSET) + * interval is partially occupied by the firmware (like on APM + * Mustang), we can still place the kernel at the address + * 'dram_base + TEXT_OFFSET'. + */ + if (*image_addr == preferred_offset) + return EFI_SUCCESS; + + *image_addr = *reserve_addr = preferred_offset; + *reserve_size = round_up(kernel_memsize, EFI_ALLOC_ALIGN); + + status = efi_bs_call(allocate_pages, EFI_ALLOCATE_ADDRESS, + EFI_LOADER_DATA, + *reserve_size / EFI_PAGE_SIZE, + (efi_physical_addr_t *)reserve_addr); + + if (status != EFI_SUCCESS) { + *reserve_size = kernel_memsize + TEXT_OFFSET; + status = efi_low_alloc(*reserve_size, MIN_KIMG_ALIGN, + reserve_addr); + + if (status != EFI_SUCCESS) { + pr_efi_err("Failed to relocate kernel\n"); + *reserve_size = 0; + return status; + } + *image_addr = *reserve_addr + TEXT_OFFSET; + } + memcpy((void *)*image_addr, image->image_base, 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 + drivers/firmware/efi/libstub/Makefile | 8 ++ drivers/firmware/efi/libstub/riscv-stub.c | 135 ++++++++++++++++++++++ 5 files changed, 165 insertions(+) create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c