* [RFC PATCH 1/5] efi: Move arm-stub to a common file
2020-02-26 1:10 [RFC PATCH 0/5] Add UEFI support for RISC-V Atish Patra
@ 2020-02-26 1:10 ` Atish Patra
2020-02-26 7:04 ` Ard Biesheuvel
2020-02-26 1:10 ` [RFC PATCH 2/5] include: pe.h: Add RISC-V related PE definition Atish Patra
` (4 subsequent siblings)
5 siblings, 1 reply; 26+ messages in thread
From: Atish Patra @ 2020-02-26 1:10 UTC (permalink / raw)
To: linux-kernel
Cc: Atish Patra, Albert Ou, Alexios Zavras, Allison Randal,
Andrew Morton, Anup Patel, Ard Biesheuvel, Arnd Bergmann,
Borislav Petkov, Catalin Marinas, Greentime Hu,
Greg Kroah-Hartman, Ingo Molnar, Kate Stewart, Kees Cook,
Linus Walleij, linux-efi, linux-riscv, Mao Han,
Mauro Carvalho Chehab, Michal Simek, Mike Rapoport,
Palmer Dabbelt, Paolo Bonzini, Paul Walmsley, Russell King,
Thomas Gleixner, Will Deacon, Alexander Graf, leif, Chang,
Abner (HPS SW/FW Technologist),
daniel.schaefer
Most of the arm-stub code is written in an architecture independent manner.
As a result, RISC-V can reuse most of the arm-stub code.
Rename the arm-stub.c to efi-stub.c so that ARM, ARM64 and RISC-V can use it.
This patch doesn't introduce any functional changes.
Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
arch/arm/Kconfig | 2 +-
arch/arm64/Kconfig | 2 +-
drivers/firmware/efi/Kconfig | 6 +++---
drivers/firmware/efi/libstub/Makefile | 12 ++++++------
.../firmware/efi/libstub/{arm-stub.c => efi-stub.c} | 7 ++++++-
5 files changed, 17 insertions(+), 12 deletions(-)
rename drivers/firmware/efi/libstub/{arm-stub.c => efi-stub.c} (98%)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 97864aabc2a6..9931fea06076 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1955,7 +1955,7 @@ config EFI
select UCS2_STRING
select EFI_PARAMS_FROM_FDT
select EFI_STUB
- select EFI_ARMSTUB
+ select EFI_GENERIC_ARCH_STUB
select EFI_RUNTIME_WRAPPERS
---help---
This option provides support for runtime services provided
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 0b30e884e088..ae776d8ef2f9 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1720,7 +1720,7 @@ config EFI
select EFI_PARAMS_FROM_FDT
select EFI_RUNTIME_WRAPPERS
select EFI_STUB
- select EFI_ARMSTUB
+ select EFI_GENERIC_ARCH_STUB
default y
help
This option provides support for runtime services provided
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index ecc83e2f032c..1bcedb7812da 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -106,12 +106,12 @@ config EFI_PARAMS_FROM_FDT
config EFI_RUNTIME_WRAPPERS
bool
-config EFI_ARMSTUB
+config EFI_GENERIC_ARCH_STUB
bool
-config EFI_ARMSTUB_DTB_LOADER
+config EFI_STUB_DTB_LOADER
bool "Enable the DTB loader"
- depends on EFI_ARMSTUB
+ depends on EFI_GENERIC_ARCH_STUB
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 4d6246c6f651..2c5b76787126 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -22,7 +22,7 @@ cflags-$(CONFIG_ARM) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \
-fno-builtin -fpic \
$(call cc-option,-mno-single-pic-base)
-cflags-$(CONFIG_EFI_ARMSTUB) += -I$(srctree)/scripts/dtc/libfdt
+cflags-$(CONFIG_EFI_GENERIC_ARCH_STUB) += -I$(srctree)/scripts/dtc/libfdt
KBUILD_CFLAGS := $(cflags-y) -DDISABLE_BRANCH_PROFILING \
-include $(srctree)/drivers/firmware/efi/libstub/hidden.h \
@@ -44,13 +44,13 @@ lib-y := efi-stub-helper.o gop.o secureboot.o tpm.o \
skip_spaces.o lib-cmdline.o lib-ctype.o
# include the stub's generic dependencies from lib/ when building for ARM/arm64
-arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
+efi-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
$(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
$(call if_changed_rule,cc_o_c)
-lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o \
- $(patsubst %.c,lib-%.o,$(arm-deps-y))
+lib-$(CONFIG_EFI_GENERIC_ARCH_STUB) += efi-stub.o fdt.o string.o \
+ $(patsubst %.c,lib-%.o,$(efi-deps-y))
lib-$(CONFIG_ARM) += arm32-stub.o
lib-$(CONFIG_ARM64) += arm64-stub.o
@@ -72,8 +72,8 @@ CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET)
# a verification pass to see if any absolute relocations exist in any of the
# object files.
#
-extra-$(CONFIG_EFI_ARMSTUB) := $(lib-y)
-lib-$(CONFIG_EFI_ARMSTUB) := $(patsubst %.o,%.stub.o,$(lib-y))
+extra-$(CONFIG_EFI_GENERIC_ARCH_STUB) := $(lib-y)
+lib-$(CONFIG_EFI_GENERIC_ARCH_STUB) := $(patsubst %.o,%.stub.o,$(lib-y))
STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \
--prefix-symbols=__efistub_
diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
similarity index 98%
rename from drivers/firmware/efi/libstub/arm-stub.c
rename to drivers/firmware/efi/libstub/efi-stub.c
index 13559c7e6643..b87c3f70430c 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -15,6 +15,7 @@
#include "efistub.h"
+#if IS_ENABLED(CONFIG_ARM64) || IS_ENABLED(CONFIG_ARM)
/*
* This is the base address at which to start allocating virtual memory ranges
* for UEFI Runtime Services. This is in the low TTBR0 range so that we can use
@@ -27,6 +28,10 @@
* entire footprint of the UEFI runtime services memory regions)
*/
#define EFI_RT_VIRTUAL_BASE SZ_512M
+#else
+#define EFI_RT_VIRTUAL_BASE SZ_2G
+#endif
+
#define EFI_RT_VIRTUAL_SIZE SZ_512M
#ifdef CONFIG_ARM64
@@ -243,7 +248,7 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
* 'dtb=' unless UEFI Secure Boot is disabled. We assume that secure
* boot is enabled if we can't determine its state.
*/
- if (!IS_ENABLED(CONFIG_EFI_ARMSTUB_DTB_LOADER) ||
+ if (!IS_ENABLED(CONFIG_EFI_STUB_DTB_LOADER) ||
secure_boot != efi_secureboot_mode_disabled) {
if (strstr(cmdline_ptr, "dtb="))
pr_efi("Ignoring DTB from command line.\n");
--
2.24.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 1/5] efi: Move arm-stub to a common file
2020-02-26 1:10 ` [RFC PATCH 1/5] efi: Move arm-stub to a common file Atish Patra
@ 2020-02-26 7:04 ` Ard Biesheuvel
2020-02-27 1:16 ` Atish Patra
0 siblings, 1 reply; 26+ messages in thread
From: Ard Biesheuvel @ 2020-02-26 7:04 UTC (permalink / raw)
To: Atish Patra
Cc: Linux Kernel Mailing List, Albert Ou, Alexios Zavras,
Allison Randal, Andrew Morton, Anup Patel, Arnd Bergmann,
Borislav Petkov, Catalin Marinas, Greentime Hu,
Greg Kroah-Hartman, Ingo Molnar, Kate Stewart, Kees Cook,
Linus Walleij, linux-efi, linux-riscv, Mao Han,
Mauro Carvalho Chehab, Michal Simek, Mike Rapoport,
Palmer Dabbelt, Paolo Bonzini, Paul Walmsley, Russell King,
Thomas Gleixner, Will Deacon, Alexander Graf, leif, Chang,
Abner (HPS SW/FW Technologist), Schaefer, Daniel (DualStudy)
Hi Atish,
On Wed, 26 Feb 2020 at 02:10, Atish Patra <atish.patra@wdc.com> wrote:
>
> Most of the arm-stub code is written in an architecture independent manner.
> As a result, RISC-V can reuse most of the arm-stub code.
>
> Rename the arm-stub.c to efi-stub.c so that ARM, ARM64 and RISC-V can use it.
> This patch doesn't introduce any functional changes.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
> arch/arm/Kconfig | 2 +-
> arch/arm64/Kconfig | 2 +-
> drivers/firmware/efi/Kconfig | 6 +++---
> drivers/firmware/efi/libstub/Makefile | 12 ++++++------
> .../firmware/efi/libstub/{arm-stub.c => efi-stub.c} | 7 ++++++-
> 5 files changed, 17 insertions(+), 12 deletions(-)
> rename drivers/firmware/efi/libstub/{arm-stub.c => efi-stub.c} (98%)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 97864aabc2a6..9931fea06076 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1955,7 +1955,7 @@ config EFI
> select UCS2_STRING
> select EFI_PARAMS_FROM_FDT
> select EFI_STUB
> - select EFI_ARMSTUB
> + select EFI_GENERIC_ARCH_STUB
> select EFI_RUNTIME_WRAPPERS
> ---help---
> This option provides support for runtime services provided
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 0b30e884e088..ae776d8ef2f9 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1720,7 +1720,7 @@ config EFI
> select EFI_PARAMS_FROM_FDT
> select EFI_RUNTIME_WRAPPERS
> select EFI_STUB
> - select EFI_ARMSTUB
> + select EFI_GENERIC_ARCH_STUB
> default y
> help
> This option provides support for runtime services provided
> diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
> index ecc83e2f032c..1bcedb7812da 100644
> --- a/drivers/firmware/efi/Kconfig
> +++ b/drivers/firmware/efi/Kconfig
> @@ -106,12 +106,12 @@ config EFI_PARAMS_FROM_FDT
> config EFI_RUNTIME_WRAPPERS
> bool
>
> -config EFI_ARMSTUB
> +config EFI_GENERIC_ARCH_STUB
Let's call it EFI_GENERIC_STUB
> bool
>
> -config EFI_ARMSTUB_DTB_LOADER
> +config EFI_STUB_DTB_LOADER
> bool "Enable the DTB loader"
> - depends on EFI_ARMSTUB
> + depends on EFI_GENERIC_ARCH_STUB
Do you actually need the DTB loader? I feel adding this for ARM was a
mistake, so you may want to make this depend on !RISCV while you're at
it.
> 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 4d6246c6f651..2c5b76787126 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -22,7 +22,7 @@ cflags-$(CONFIG_ARM) := $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \
> -fno-builtin -fpic \
> $(call cc-option,-mno-single-pic-base)
>
> -cflags-$(CONFIG_EFI_ARMSTUB) += -I$(srctree)/scripts/dtc/libfdt
> +cflags-$(CONFIG_EFI_GENERIC_ARCH_STUB) += -I$(srctree)/scripts/dtc/libfdt
>
> KBUILD_CFLAGS := $(cflags-y) -DDISABLE_BRANCH_PROFILING \
> -include $(srctree)/drivers/firmware/efi/libstub/hidden.h \
> @@ -44,13 +44,13 @@ lib-y := efi-stub-helper.o gop.o secureboot.o tpm.o \
> skip_spaces.o lib-cmdline.o lib-ctype.o
>
> # include the stub's generic dependencies from lib/ when building for ARM/arm64
> -arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
> +efi-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
>
> $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
> $(call if_changed_rule,cc_o_c)
>
> -lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o \
> - $(patsubst %.c,lib-%.o,$(arm-deps-y))
> +lib-$(CONFIG_EFI_GENERIC_ARCH_STUB) += efi-stub.o fdt.o string.o \
> + $(patsubst %.c,lib-%.o,$(efi-deps-y))
>
> lib-$(CONFIG_ARM) += arm32-stub.o
> lib-$(CONFIG_ARM64) += arm64-stub.o
> @@ -72,8 +72,8 @@ CFLAGS_arm64-stub.o := -DTEXT_OFFSET=$(TEXT_OFFSET)
> # a verification pass to see if any absolute relocations exist in any of the
> # object files.
> #
> -extra-$(CONFIG_EFI_ARMSTUB) := $(lib-y)
> -lib-$(CONFIG_EFI_ARMSTUB) := $(patsubst %.o,%.stub.o,$(lib-y))
> +extra-$(CONFIG_EFI_GENERIC_ARCH_STUB) := $(lib-y)
> +lib-$(CONFIG_EFI_GENERIC_ARCH_STUB) := $(patsubst %.o,%.stub.o,$(lib-y))
>
> STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \
> --prefix-symbols=__efistub_
> diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
> similarity index 98%
> rename from drivers/firmware/efi/libstub/arm-stub.c
> rename to drivers/firmware/efi/libstub/efi-stub.c
> index 13559c7e6643..b87c3f70430c 100644
> --- a/drivers/firmware/efi/libstub/arm-stub.c
> +++ b/drivers/firmware/efi/libstub/efi-stub.c
> @@ -15,6 +15,7 @@
>
> #include "efistub.h"
>
> +#if IS_ENABLED(CONFIG_ARM64) || IS_ENABLED(CONFIG_ARM)
> /*
> * This is the base address at which to start allocating virtual memory ranges
> * for UEFI Runtime Services. This is in the low TTBR0 range so that we can use
> @@ -27,6 +28,10 @@
> * entire footprint of the UEFI runtime services memory regions)
> */
> #define EFI_RT_VIRTUAL_BASE SZ_512M
> +#else
> +#define EFI_RT_VIRTUAL_BASE SZ_2G
> +#endif
> +
Can we drop this hunk for now? It should be part of your runtime
services series.
> #define EFI_RT_VIRTUAL_SIZE SZ_512M
>
> #ifdef CONFIG_ARM64
> @@ -243,7 +248,7 @@ efi_status_t efi_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg)
> * 'dtb=' unless UEFI Secure Boot is disabled. We assume that secure
> * boot is enabled if we can't determine its state.
> */
> - if (!IS_ENABLED(CONFIG_EFI_ARMSTUB_DTB_LOADER) ||
> + if (!IS_ENABLED(CONFIG_EFI_STUB_DTB_LOADER) ||
> secure_boot != efi_secureboot_mode_disabled) {
> if (strstr(cmdline_ptr, "dtb="))
> pr_efi("Ignoring DTB from command line.\n");
> --
> 2.24.0
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [RFC PATCH 1/5] efi: Move arm-stub to a common file
2020-02-26 7:04 ` Ard Biesheuvel
@ 2020-02-27 1:16 ` Atish Patra
0 siblings, 0 replies; 26+ messages in thread
From: Atish Patra @ 2020-02-27 1:16 UTC (permalink / raw)
To: ardb
Cc: alexios.zavras, tglx, mchehab+samsung, pbonzini, michal.simek,
linux, abner.chang, linux-riscv, catalin.marinas, linux-kernel,
daniel.schaefer, Anup Patel, kstewart, palmer, aou, arnd, rppt,
linux-efi, bp, greentime.hu, keescook, agraf, will, gregkh,
mingo, allison, han_mao, paul.walmsley, linus.walleij, leif,
akpm
On Wed, 2020-02-26 at 08:04 +0100, Ard Biesheuvel wrote:
> Hi Atish,
>
> On Wed, 26 Feb 2020 at 02:10, Atish Patra <atish.patra@wdc.com>
> wrote:
> > Most of the arm-stub code is written in an architecture independent
> > manner.
> > As a result, RISC-V can reuse most of the arm-stub code.
> >
> > Rename the arm-stub.c to efi-stub.c so that ARM, ARM64 and RISC-V
> > can use it.
> > This patch doesn't introduce any functional changes.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > ---
> > arch/arm/Kconfig | 2 +-
> > arch/arm64/Kconfig | 2 +-
> > drivers/firmware/efi/Kconfig | 6 +++---
> > drivers/firmware/efi/libstub/Makefile | 12 ++++++
> > ------
> > .../firmware/efi/libstub/{arm-stub.c => efi-stub.c} | 7 ++++++-
> > 5 files changed, 17 insertions(+), 12 deletions(-)
> > rename drivers/firmware/efi/libstub/{arm-stub.c => efi-stub.c}
> > (98%)
> >
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index 97864aabc2a6..9931fea06076 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -1955,7 +1955,7 @@ config EFI
> > select UCS2_STRING
> > select EFI_PARAMS_FROM_FDT
> > select EFI_STUB
> > - select EFI_ARMSTUB
> > + select EFI_GENERIC_ARCH_STUB
> > select EFI_RUNTIME_WRAPPERS
> > ---help---
> > This option provides support for runtime services
> > provided
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 0b30e884e088..ae776d8ef2f9 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -1720,7 +1720,7 @@ config EFI
> > select EFI_PARAMS_FROM_FDT
> > select EFI_RUNTIME_WRAPPERS
> > select EFI_STUB
> > - select EFI_ARMSTUB
> > + select EFI_GENERIC_ARCH_STUB
> > default y
> > help
> > This option provides support for runtime services
> > provided
> > diff --git a/drivers/firmware/efi/Kconfig
> > b/drivers/firmware/efi/Kconfig
> > index ecc83e2f032c..1bcedb7812da 100644
> > --- a/drivers/firmware/efi/Kconfig
> > +++ b/drivers/firmware/efi/Kconfig
> > @@ -106,12 +106,12 @@ config EFI_PARAMS_FROM_FDT
> > config EFI_RUNTIME_WRAPPERS
> > bool
> >
> > -config EFI_ARMSTUB
> > +config EFI_GENERIC_ARCH_STUB
>
> Let's call it EFI_GENERIC_STUB
>
> > bool
> >
> > -config EFI_ARMSTUB_DTB_LOADER
> > +config EFI_STUB_DTB_LOADER
> > bool "Enable the DTB loader"
> > - depends on EFI_ARMSTUB
> > + depends on EFI_GENERIC_ARCH_STUB
>
> Do you actually need the DTB loader? I feel adding this for ARM was a
> mistake, so you may want to make this depend on !RISCV while you're
> at
> it.
>
Actually we don't need it RISC-V. I think I added that that's a valid
option to have. But your recent cleaup seems otherwise. I will make it
depend on !RISCV. It also doesn't make sense to rename it to
EFI_STUB_DTB_LOADER. So I will just leave EFI_ARMSTUB_DTB_LOADER as it
is.
> > 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 4d6246c6f651..2c5b76787126 100644
> > --- a/drivers/firmware/efi/libstub/Makefile
> > +++ b/drivers/firmware/efi/libstub/Makefile
> > @@ -22,7 +22,7 @@ cflags-$(CONFIG_ARM) := $(subst
> > $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \
> > -fno-builtin -fpic \
> > $(call cc-option,-mno-single-
> > pic-base)
> >
> > -cflags-$(CONFIG_EFI_ARMSTUB) += -I$(srctree)/scripts/dtc/libfdt
> > +cflags-$(CONFIG_EFI_GENERIC_ARCH_STUB) +=
> > -I$(srctree)/scripts/dtc/libfdt
> >
> > KBUILD_CFLAGS := $(cflags-y)
> > -DDISABLE_BRANCH_PROFILING \
> > -include
> > $(srctree)/drivers/firmware/efi/libstub/hidden.h \
> > @@ -44,13 +44,13 @@ lib-y := efi-
> > stub-helper.o gop.o secureboot.o tpm.o \
> > skip_spaces.o lib-cmdline.o lib-
> > ctype.o
> >
> > # include the stub's generic dependencies from lib/ when building
> > for ARM/arm64
> > -arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c
> > fdt_sw.c
> > +efi-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c
> > fdt_sw.c
> >
> > $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE
> > $(call if_changed_rule,cc_o_c)
> >
> > -lib-$(CONFIG_EFI_ARMSTUB) += arm-stub.o fdt.o string.o \
> > - $(patsubst %.c,lib-%.o,$(arm-
> > deps-y))
> > +lib-$(CONFIG_EFI_GENERIC_ARCH_STUB) += efi-stub.o fdt.o
> > string.o \
> > + $(patsubst %.c,lib-%.o,$(efi-
> > deps-y))
> >
> > lib-$(CONFIG_ARM) += arm32-stub.o
> > lib-$(CONFIG_ARM64) += arm64-stub.o
> > @@ -72,8 +72,8 @@ CFLAGS_arm64-stub.o :=
> > -DTEXT_OFFSET=$(TEXT_OFFSET)
> > # a verification pass to see if any absolute relocations exist in
> > any of the
> > # object files.
> > #
> > -extra-$(CONFIG_EFI_ARMSTUB) := $(lib-y)
> > -lib-$(CONFIG_EFI_ARMSTUB) := $(patsubst %.o,%.stub.o,$(lib-
> > y))
> > +extra-$(CONFIG_EFI_GENERIC_ARCH_STUB) := $(lib-y)
> > +lib-$(CONFIG_EFI_GENERIC_ARCH_STUB) := $(patsubst
> > %.o,%.stub.o,$(lib-y))
> >
> > STUBCOPY_FLAGS-$(CONFIG_ARM64) += --prefix-alloc-sections=.init \
> > --prefix-symbols=__efistub_
> > diff --git a/drivers/firmware/efi/libstub/arm-stub.c
> > b/drivers/firmware/efi/libstub/efi-stub.c
> > similarity index 98%
> > rename from drivers/firmware/efi/libstub/arm-stub.c
> > rename to drivers/firmware/efi/libstub/efi-stub.c
> > index 13559c7e6643..b87c3f70430c 100644
> > --- a/drivers/firmware/efi/libstub/arm-stub.c
> > +++ b/drivers/firmware/efi/libstub/efi-stub.c
> > @@ -15,6 +15,7 @@
> >
> > #include "efistub.h"
> >
> > +#if IS_ENABLED(CONFIG_ARM64) || IS_ENABLED(CONFIG_ARM)
> > /*
> > * This is the base address at which to start allocating virtual
> > memory ranges
> > * for UEFI Runtime Services. This is in the low TTBR0 range so
> > that we can use
> > @@ -27,6 +28,10 @@
> > * entire footprint of the UEFI runtime services memory regions)
> > */
> > #define EFI_RT_VIRTUAL_BASE SZ_512M
> > +#else
> > +#define EFI_RT_VIRTUAL_BASE SZ_2G
> > +#endif
> > +
>
> Can we drop this hunk for now? It should be part of your runtime
> services series.
>
Sure. Will do it.
> > #define EFI_RT_VIRTUAL_SIZE SZ_512M
> >
> > #ifdef CONFIG_ARM64
> > @@ -243,7 +248,7 @@ efi_status_t efi_entry(efi_handle_t handle,
> > efi_system_table_t *sys_table_arg)
> > * 'dtb=' unless UEFI Secure Boot is disabled. We assume
> > that secure
> > * boot is enabled if we can't determine its state.
> > */
> > - if (!IS_ENABLED(CONFIG_EFI_ARMSTUB_DTB_LOADER) ||
> > + if (!IS_ENABLED(CONFIG_EFI_STUB_DTB_LOADER) ||
> > secure_boot != efi_secureboot_mode_disabled) {
> > if (strstr(cmdline_ptr, "dtb="))
> > pr_efi("Ignoring DTB from command
> > line.\n");
> > --
> > 2.24.0
> >
--
Regards,
Atish
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 2/5] include: pe.h: Add RISC-V related PE definition
2020-02-26 1:10 [RFC PATCH 0/5] Add UEFI support for RISC-V Atish Patra
2020-02-26 1:10 ` [RFC PATCH 1/5] efi: Move arm-stub to a common file Atish Patra
@ 2020-02-26 1:10 ` Atish Patra
2020-02-26 7:06 ` Ard Biesheuvel
2020-02-26 1:10 ` [RFC PATCH 3/5] RISC-V: Define fixmap bindings for generic early ioremap support Atish Patra
` (3 subsequent siblings)
5 siblings, 1 reply; 26+ messages in thread
From: Atish Patra @ 2020-02-26 1:10 UTC (permalink / raw)
To: linux-kernel
Cc: Atish Patra, Albert Ou, Alexios Zavras, Allison Randal,
Andrew Morton, Anup Patel, Ard Biesheuvel, Arnd Bergmann,
Borislav Petkov, Catalin Marinas, Greentime Hu,
Greg Kroah-Hartman, Ingo Molnar, Kate Stewart, Kees Cook,
Linus Walleij, linux-efi, linux-riscv, Mao Han,
Mauro Carvalho Chehab, Michal Simek, Mike Rapoport,
Palmer Dabbelt, Paolo Bonzini, Paul Walmsley, Russell King,
Thomas Gleixner, Will Deacon, Alexander Graf, leif, Chang,
Abner (HPS SW/FW Technologist),
daniel.schaefer
Define RISC-V related machine types.
Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
include/linux/pe.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/pe.h b/include/linux/pe.h
index 8ad71d763a77..6a7c497e4b1f 100644
--- a/include/linux/pe.h
+++ b/include/linux/pe.h
@@ -56,6 +56,9 @@
#define IMAGE_FILE_MACHINE_POWERPCFP 0x01f1
#define IMAGE_FILE_MACHINE_R4000 0x0166
#define IMAGE_FILE_MACHINE_SH3 0x01a2
+#define IMAGE_FILE_MACHINE_RISCV32 0x5032
+#define IMAGE_FILE_MACHINE_RISCV64 0x5064
+#define IMAGE_FILE_MACHINE_RISCV128 0x5128
#define IMAGE_FILE_MACHINE_SH3DSP 0x01a3
#define IMAGE_FILE_MACHINE_SH3E 0x01a4
#define IMAGE_FILE_MACHINE_SH4 0x01a6
--
2.24.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 2/5] include: pe.h: Add RISC-V related PE definition
2020-02-26 1:10 ` [RFC PATCH 2/5] include: pe.h: Add RISC-V related PE definition Atish Patra
@ 2020-02-26 7:06 ` Ard Biesheuvel
2020-02-27 0:53 ` Atish Patra
0 siblings, 1 reply; 26+ messages in thread
From: Ard Biesheuvel @ 2020-02-26 7:06 UTC (permalink / raw)
To: Atish Patra
Cc: Linux Kernel Mailing List, Albert Ou, Alexios Zavras,
Allison Randal, Andrew Morton, Anup Patel, Arnd Bergmann,
Borislav Petkov, Catalin Marinas, Greentime Hu,
Greg Kroah-Hartman, Ingo Molnar, Kate Stewart, Kees Cook,
Linus Walleij, linux-efi, linux-riscv, Mao Han,
Mauro Carvalho Chehab, Michal Simek, Mike Rapoport,
Palmer Dabbelt, Paolo Bonzini, Paul Walmsley, Russell King,
Thomas Gleixner, Will Deacon, Alexander Graf, leif, Chang,
Abner (HPS SW/FW Technologist), Schaefer, Daniel (DualStudy)
On Wed, 26 Feb 2020 at 02:10, Atish Patra <atish.patra@wdc.com> wrote:
>
> Define RISC-V related machine types.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
If you put them in alphabetical order wrt SH3:
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> include/linux/pe.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/linux/pe.h b/include/linux/pe.h
> index 8ad71d763a77..6a7c497e4b1f 100644
> --- a/include/linux/pe.h
> +++ b/include/linux/pe.h
> @@ -56,6 +56,9 @@
> #define IMAGE_FILE_MACHINE_POWERPCFP 0x01f1
> #define IMAGE_FILE_MACHINE_R4000 0x0166
> #define IMAGE_FILE_MACHINE_SH3 0x01a2
> +#define IMAGE_FILE_MACHINE_RISCV32 0x5032
> +#define IMAGE_FILE_MACHINE_RISCV64 0x5064
> +#define IMAGE_FILE_MACHINE_RISCV128 0x5128
> #define IMAGE_FILE_MACHINE_SH3DSP 0x01a3
> #define IMAGE_FILE_MACHINE_SH3E 0x01a4
> #define IMAGE_FILE_MACHINE_SH4 0x01a6
> --
> 2.24.0
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH 2/5] include: pe.h: Add RISC-V related PE definition
2020-02-26 7:06 ` Ard Biesheuvel
@ 2020-02-27 0:53 ` Atish Patra
0 siblings, 0 replies; 26+ messages in thread
From: Atish Patra @ 2020-02-27 0:53 UTC (permalink / raw)
To: ardb
Cc: alexios.zavras, tglx, mchehab+samsung, pbonzini, michal.simek,
linux, abner.chang, linux-riscv, catalin.marinas, linux-kernel,
daniel.schaefer, Anup Patel, kstewart, palmer, aou, arnd, rppt,
linux-efi, bp, greentime.hu, keescook, agraf, will, gregkh,
mingo, allison, han_mao, paul.walmsley, linus.walleij, leif,
akpm
On Wed, 2020-02-26 at 08:06 +0100, Ard Biesheuvel wrote:
> On Wed, 26 Feb 2020 at 02:10, Atish Patra <atish.patra@wdc.com>
> wrote:
> > Define RISC-V related machine types.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
>
> If you put them in alphabetical order wrt SH3:
>
Done.
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
>
>
> > ---
> > include/linux/pe.h | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/include/linux/pe.h b/include/linux/pe.h
> > index 8ad71d763a77..6a7c497e4b1f 100644
> > --- a/include/linux/pe.h
> > +++ b/include/linux/pe.h
> > @@ -56,6 +56,9 @@
> > #define IMAGE_FILE_MACHINE_POWERPCFP 0x01f1
> > #define IMAGE_FILE_MACHINE_R4000 0x0166
> > #define IMAGE_FILE_MACHINE_SH3 0x01a2
> > +#define IMAGE_FILE_MACHINE_RISCV32 0x5032
> > +#define IMAGE_FILE_MACHINE_RISCV64 0x5064
> > +#define IMAGE_FILE_MACHINE_RISCV128 0x5128
> > #define IMAGE_FILE_MACHINE_SH3DSP 0x01a3
> > #define IMAGE_FILE_MACHINE_SH3E 0x01a4
> > #define IMAGE_FILE_MACHINE_SH4 0x01a6
> > --
> > 2.24.0
> >
--
Regards,
Atish
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 3/5] RISC-V: Define fixmap bindings for generic early ioremap support
2020-02-26 1:10 [RFC PATCH 0/5] Add UEFI support for RISC-V Atish Patra
2020-02-26 1:10 ` [RFC PATCH 1/5] efi: Move arm-stub to a common file Atish Patra
2020-02-26 1:10 ` [RFC PATCH 2/5] include: pe.h: Add RISC-V related PE definition Atish Patra
@ 2020-02-26 1:10 ` Atish Patra
2020-02-26 7:08 ` Ard Biesheuvel
2020-02-26 1:10 ` [RFC PATCH 4/5] RISC-V: Add PE/COFF header for EFI stub Atish Patra
` (2 subsequent siblings)
5 siblings, 1 reply; 26+ messages in thread
From: Atish Patra @ 2020-02-26 1:10 UTC (permalink / raw)
To: linux-kernel
Cc: Atish Patra, Albert Ou, Alexios Zavras, Allison Randal,
Andrew Morton, Anup Patel, Ard Biesheuvel, Arnd Bergmann,
Borislav Petkov, Catalin Marinas, Greentime Hu,
Greg Kroah-Hartman, Ingo Molnar, Kate Stewart, Kees Cook,
Linus Walleij, linux-efi, linux-riscv, Mao Han,
Mauro Carvalho Chehab, Michal Simek, Mike Rapoport,
Palmer Dabbelt, Paolo Bonzini, Paul Walmsley, Russell King,
Thomas Gleixner, Will Deacon, Alexander Graf, leif, Chang,
Abner (HPS SW/FW Technologist),
daniel.schaefer
UEFI uses early IO or memory mappings for runtime services before
normal ioremap() is usable. This patch only adds minimum necessary
fixmap bindings and headers for generic ioremap support to work.
Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
arch/riscv/Kconfig | 1 +
arch/riscv/include/asm/Kbuild | 1 +
arch/riscv/include/asm/fixmap.h | 21 ++++++++++++++++++++-
arch/riscv/include/asm/io.h | 1 +
4 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 27bfc7947e44..42c122170cfd 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -65,6 +65,7 @@ config RISCV
select ARCH_HAS_GCOV_PROFILE_ALL
select HAVE_COPY_THREAD_TLS
select HAVE_ARCH_KASAN if MMU && 64BIT
+ select GENERIC_EARLY_IOREMAP
config ARCH_MMAP_RND_BITS_MIN
default 18 if 64BIT
diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
index ec0ca8c6ab64..517394390106 100644
--- a/arch/riscv/include/asm/Kbuild
+++ b/arch/riscv/include/asm/Kbuild
@@ -4,6 +4,7 @@ generic-y += checksum.h
generic-y += compat.h
generic-y += device.h
generic-y += div64.h
+generic-y += early_ioremap.h
generic-y += extable.h
generic-y += flat.h
generic-y += dma.h
diff --git a/arch/riscv/include/asm/fixmap.h b/arch/riscv/include/asm/fixmap.h
index 42d2c42f3cc9..7a4beb7e29a3 100644
--- a/arch/riscv/include/asm/fixmap.h
+++ b/arch/riscv/include/asm/fixmap.h
@@ -25,9 +25,28 @@ enum fixed_addresses {
#define FIX_FDT_SIZE SZ_1M
FIX_FDT_END,
FIX_FDT = FIX_FDT_END + FIX_FDT_SIZE / PAGE_SIZE - 1,
+ FIX_EARLYCON_MEM_BASE,
+
FIX_PTE,
FIX_PMD,
- FIX_EARLYCON_MEM_BASE,
+ /*
+ * Make sure that it is 2MB aligned.
+ */
+#define NR_FIX_SZ_2M (SZ_2M / PAGE_SIZE)
+ FIX_THOLE = NR_FIX_SZ_2M - FIX_PMD - 1,
+
+ __end_of_permanent_fixed_addresses,
+ /*
+ * Temporary boot-time mappings, used by early_ioremap(),
+ * before ioremap() is functional.
+ */
+#define NR_FIX_BTMAPS (SZ_256K / PAGE_SIZE)
+#define FIX_BTMAPS_SLOTS 7
+#define TOTAL_FIX_BTMAPS (NR_FIX_BTMAPS * FIX_BTMAPS_SLOTS)
+
+ FIX_BTMAP_END = __end_of_permanent_fixed_addresses,
+ FIX_BTMAP_BEGIN = FIX_BTMAP_END + TOTAL_FIX_BTMAPS - 1,
+
__end_of_fixed_addresses
};
diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
index 0f477206a4ed..047f414b6948 100644
--- a/arch/riscv/include/asm/io.h
+++ b/arch/riscv/include/asm/io.h
@@ -14,6 +14,7 @@
#include <linux/types.h>
#include <asm/mmiowb.h>
#include <asm/pgtable.h>
+#include <asm/early_ioremap.h>
/*
* MMIO access functions are separated out to break dependency cycles
--
2.24.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 3/5] RISC-V: Define fixmap bindings for generic early ioremap support
2020-02-26 1:10 ` [RFC PATCH 3/5] RISC-V: Define fixmap bindings for generic early ioremap support Atish Patra
@ 2020-02-26 7:08 ` Ard Biesheuvel
2020-02-27 19:58 ` Atish Patra
0 siblings, 1 reply; 26+ messages in thread
From: Ard Biesheuvel @ 2020-02-26 7:08 UTC (permalink / raw)
To: Atish Patra
Cc: Linux Kernel Mailing List, Albert Ou, Alexios Zavras,
Allison Randal, Andrew Morton, Anup Patel, Arnd Bergmann,
Borislav Petkov, Catalin Marinas, Greentime Hu,
Greg Kroah-Hartman, Ingo Molnar, Kate Stewart, Kees Cook,
Linus Walleij, linux-efi, linux-riscv, Mao Han,
Mauro Carvalho Chehab, Michal Simek, Mike Rapoport,
Palmer Dabbelt, Paolo Bonzini, Paul Walmsley, Russell King,
Thomas Gleixner, Will Deacon, Alexander Graf, leif, Chang,
Abner (HPS SW/FW Technologist), Schaefer, Daniel (DualStudy)
On Wed, 26 Feb 2020 at 02:10, Atish Patra <atish.patra@wdc.com> wrote:
>
> UEFI uses early IO or memory mappings for runtime services before
> normal ioremap() is usable. This patch only adds minimum necessary
> fixmap bindings and headers for generic ioremap support to work.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
Looks reasonable to me,
Acked-by: Ard Biesheuvel <ardb@kernel.org>
although I wonder why it is part of this series?
> ---
> arch/riscv/Kconfig | 1 +
> arch/riscv/include/asm/Kbuild | 1 +
> arch/riscv/include/asm/fixmap.h | 21 ++++++++++++++++++++-
> arch/riscv/include/asm/io.h | 1 +
> 4 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 27bfc7947e44..42c122170cfd 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -65,6 +65,7 @@ config RISCV
> select ARCH_HAS_GCOV_PROFILE_ALL
> select HAVE_COPY_THREAD_TLS
> select HAVE_ARCH_KASAN if MMU && 64BIT
> + select GENERIC_EARLY_IOREMAP
>
> config ARCH_MMAP_RND_BITS_MIN
> default 18 if 64BIT
> diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
> index ec0ca8c6ab64..517394390106 100644
> --- a/arch/riscv/include/asm/Kbuild
> +++ b/arch/riscv/include/asm/Kbuild
> @@ -4,6 +4,7 @@ generic-y += checksum.h
> generic-y += compat.h
> generic-y += device.h
> generic-y += div64.h
> +generic-y += early_ioremap.h
> generic-y += extable.h
> generic-y += flat.h
> generic-y += dma.h
> diff --git a/arch/riscv/include/asm/fixmap.h b/arch/riscv/include/asm/fixmap.h
> index 42d2c42f3cc9..7a4beb7e29a3 100644
> --- a/arch/riscv/include/asm/fixmap.h
> +++ b/arch/riscv/include/asm/fixmap.h
> @@ -25,9 +25,28 @@ enum fixed_addresses {
> #define FIX_FDT_SIZE SZ_1M
> FIX_FDT_END,
> FIX_FDT = FIX_FDT_END + FIX_FDT_SIZE / PAGE_SIZE - 1,
> + FIX_EARLYCON_MEM_BASE,
> +
> FIX_PTE,
> FIX_PMD,
> - FIX_EARLYCON_MEM_BASE,
> + /*
> + * Make sure that it is 2MB aligned.
> + */
> +#define NR_FIX_SZ_2M (SZ_2M / PAGE_SIZE)
> + FIX_THOLE = NR_FIX_SZ_2M - FIX_PMD - 1,
> +
> + __end_of_permanent_fixed_addresses,
> + /*
> + * Temporary boot-time mappings, used by early_ioremap(),
> + * before ioremap() is functional.
> + */
> +#define NR_FIX_BTMAPS (SZ_256K / PAGE_SIZE)
> +#define FIX_BTMAPS_SLOTS 7
> +#define TOTAL_FIX_BTMAPS (NR_FIX_BTMAPS * FIX_BTMAPS_SLOTS)
> +
> + FIX_BTMAP_END = __end_of_permanent_fixed_addresses,
> + FIX_BTMAP_BEGIN = FIX_BTMAP_END + TOTAL_FIX_BTMAPS - 1,
> +
> __end_of_fixed_addresses
> };
>
> diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
> index 0f477206a4ed..047f414b6948 100644
> --- a/arch/riscv/include/asm/io.h
> +++ b/arch/riscv/include/asm/io.h
> @@ -14,6 +14,7 @@
> #include <linux/types.h>
> #include <asm/mmiowb.h>
> #include <asm/pgtable.h>
> +#include <asm/early_ioremap.h>
>
> /*
> * MMIO access functions are separated out to break dependency cycles
> --
> 2.24.0
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [RFC PATCH 3/5] RISC-V: Define fixmap bindings for generic early ioremap support
2020-02-26 7:08 ` Ard Biesheuvel
@ 2020-02-27 19:58 ` Atish Patra
2020-02-27 20:00 ` Ard Biesheuvel
0 siblings, 1 reply; 26+ messages in thread
From: Atish Patra @ 2020-02-27 19:58 UTC (permalink / raw)
To: ardb
Cc: alexios.zavras, tglx, mchehab+samsung, pbonzini, michal.simek,
linux, abner.chang, linux-riscv, catalin.marinas, linux-kernel,
daniel.schaefer, Anup Patel, kstewart, palmer, aou, arnd, rppt,
linux-efi, bp, greentime.hu, keescook, agraf, will, gregkh,
mingo, allison, han_mao, paul.walmsley, linus.walleij, leif,
akpm
On Wed, 2020-02-26 at 08:08 +0100, Ard Biesheuvel wrote:
> On Wed, 26 Feb 2020 at 02:10, Atish Patra <atish.patra@wdc.com>
> wrote:
> > UEFI uses early IO or memory mappings for runtime services before
> > normal ioremap() is usable. This patch only adds minimum necessary
> > fixmap bindings and headers for generic ioremap support to work.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
>
> Looks reasonable to me,
>
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
>
> although I wonder why it is part of this series?
>
because of CONFIG_EFI. With CONFIG_EFI enabled, all the run time
service memory mapping code is compiled for RISC-V. I could have
createa a separate config for only boot time services or used EFI_STUB
at places where CONFI_EFI. But it seems redundant as we will support
runtime services soon. Let me know if that's a wrong approach.
> > ---
> > arch/riscv/Kconfig | 1 +
> > arch/riscv/include/asm/Kbuild | 1 +
> > arch/riscv/include/asm/fixmap.h | 21 ++++++++++++++++++++-
> > arch/riscv/include/asm/io.h | 1 +
> > 4 files changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 27bfc7947e44..42c122170cfd 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -65,6 +65,7 @@ config RISCV
> > select ARCH_HAS_GCOV_PROFILE_ALL
> > select HAVE_COPY_THREAD_TLS
> > select HAVE_ARCH_KASAN if MMU && 64BIT
> > + select GENERIC_EARLY_IOREMAP
> >
> > config ARCH_MMAP_RND_BITS_MIN
> > default 18 if 64BIT
> > diff --git a/arch/riscv/include/asm/Kbuild
> > b/arch/riscv/include/asm/Kbuild
> > index ec0ca8c6ab64..517394390106 100644
> > --- a/arch/riscv/include/asm/Kbuild
> > +++ b/arch/riscv/include/asm/Kbuild
> > @@ -4,6 +4,7 @@ generic-y += checksum.h
> > generic-y += compat.h
> > generic-y += device.h
> > generic-y += div64.h
> > +generic-y += early_ioremap.h
> > generic-y += extable.h
> > generic-y += flat.h
> > generic-y += dma.h
> > diff --git a/arch/riscv/include/asm/fixmap.h
> > b/arch/riscv/include/asm/fixmap.h
> > index 42d2c42f3cc9..7a4beb7e29a3 100644
> > --- a/arch/riscv/include/asm/fixmap.h
> > +++ b/arch/riscv/include/asm/fixmap.h
> > @@ -25,9 +25,28 @@ enum fixed_addresses {
> > #define FIX_FDT_SIZE SZ_1M
> > FIX_FDT_END,
> > FIX_FDT = FIX_FDT_END + FIX_FDT_SIZE / PAGE_SIZE - 1,
> > + FIX_EARLYCON_MEM_BASE,
> > +
> > FIX_PTE,
> > FIX_PMD,
> > - FIX_EARLYCON_MEM_BASE,
> > + /*
> > + * Make sure that it is 2MB aligned.
> > + */
> > +#define NR_FIX_SZ_2M (SZ_2M / PAGE_SIZE)
> > + FIX_THOLE = NR_FIX_SZ_2M - FIX_PMD - 1,
> > +
> > + __end_of_permanent_fixed_addresses,
> > + /*
> > + * Temporary boot-time mappings, used by early_ioremap(),
> > + * before ioremap() is functional.
> > + */
> > +#define NR_FIX_BTMAPS (SZ_256K / PAGE_SIZE)
> > +#define FIX_BTMAPS_SLOTS 7
> > +#define TOTAL_FIX_BTMAPS (NR_FIX_BTMAPS * FIX_BTMAPS_SLOTS)
> > +
> > + FIX_BTMAP_END = __end_of_permanent_fixed_addresses,
> > + FIX_BTMAP_BEGIN = FIX_BTMAP_END + TOTAL_FIX_BTMAPS - 1,
> > +
> > __end_of_fixed_addresses
> > };
> >
> > diff --git a/arch/riscv/include/asm/io.h
> > b/arch/riscv/include/asm/io.h
> > index 0f477206a4ed..047f414b6948 100644
> > --- a/arch/riscv/include/asm/io.h
> > +++ b/arch/riscv/include/asm/io.h
> > @@ -14,6 +14,7 @@
> > #include <linux/types.h>
> > #include <asm/mmiowb.h>
> > #include <asm/pgtable.h>
> > +#include <asm/early_ioremap.h>
> >
> > /*
> > * MMIO access functions are separated out to break dependency
> > cycles
> > --
> > 2.24.0
> >
--
Regards,
Atish
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [RFC PATCH 3/5] RISC-V: Define fixmap bindings for generic early ioremap support
2020-02-27 19:58 ` Atish Patra
@ 2020-02-27 20:00 ` Ard Biesheuvel
0 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2020-02-27 20:00 UTC (permalink / raw)
To: Atish Patra
Cc: alexios.zavras, tglx, mchehab+samsung, pbonzini, michal.simek,
linux, abner.chang, linux-riscv, catalin.marinas, linux-kernel,
daniel.schaefer, Anup Patel, kstewart, palmer, aou, arnd, rppt,
linux-efi, bp, greentime.hu, keescook, agraf, will, gregkh,
mingo, allison, paul.walmsley, linus.walleij, leif, akpm
On Thu, 27 Feb 2020 at 20:58, Atish Patra <Atish.Patra@wdc.com> wrote:
>
> On Wed, 2020-02-26 at 08:08 +0100, Ard Biesheuvel wrote:
> > On Wed, 26 Feb 2020 at 02:10, Atish Patra <atish.patra@wdc.com>
> > wrote:
> > > UEFI uses early IO or memory mappings for runtime services before
> > > normal ioremap() is usable. This patch only adds minimum necessary
> > > fixmap bindings and headers for generic ioremap support to work.
> > >
> > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> >
> > Looks reasonable to me,
> >
> > Acked-by: Ard Biesheuvel <ardb@kernel.org>
> >
> > although I wonder why it is part of this series?
> >
>
> because of CONFIG_EFI. With CONFIG_EFI enabled, all the run time
> service memory mapping code is compiled for RISC-V. I could have
> createa a separate config for only boot time services or used EFI_STUB
> at places where CONFI_EFI. But it seems redundant as we will support
> runtime services soon. Let me know if that's a wrong approach.
>
No that's fine
> > > ---
> > > arch/riscv/Kconfig | 1 +
> > > arch/riscv/include/asm/Kbuild | 1 +
> > > arch/riscv/include/asm/fixmap.h | 21 ++++++++++++++++++++-
> > > arch/riscv/include/asm/io.h | 1 +
> > > 4 files changed, 23 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index 27bfc7947e44..42c122170cfd 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -65,6 +65,7 @@ config RISCV
> > > select ARCH_HAS_GCOV_PROFILE_ALL
> > > select HAVE_COPY_THREAD_TLS
> > > select HAVE_ARCH_KASAN if MMU && 64BIT
> > > + select GENERIC_EARLY_IOREMAP
> > >
> > > config ARCH_MMAP_RND_BITS_MIN
> > > default 18 if 64BIT
> > > diff --git a/arch/riscv/include/asm/Kbuild
> > > b/arch/riscv/include/asm/Kbuild
> > > index ec0ca8c6ab64..517394390106 100644
> > > --- a/arch/riscv/include/asm/Kbuild
> > > +++ b/arch/riscv/include/asm/Kbuild
> > > @@ -4,6 +4,7 @@ generic-y += checksum.h
> > > generic-y += compat.h
> > > generic-y += device.h
> > > generic-y += div64.h
> > > +generic-y += early_ioremap.h
> > > generic-y += extable.h
> > > generic-y += flat.h
> > > generic-y += dma.h
> > > diff --git a/arch/riscv/include/asm/fixmap.h
> > > b/arch/riscv/include/asm/fixmap.h
> > > index 42d2c42f3cc9..7a4beb7e29a3 100644
> > > --- a/arch/riscv/include/asm/fixmap.h
> > > +++ b/arch/riscv/include/asm/fixmap.h
> > > @@ -25,9 +25,28 @@ enum fixed_addresses {
> > > #define FIX_FDT_SIZE SZ_1M
> > > FIX_FDT_END,
> > > FIX_FDT = FIX_FDT_END + FIX_FDT_SIZE / PAGE_SIZE - 1,
> > > + FIX_EARLYCON_MEM_BASE,
> > > +
> > > FIX_PTE,
> > > FIX_PMD,
> > > - FIX_EARLYCON_MEM_BASE,
> > > + /*
> > > + * Make sure that it is 2MB aligned.
> > > + */
> > > +#define NR_FIX_SZ_2M (SZ_2M / PAGE_SIZE)
> > > + FIX_THOLE = NR_FIX_SZ_2M - FIX_PMD - 1,
> > > +
> > > + __end_of_permanent_fixed_addresses,
> > > + /*
> > > + * Temporary boot-time mappings, used by early_ioremap(),
> > > + * before ioremap() is functional.
> > > + */
> > > +#define NR_FIX_BTMAPS (SZ_256K / PAGE_SIZE)
> > > +#define FIX_BTMAPS_SLOTS 7
> > > +#define TOTAL_FIX_BTMAPS (NR_FIX_BTMAPS * FIX_BTMAPS_SLOTS)
> > > +
> > > + FIX_BTMAP_END = __end_of_permanent_fixed_addresses,
> > > + FIX_BTMAP_BEGIN = FIX_BTMAP_END + TOTAL_FIX_BTMAPS - 1,
> > > +
> > > __end_of_fixed_addresses
> > > };
> > >
> > > diff --git a/arch/riscv/include/asm/io.h
> > > b/arch/riscv/include/asm/io.h
> > > index 0f477206a4ed..047f414b6948 100644
> > > --- a/arch/riscv/include/asm/io.h
> > > +++ b/arch/riscv/include/asm/io.h
> > > @@ -14,6 +14,7 @@
> > > #include <linux/types.h>
> > > #include <asm/mmiowb.h>
> > > #include <asm/pgtable.h>
> > > +#include <asm/early_ioremap.h>
> > >
> > > /*
> > > * MMIO access functions are separated out to break dependency
> > > cycles
> > > --
> > > 2.24.0
> > >
>
> --
> Regards,
> Atish
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 4/5] RISC-V: Add PE/COFF header for EFI stub
2020-02-26 1:10 [RFC PATCH 0/5] Add UEFI support for RISC-V Atish Patra
` (2 preceding siblings ...)
2020-02-26 1:10 ` [RFC PATCH 3/5] RISC-V: Define fixmap bindings for generic early ioremap support Atish Patra
@ 2020-02-26 1:10 ` Atish Patra
2020-02-26 7:14 ` Ard Biesheuvel
2020-02-26 1:10 ` [RFC PATCH 5/5] RISC-V: Add EFI stub support Atish Patra
2020-02-26 23:46 ` [RFC PATCH 0/5] Add UEFI support for RISC-V Palmer Dabbelt
5 siblings, 1 reply; 26+ messages in thread
From: Atish Patra @ 2020-02-26 1:10 UTC (permalink / raw)
To: linux-kernel
Cc: Atish Patra, Albert Ou, Alexios Zavras, Allison Randal,
Andrew Morton, Anup Patel, Ard Biesheuvel, Arnd Bergmann,
Borislav Petkov, Catalin Marinas, Greentime Hu,
Greg Kroah-Hartman, Ingo Molnar, Kate Stewart, Kees Cook,
Linus Walleij, linux-efi, linux-riscv, Mao Han,
Mauro Carvalho Chehab, Michal Simek, Mike Rapoport,
Palmer Dabbelt, Paolo Bonzini, Paul Walmsley, Russell King,
Thomas Gleixner, Will Deacon, Alexander Graf, leif, Chang,
Abner (HPS SW/FW Technologist),
daniel.schaefer
Linux kernel Image can appear as an EFI application With appropriate
PE/COFF header fields in the beginning of the Image header. An EFI
application loader can directly load a Linux kernel Image and an EFI
stub residing in kernel can boot Linux kernel directly.
Add the necessary PE/COFF header.
Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
arch/riscv/include/asm/Kbuild | 1 -
arch/riscv/include/asm/sections.h | 13 ++++
arch/riscv/kernel/Makefile | 4 ++
| 107 ++++++++++++++++++++++++++++++
arch/riscv/kernel/head.S | 15 +++++
arch/riscv/kernel/image-vars.h | 52 +++++++++++++++
arch/riscv/kernel/vmlinux.lds.S | 27 ++++++--
7 files changed, 212 insertions(+), 7 deletions(-)
create mode 100644 arch/riscv/include/asm/sections.h
create mode 100644 arch/riscv/kernel/efi-header.S
create mode 100644 arch/riscv/kernel/image-vars.h
diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
index 517394390106..ef797fe44934 100644
--- a/arch/riscv/include/asm/Kbuild
+++ b/arch/riscv/include/asm/Kbuild
@@ -24,7 +24,6 @@ generic-y += local64.h
generic-y += mm-arch-hooks.h
generic-y += percpu.h
generic-y += preempt.h
-generic-y += sections.h
generic-y += serial.h
generic-y += shmparam.h
generic-y += topology.h
diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h
new file mode 100644
index 000000000000..3a9971b1210f
--- /dev/null
+++ b/arch/riscv/include/asm/sections.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 Western Digital Corporation or its affiliates.
+ */
+#ifndef __ASM_SECTIONS_H
+#define __ASM_SECTIONS_H
+
+#include <asm-generic/sections.h>
+
+extern char _start[];
+extern char _start_kernel[];
+
+#endif /* __ASM_SECTIONS_H */
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index 9601ac907f70..471b1c73f77d 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -29,6 +29,10 @@ obj-y += cacheinfo.o
obj-$(CONFIG_MMU) += vdso.o vdso/
obj-$(CONFIG_RISCV_M_MODE) += clint.o
+OBJCOPYFLAGS := --prefix-symbols=__efistub_
+$(obj)/%.stub.o: $(obj)/%.o FORCE
+ $(call if_changed,objcopy)
+
obj-$(CONFIG_FPU) += fpu.o
obj-$(CONFIG_SMP) += smpboot.o
obj-$(CONFIG_SMP) += smp.o
--git a/arch/riscv/kernel/efi-header.S b/arch/riscv/kernel/efi-header.S
new file mode 100644
index 000000000000..af959e748d93
--- /dev/null
+++ b/arch/riscv/kernel/efi-header.S
@@ -0,0 +1,107 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2019 Western Digital Corporation or its affiliates.
+ * Adapted from arch/arm64/kernel/efi-header.S
+ */
+
+#include <linux/pe.h>
+#include <linux/sizes.h>
+
+ .macro __EFI_PE_HEADER
+ .long PE_MAGIC
+coff_header:
+ .short IMAGE_FILE_MACHINE_RISCV64 // Machine
+ .short section_count // NumberOfSections
+ .long 0 // TimeDateStamp
+ .long 0 // PointerToSymbolTable
+ .long 0 // NumberOfSymbols
+ .short section_table - optional_header // SizeOfOptionalHeader
+ .short IMAGE_FILE_DEBUG_STRIPPED | \
+ IMAGE_FILE_EXECUTABLE_IMAGE | \
+ IMAGE_FILE_LINE_NUMS_STRIPPED // Characteristics
+
+optional_header:
+ .short PE_OPT_MAGIC_PE32PLUS // PE32+ format
+ .byte 0x02 // MajorLinkerVersion
+ .byte 0x14 // MinorLinkerVersion
+ .long __text_end - efi_header_end // SizeOfCode
+ .long _end - __text_end // SizeOfInitializedData
+ .long 0 // SizeOfUninitializedData
+ .long __efistub_efi_entry - _start // AddressOfEntryPoint
+ .long efi_header_end - _start // BaseOfCode
+
+extra_header_fields:
+ .quad 0 // ImageBase
+ .long SZ_4K // SectionAlignment
+ .long PECOFF_FILE_ALIGNMENT // FileAlignment
+ .short 0 // MajorOperatingSystemVersion
+ .short 0 // MinorOperatingSystemVersion
+ .short 0 // MajorImageVersion
+ .short 0 // MinorImageVersion
+ .short 0 // MajorSubsystemVersion
+ .short 0 // MinorSubsystemVersion
+ .long 0 // Win32VersionValue
+
+ .long _end - _start // SizeOfImage
+
+ // Everything before the kernel image is considered part of the header
+ .long efi_header_end - _start // SizeOfHeaders
+ .long 0 // CheckSum
+ .short IMAGE_SUBSYSTEM_EFI_APPLICATION // Subsystem
+ .short 0 // DllCharacteristics
+ .quad 0 // SizeOfStackReserve
+ .quad 0 // SizeOfStackCommit
+ .quad 0 // SizeOfHeapReserve
+ .quad 0 // SizeOfHeapCommit
+ .long 0 // LoaderFlags
+ .long (section_table - .) / 8 // NumberOfRvaAndSizes
+
+ .quad 0 // ExportTable
+ .quad 0 // ImportTable
+ .quad 0 // ResourceTable
+ .quad 0 // ExceptionTable
+ .quad 0 // CertificationTable
+ .quad 0 // BaseRelocationTable
+
+ // Section table
+section_table:
+ .ascii ".text\0\0\0"
+ .long __text_end - efi_header_end // VirtualSize
+ .long efi_header_end - _start // VirtualAddress
+ .long __text_end - efi_header_end // SizeOfRawData
+ .long efi_header_end - _start // PointerToRawData
+
+ .long 0 // PointerToRelocations
+ .long 0 // PointerToLineNumbers
+ .short 0 // NumberOfRelocations
+ .short 0 // NumberOfLineNumbers
+ .long IMAGE_SCN_CNT_CODE | \
+ IMAGE_SCN_MEM_READ | \
+ IMAGE_SCN_MEM_EXECUTE // Characteristics
+
+ .ascii ".data\0\0\0"
+ .long __data_virt_size // VirtualSize
+ .long __text_end - _start // VirtualAddress
+ .long __data_raw_size // SizeOfRawData
+ .long __text_end - _start // PointerToRawData
+
+ .long 0 // PointerToRelocations
+ .long 0 // PointerToLineNumbers
+ .short 0 // NumberOfRelocations
+ .short 0 // NumberOfLineNumbers
+ .long IMAGE_SCN_CNT_INITIALIZED_DATA | \
+ IMAGE_SCN_MEM_READ | \
+ IMAGE_SCN_MEM_WRITE // Characteristics
+
+ .set section_count, (. - section_table) / 40
+
+ /*
+ * EFI will load .text onwards at the 4k section alignment
+ * described in the PE/COFF header. To ensure that instruction
+ * sequences using an adrp and a :lo12: immediate will function
+ * correctly at this alignment, we must ensure that .text is
+ * placed at a 4k boundary in the Image to begin with.
+ */
+ .align 12
+efi_header_end:
+ .endm
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index ac5b0e0a02f6..835dc76de285 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -13,6 +13,7 @@
#include <asm/csr.h>
#include <asm/hwcap.h>
#include <asm/image.h>
+#include "efi-header.S"
__HEAD
ENTRY(_start)
@@ -22,10 +23,17 @@ ENTRY(_start)
* Do not modify it without modifying the structure and all bootloaders
* that expects this header format!!
*/
+#ifdef CONFIG_EFI
+ /*
+ * This instruction decodes to "MZ" ASCII required by UEFI.
+ */
+ li s4,-13
+#else
/* jump to start kernel */
j _start_kernel
/* reserved */
.word 0
+#endif
.balign 8
#if __riscv_xlen == 64
/* Image load offset(2MB) from start of RAM */
@@ -43,7 +51,14 @@ ENTRY(_start)
.ascii RISCV_IMAGE_MAGIC
.balign 4
.ascii RISCV_IMAGE_MAGIC2
+#ifdef CONFIG_EFI
+ .word pe_head_start - _start
+pe_head_start:
+
+ __EFI_PE_HEADER
+#else
.word 0
+#endif
.align 2
#ifdef CONFIG_MMU
diff --git a/arch/riscv/kernel/image-vars.h b/arch/riscv/kernel/image-vars.h
new file mode 100644
index 000000000000..57abb85065e9
--- /dev/null
+++ b/arch/riscv/kernel/image-vars.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Linker script variables to be set after section resolution, as
+ * ld.lld does not like variables assigned before SECTIONS is processed.
+ * Based on arch/arm64/kerne/image-vars.h
+ */
+#ifndef __RISCV_KERNEL_IMAGE_VARS_H
+#define __RISCV_KERNEL_IMAGE_VARS_H
+
+#ifndef LINKER_SCRIPT
+#error This file should only be included in vmlinux.lds.S
+#endif
+
+#ifdef CONFIG_EFI
+
+__efistub_stext_offset = _start_kernel - _start;
+
+/*
+ * The EFI stub has its own symbol namespace prefixed by __efistub_, to
+ * isolate it from the kernel proper. The following symbols are legally
+ * accessed by the stub, so provide some aliases to make them accessible.
+ * Only include data symbols here, or text symbols of functions that are
+ * guaranteed to be safe when executed at another offset than they were
+ * linked at. The routines below are all implemented in assembler in a
+ * position independent manner
+ */
+__efistub_memcmp = memcmp;
+__efistub_memchr = memchr;
+__efistub_memcpy = memcpy;
+__efistub_memmove = memmove;
+__efistub_memset = memset;
+__efistub_strlen = strlen;
+__efistub_strnlen = strnlen;
+__efistub_strcmp = strcmp;
+__efistub_strncmp = strncmp;
+__efistub_strrchr = strrchr;
+
+#ifdef CONFIG_KASAN
+__efistub___memcpy = memcpy;
+__efistub___memmove = memmove;
+__efistub___memset = memset;
+#endif
+
+__efistub__start = _start;
+__efistub__start_kernel = _start_kernel;
+__efistub__end = _end;
+__efistub__edata = _edata;
+__efistub_screen_info = screen_info;
+
+#endif
+
+#endif /* __RISCV_KERNEL_IMAGE_VARS_H */
diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
index b32640300d07..933b9e9a4b39 100644
--- a/arch/riscv/kernel/vmlinux.lds.S
+++ b/arch/riscv/kernel/vmlinux.lds.S
@@ -9,6 +9,7 @@
#include <asm/page.h>
#include <asm/cache.h>
#include <asm/thread_info.h>
+#include "image-vars.h"
#include <linux/sizes.h>
OUTPUT_ARCH(riscv)
@@ -16,6 +17,14 @@ ENTRY(_start)
jiffies = jiffies_64;
+PECOFF_FILE_ALIGNMENT = 0x200;
+#ifdef CONFIG_EFI
+#define PECOFF_EDATA_PADDING \
+ .pecoff_edata_padding : { BYTE(0); . = ALIGN(PECOFF_FILE_ALIGNMENT); }
+#else
+#define PECOFF_EDATA_PADDING
+#endif
+
SECTIONS
{
/* Beginning of code and text segment */
@@ -26,12 +35,15 @@ SECTIONS
__init_begin = .;
INIT_TEXT_SECTION(PAGE_SIZE)
+
+ /* Start of data section */
INIT_DATA_SECTION(16)
/* we have to discard exit text and such at runtime, not link time */
.exit.text :
{
EXIT_TEXT
}
+
.exit.data :
{
EXIT_DATA
@@ -54,7 +66,8 @@ SECTIONS
_etext = .;
}
- /* Start of data section */
+ __text_end = .;
+
_sdata = .;
RO_DATA(L1_CACHE_BYTES)
.srodata : {
@@ -65,19 +78,21 @@ SECTIONS
.sdata : {
__global_pointer$ = . + 0x800;
*(.sdata*)
- /* End of data section */
- _edata = .;
*(.sbss*)
}
-
- BSS_SECTION(PAGE_SIZE, PAGE_SIZE, 0)
-
+ PECOFF_EDATA_PADDING
+ __data_raw_size = ABSOLUTE(. - __text_end);
+ /* End of data section */
+ _edata = .;
EXCEPTION_TABLE(0x10)
.rel.dyn : {
*(.rel.dyn*)
}
+ BSS_SECTION(PAGE_SIZE, PAGE_SIZE, 0)
+ __data_virt_size = ABSOLUTE(. - __text_end);
+
_end = .;
STABS_DEBUG
--
2.24.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 4/5] RISC-V: Add PE/COFF header for EFI stub
2020-02-26 1:10 ` [RFC PATCH 4/5] RISC-V: Add PE/COFF header for EFI stub Atish Patra
@ 2020-02-26 7:14 ` Ard Biesheuvel
2020-02-27 1:29 ` Atish Patra
0 siblings, 1 reply; 26+ messages in thread
From: Ard Biesheuvel @ 2020-02-26 7:14 UTC (permalink / raw)
To: Atish Patra
Cc: Linux Kernel Mailing List, Albert Ou, Alexios Zavras,
Allison Randal, Andrew Morton, Anup Patel, Arnd Bergmann,
Borislav Petkov, Catalin Marinas, Greentime Hu,
Greg Kroah-Hartman, Ingo Molnar, Kate Stewart, Kees Cook,
Linus Walleij, linux-efi, linux-riscv, Mao Han,
Mauro Carvalho Chehab, Michal Simek, Mike Rapoport,
Palmer Dabbelt, Paolo Bonzini, Paul Walmsley, Russell King,
Thomas Gleixner, Will Deacon, Alexander Graf, leif, Chang,
Abner (HPS SW/FW Technologist), Schaefer, Daniel (DualStudy)
On Wed, 26 Feb 2020 at 02:10, Atish Patra <atish.patra@wdc.com> wrote:
>
> Linux kernel Image can appear as an EFI application With appropriate
> PE/COFF header fields in the beginning of the Image header. An EFI
> application loader can directly load a Linux kernel Image and an EFI
> stub residing in kernel can boot Linux kernel directly.
>
> Add the necessary PE/COFF header.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
> arch/riscv/include/asm/Kbuild | 1 -
> arch/riscv/include/asm/sections.h | 13 ++++
> arch/riscv/kernel/Makefile | 4 ++
> arch/riscv/kernel/efi-header.S | 107 ++++++++++++++++++++++++++++++
> arch/riscv/kernel/head.S | 15 +++++
> arch/riscv/kernel/image-vars.h | 52 +++++++++++++++
> arch/riscv/kernel/vmlinux.lds.S | 27 ++++++--
> 7 files changed, 212 insertions(+), 7 deletions(-)
> create mode 100644 arch/riscv/include/asm/sections.h
> create mode 100644 arch/riscv/kernel/efi-header.S
> create mode 100644 arch/riscv/kernel/image-vars.h
>
> diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
> index 517394390106..ef797fe44934 100644
> --- a/arch/riscv/include/asm/Kbuild
> +++ b/arch/riscv/include/asm/Kbuild
> @@ -24,7 +24,6 @@ generic-y += local64.h
> generic-y += mm-arch-hooks.h
> generic-y += percpu.h
> generic-y += preempt.h
> -generic-y += sections.h
> generic-y += serial.h
> generic-y += shmparam.h
> generic-y += topology.h
> diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h
> new file mode 100644
> index 000000000000..3a9971b1210f
> --- /dev/null
> +++ b/arch/riscv/include/asm/sections.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 Western Digital Corporation or its affiliates.
> + */
> +#ifndef __ASM_SECTIONS_H
> +#define __ASM_SECTIONS_H
> +
> +#include <asm-generic/sections.h>
> +
> +extern char _start[];
> +extern char _start_kernel[];
> +
> +#endif /* __ASM_SECTIONS_H */
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index 9601ac907f70..471b1c73f77d 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -29,6 +29,10 @@ obj-y += cacheinfo.o
> obj-$(CONFIG_MMU) += vdso.o vdso/
>
> obj-$(CONFIG_RISCV_M_MODE) += clint.o
> +OBJCOPYFLAGS := --prefix-symbols=__efistub_
> +$(obj)/%.stub.o: $(obj)/%.o FORCE
> + $(call if_changed,objcopy)
> +
> obj-$(CONFIG_FPU) += fpu.o
> obj-$(CONFIG_SMP) += smpboot.o
> obj-$(CONFIG_SMP) += smp.o
> diff --git a/arch/riscv/kernel/efi-header.S b/arch/riscv/kernel/efi-header.S
> new file mode 100644
> index 000000000000..af959e748d93
> --- /dev/null
> +++ b/arch/riscv/kernel/efi-header.S
> @@ -0,0 +1,107 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2019 Western Digital Corporation or its affiliates.
> + * Adapted from arch/arm64/kernel/efi-header.S
> + */
> +
> +#include <linux/pe.h>
> +#include <linux/sizes.h>
> +
> + .macro __EFI_PE_HEADER
> + .long PE_MAGIC
> +coff_header:
> + .short IMAGE_FILE_MACHINE_RISCV64 // Machine
> + .short section_count // NumberOfSections
> + .long 0 // TimeDateStamp
> + .long 0 // PointerToSymbolTable
> + .long 0 // NumberOfSymbols
> + .short section_table - optional_header // SizeOfOptionalHeader
> + .short IMAGE_FILE_DEBUG_STRIPPED | \
> + IMAGE_FILE_EXECUTABLE_IMAGE | \
> + IMAGE_FILE_LINE_NUMS_STRIPPED // Characteristics
> +
> +optional_header:
> + .short PE_OPT_MAGIC_PE32PLUS // PE32+ format
> + .byte 0x02 // MajorLinkerVersion
> + .byte 0x14 // MinorLinkerVersion
> + .long __text_end - efi_header_end // SizeOfCode
> + .long _end - __text_end // SizeOfInitializedData
> + .long 0 // SizeOfUninitializedData
> + .long __efistub_efi_entry - _start // AddressOfEntryPoint
> + .long efi_header_end - _start // BaseOfCode
> +
> +extra_header_fields:
> + .quad 0 // ImageBase
> + .long SZ_4K // SectionAlignment
> + .long PECOFF_FILE_ALIGNMENT // FileAlignment
> + .short 0 // MajorOperatingSystemVersion
> + .short 0 // MinorOperatingSystemVersion
> + .short 0 // MajorImageVersion
> + .short 0 // MinorImageVersion
Put LINUX_EFISTUB_MAJOR_VERSION and LINUX_EFISTUB_MINOR_VERSION here
> + .short 0 // MajorSubsystemVersion
> + .short 0 // MinorSubsystemVersion
> + .long 0 // Win32VersionValue
> +
> + .long _end - _start // SizeOfImage
> +
> + // Everything before the kernel image is considered part of the header
> + .long efi_header_end - _start // SizeOfHeaders
> + .long 0 // CheckSum
> + .short IMAGE_SUBSYSTEM_EFI_APPLICATION // Subsystem
> + .short 0 // DllCharacteristics
> + .quad 0 // SizeOfStackReserve
> + .quad 0 // SizeOfStackCommit
> + .quad 0 // SizeOfHeapReserve
> + .quad 0 // SizeOfHeapCommit
> + .long 0 // LoaderFlags
> + .long (section_table - .) / 8 // NumberOfRvaAndSizes
> +
> + .quad 0 // ExportTable
> + .quad 0 // ImportTable
> + .quad 0 // ResourceTable
> + .quad 0 // ExceptionTable
> + .quad 0 // CertificationTable
> + .quad 0 // BaseRelocationTable
> +
> + // Section table
> +section_table:
> + .ascii ".text\0\0\0"
> + .long __text_end - efi_header_end // VirtualSize
> + .long efi_header_end - _start // VirtualAddress
> + .long __text_end - efi_header_end // SizeOfRawData
> + .long efi_header_end - _start // PointerToRawData
> +
> + .long 0 // PointerToRelocations
> + .long 0 // PointerToLineNumbers
> + .short 0 // NumberOfRelocations
> + .short 0 // NumberOfLineNumbers
> + .long IMAGE_SCN_CNT_CODE | \
> + IMAGE_SCN_MEM_READ | \
> + IMAGE_SCN_MEM_EXECUTE // Characteristics
> +
> + .ascii ".data\0\0\0"
> + .long __data_virt_size // VirtualSize
> + .long __text_end - _start // VirtualAddress
> + .long __data_raw_size // SizeOfRawData
> + .long __text_end - _start // PointerToRawData
> +
> + .long 0 // PointerToRelocations
> + .long 0 // PointerToLineNumbers
> + .short 0 // NumberOfRelocations
> + .short 0 // NumberOfLineNumbers
> + .long IMAGE_SCN_CNT_INITIALIZED_DATA | \
> + IMAGE_SCN_MEM_READ | \
> + IMAGE_SCN_MEM_WRITE // Characteristics
> +
> + .set section_count, (. - section_table) / 40
> +
You dropped the debug header here, which is actually *very* useful if
you want to single step through the stub from DEBUG edk2 firmware.
> + /*
> + * EFI will load .text onwards at the 4k section alignment
> + * described in the PE/COFF header. To ensure that instruction
> + * sequences using an adrp and a :lo12: immediate will function
Surely, this is inaccurate for RISC-V?
> + * correctly at this alignment, we must ensure that .text is
> + * placed at a 4k boundary in the Image to begin with.
> + */
> + .align 12
> +efi_header_end:
> + .endm
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index ac5b0e0a02f6..835dc76de285 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -13,6 +13,7 @@
> #include <asm/csr.h>
> #include <asm/hwcap.h>
> #include <asm/image.h>
> +#include "efi-header.S"
>
> __HEAD
> ENTRY(_start)
> @@ -22,10 +23,17 @@ ENTRY(_start)
> * Do not modify it without modifying the structure and all bootloaders
> * that expects this header format!!
> */
> +#ifdef CONFIG_EFI
> + /*
> + * This instruction decodes to "MZ" ASCII required by UEFI.
> + */
> + li s4,-13
What happens if you try to do plain boot on an EFI kernel? On ARM and
x86, we took care to use a 'MZ' opcode that behaves as a pseudo-NOP,
and jump to start_kernel right after, so if you boot the EFI kernel as
a normal kernel, it still works.
> +#else
> /* jump to start kernel */
> j _start_kernel
> /* reserved */
> .word 0
> +#endif
> .balign 8
> #if __riscv_xlen == 64
> /* Image load offset(2MB) from start of RAM */
> @@ -43,7 +51,14 @@ ENTRY(_start)
> .ascii RISCV_IMAGE_MAGIC
> .balign 4
> .ascii RISCV_IMAGE_MAGIC2
> +#ifdef CONFIG_EFI
> + .word pe_head_start - _start
> +pe_head_start:
> +
> + __EFI_PE_HEADER
> +#else
> .word 0
> +#endif
>
> .align 2
> #ifdef CONFIG_MMU
> diff --git a/arch/riscv/kernel/image-vars.h b/arch/riscv/kernel/image-vars.h
> new file mode 100644
> index 000000000000..57abb85065e9
> --- /dev/null
> +++ b/arch/riscv/kernel/image-vars.h
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Linker script variables to be set after section resolution, as
> + * ld.lld does not like variables assigned before SECTIONS is processed.
> + * Based on arch/arm64/kerne/image-vars.h
> + */
> +#ifndef __RISCV_KERNEL_IMAGE_VARS_H
> +#define __RISCV_KERNEL_IMAGE_VARS_H
> +
> +#ifndef LINKER_SCRIPT
> +#error This file should only be included in vmlinux.lds.S
> +#endif
> +
> +#ifdef CONFIG_EFI
> +
> +__efistub_stext_offset = _start_kernel - _start;
> +
> +/*
> + * The EFI stub has its own symbol namespace prefixed by __efistub_, to
> + * isolate it from the kernel proper. The following symbols are legally
> + * accessed by the stub, so provide some aliases to make them accessible.
> + * Only include data symbols here, or text symbols of functions that are
> + * guaranteed to be safe when executed at another offset than they were
> + * linked at. The routines below are all implemented in assembler in a
> + * position independent manner
> + */
> +__efistub_memcmp = memcmp;
> +__efistub_memchr = memchr;
> +__efistub_memcpy = memcpy;
> +__efistub_memmove = memmove;
> +__efistub_memset = memset;
> +__efistub_strlen = strlen;
> +__efistub_strnlen = strnlen;
> +__efistub_strcmp = strcmp;
> +__efistub_strncmp = strncmp;
> +__efistub_strrchr = strrchr;
> +
> +#ifdef CONFIG_KASAN
> +__efistub___memcpy = memcpy;
> +__efistub___memmove = memmove;
> +__efistub___memset = memset;
> +#endif
> +
> +__efistub__start = _start;
> +__efistub__start_kernel = _start_kernel;
> +__efistub__end = _end;
> +__efistub__edata = _edata;
> +__efistub_screen_info = screen_info;
> +
> +#endif
> +
> +#endif /* __RISCV_KERNEL_IMAGE_VARS_H */
> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> index b32640300d07..933b9e9a4b39 100644
> --- a/arch/riscv/kernel/vmlinux.lds.S
> +++ b/arch/riscv/kernel/vmlinux.lds.S
> @@ -9,6 +9,7 @@
> #include <asm/page.h>
> #include <asm/cache.h>
> #include <asm/thread_info.h>
> +#include "image-vars.h"
>
> #include <linux/sizes.h>
> OUTPUT_ARCH(riscv)
> @@ -16,6 +17,14 @@ ENTRY(_start)
>
> jiffies = jiffies_64;
>
> +PECOFF_FILE_ALIGNMENT = 0x200;
> +#ifdef CONFIG_EFI
> +#define PECOFF_EDATA_PADDING \
> + .pecoff_edata_padding : { BYTE(0); . = ALIGN(PECOFF_FILE_ALIGNMENT); }
> +#else
> +#define PECOFF_EDATA_PADDING
> +#endif
> +
> SECTIONS
> {
> /* Beginning of code and text segment */
> @@ -26,12 +35,15 @@ SECTIONS
>
> __init_begin = .;
> INIT_TEXT_SECTION(PAGE_SIZE)
> +
> + /* Start of data section */
> INIT_DATA_SECTION(16)
> /* we have to discard exit text and such at runtime, not link time */
> .exit.text :
> {
> EXIT_TEXT
> }
> +
> .exit.data :
> {
> EXIT_DATA
> @@ -54,7 +66,8 @@ SECTIONS
> _etext = .;
> }
>
> - /* Start of data section */
> + __text_end = .;
> +
> _sdata = .;
> RO_DATA(L1_CACHE_BYTES)
> .srodata : {
> @@ -65,19 +78,21 @@ SECTIONS
> .sdata : {
> __global_pointer$ = . + 0x800;
> *(.sdata*)
> - /* End of data section */
> - _edata = .;
> *(.sbss*)
> }
> -
> - BSS_SECTION(PAGE_SIZE, PAGE_SIZE, 0)
> -
> + PECOFF_EDATA_PADDING
> + __data_raw_size = ABSOLUTE(. - __text_end);
> + /* End of data section */
> + _edata = .;
> EXCEPTION_TABLE(0x10)
>
> .rel.dyn : {
> *(.rel.dyn*)
> }
>
> + BSS_SECTION(PAGE_SIZE, PAGE_SIZE, 0)
> + __data_virt_size = ABSOLUTE(. - __text_end);
> +
> _end = .;
>
> STABS_DEBUG
> --
> 2.24.0
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [RFC PATCH 4/5] RISC-V: Add PE/COFF header for EFI stub
2020-02-26 7:14 ` Ard Biesheuvel
@ 2020-02-27 1:29 ` Atish Patra
2020-02-27 7:44 ` Ard Biesheuvel
0 siblings, 1 reply; 26+ messages in thread
From: Atish Patra @ 2020-02-27 1:29 UTC (permalink / raw)
To: ardb
Cc: alexios.zavras, tglx, mchehab+samsung, pbonzini, michal.simek,
linux, abner.chang, linux-riscv, catalin.marinas, linux-kernel,
daniel.schaefer, Anup Patel, kstewart, palmer, aou, arnd, rppt,
linux-efi, bp, greentime.hu, keescook, agraf, will, gregkh,
mingo, allison, han_mao, paul.walmsley, linus.walleij, leif,
akpm
On Wed, 2020-02-26 at 08:14 +0100, Ard Biesheuvel wrote:
> On Wed, 26 Feb 2020 at 02:10, Atish Patra <atish.patra@wdc.com>
> wrote:
> > Linux kernel Image can appear as an EFI application With
> > appropriate
> > PE/COFF header fields in the beginning of the Image header. An EFI
> > application loader can directly load a Linux kernel Image and an
> > EFI
> > stub residing in kernel can boot Linux kernel directly.
> >
> > Add the necessary PE/COFF header.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > ---
> > arch/riscv/include/asm/Kbuild | 1 -
> > arch/riscv/include/asm/sections.h | 13 ++++
> > arch/riscv/kernel/Makefile | 4 ++
> > arch/riscv/kernel/efi-header.S | 107
> > ++++++++++++++++++++++++++++++
> > arch/riscv/kernel/head.S | 15 +++++
> > arch/riscv/kernel/image-vars.h | 52 +++++++++++++++
> > arch/riscv/kernel/vmlinux.lds.S | 27 ++++++--
> > 7 files changed, 212 insertions(+), 7 deletions(-)
> > create mode 100644 arch/riscv/include/asm/sections.h
> > create mode 100644 arch/riscv/kernel/efi-header.S
> > create mode 100644 arch/riscv/kernel/image-vars.h
> >
> > diff --git a/arch/riscv/include/asm/Kbuild
> > b/arch/riscv/include/asm/Kbuild
> > index 517394390106..ef797fe44934 100644
> > --- a/arch/riscv/include/asm/Kbuild
> > +++ b/arch/riscv/include/asm/Kbuild
> > @@ -24,7 +24,6 @@ generic-y += local64.h
> > generic-y += mm-arch-hooks.h
> > generic-y += percpu.h
> > generic-y += preempt.h
> > -generic-y += sections.h
> > generic-y += serial.h
> > generic-y += shmparam.h
> > generic-y += topology.h
> > diff --git a/arch/riscv/include/asm/sections.h
> > b/arch/riscv/include/asm/sections.h
> > new file mode 100644
> > index 000000000000..3a9971b1210f
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/sections.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2020 Western Digital Corporation or its
> > affiliates.
> > + */
> > +#ifndef __ASM_SECTIONS_H
> > +#define __ASM_SECTIONS_H
> > +
> > +#include <asm-generic/sections.h>
> > +
> > +extern char _start[];
> > +extern char _start_kernel[];
> > +
> > +#endif /* __ASM_SECTIONS_H */
> > diff --git a/arch/riscv/kernel/Makefile
> > b/arch/riscv/kernel/Makefile
> > index 9601ac907f70..471b1c73f77d 100644
> > --- a/arch/riscv/kernel/Makefile
> > +++ b/arch/riscv/kernel/Makefile
> > @@ -29,6 +29,10 @@ obj-y += cacheinfo.o
> > obj-$(CONFIG_MMU) += vdso.o vdso/
> >
> > obj-$(CONFIG_RISCV_M_MODE) += clint.o
> > +OBJCOPYFLAGS := --prefix-symbols=__efistub_
> > +$(obj)/%.stub.o: $(obj)/%.o FORCE
> > + $(call if_changed,objcopy)
> > +
> > obj-$(CONFIG_FPU) += fpu.o
> > obj-$(CONFIG_SMP) += smpboot.o
> > obj-$(CONFIG_SMP) += smp.o
> > diff --git a/arch/riscv/kernel/efi-header.S
> > b/arch/riscv/kernel/efi-header.S
> > new file mode 100644
> > index 000000000000..af959e748d93
> > --- /dev/null
> > +++ b/arch/riscv/kernel/efi-header.S
> > @@ -0,0 +1,107 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2019 Western Digital Corporation or its
> > affiliates.
> > + * Adapted from arch/arm64/kernel/efi-header.S
> > + */
> > +
> > +#include <linux/pe.h>
> > +#include <linux/sizes.h>
> > +
> > + .macro __EFI_PE_HEADER
> > + .long PE_MAGIC
> > +coff_header:
> > + .short IMAGE_FILE_MACHINE_RISCV64 // Machine
> > + .short section_count //
> > NumberOfSections
> > + .long 0 //
> > TimeDateStamp
> > + .long 0 //
> > PointerToSymbolTable
> > + .long 0 //
> > NumberOfSymbols
> > + .short section_table - optional_header //
> > SizeOfOptionalHeader
> > + .short IMAGE_FILE_DEBUG_STRIPPED | \
> > + IMAGE_FILE_EXECUTABLE_IMAGE | \
> > + IMAGE_FILE_LINE_NUMS_STRIPPED //
> > Characteristics
> > +
> > +optional_header:
> > + .short PE_OPT_MAGIC_PE32PLUS // PE32+
> > format
> > + .byte 0x02 //
> > MajorLinkerVersion
> > + .byte 0x14 //
> > MinorLinkerVersion
> > + .long __text_end - efi_header_end //
> > SizeOfCode
> > + .long _end - __text_end //
> > SizeOfInitializedData
> > + .long 0 //
> > SizeOfUninitializedData
> > + .long __efistub_efi_entry - _start //
> > AddressOfEntryPoint
> > + .long efi_header_end - _start //
> > BaseOfCode
> > +
> > +extra_header_fields:
> > + .quad 0 //
> > ImageBase
> > + .long SZ_4K //
> > SectionAlignment
> > + .long PECOFF_FILE_ALIGNMENT //
> > FileAlignment
> > + .short 0 //
> > MajorOperatingSystemVersion
> > + .short 0 //
> > MinorOperatingSystemVersion
> > + .short 0 //
> > MajorImageVersion
> > + .short 0 //
> > MinorImageVersion
>
> Put LINUX_EFISTUB_MAJOR_VERSION and LINUX_EFISTUB_MINOR_VERSION here
>
Sure.
> > + .short 0 //
> > MajorSubsystemVersion
> > + .short 0 //
> > MinorSubsystemVersion
> > + .long 0 //
> > Win32VersionValue
> > +
> > + .long _end - _start //
> > SizeOfImage
> > +
> > + // Everything before the kernel image is considered part of
> > the header
> > + .long efi_header_end - _start //
> > SizeOfHeaders
> > + .long 0 // CheckSum
> > + .short IMAGE_SUBSYSTEM_EFI_APPLICATION //
> > Subsystem
> > + .short 0 //
> > DllCharacteristics
> > + .quad 0 //
> > SizeOfStackReserve
> > + .quad 0 //
> > SizeOfStackCommit
> > + .quad 0 //
> > SizeOfHeapReserve
> > + .quad 0 //
> > SizeOfHeapCommit
> > + .long 0 //
> > LoaderFlags
> > + .long (section_table - .) / 8 //
> > NumberOfRvaAndSizes
> > +
> > + .quad 0 //
> > ExportTable
> > + .quad 0 //
> > ImportTable
> > + .quad 0 //
> > ResourceTable
> > + .quad 0 //
> > ExceptionTable
> > + .quad 0 //
> > CertificationTable
> > + .quad 0 //
> > BaseRelocationTable
> > +
> > + // Section table
> > +section_table:
> > + .ascii ".text\0\0\0"
> > + .long __text_end - efi_header_end //
> > VirtualSize
> > + .long efi_header_end - _start //
> > VirtualAddress
> > + .long __text_end - efi_header_end //
> > SizeOfRawData
> > + .long efi_header_end - _start //
> > PointerToRawData
> > +
> > + .long 0 //
> > PointerToRelocations
> > + .long 0 //
> > PointerToLineNumbers
> > + .short 0 //
> > NumberOfRelocations
> > + .short 0 //
> > NumberOfLineNumbers
> > + .long IMAGE_SCN_CNT_CODE | \
> > + IMAGE_SCN_MEM_READ | \
> > + IMAGE_SCN_MEM_EXECUTE //
> > Characteristics
> > +
> > + .ascii ".data\0\0\0"
> > + .long __data_virt_size //
> > VirtualSize
> > + .long __text_end - _start //
> > VirtualAddress
> > + .long __data_raw_size //
> > SizeOfRawData
> > + .long __text_end - _start //
> > PointerToRawData
> > +
> > + .long 0 //
> > PointerToRelocations
> > + .long 0 //
> > PointerToLineNumbers
> > + .short 0 //
> > NumberOfRelocations
> > + .short 0 //
> > NumberOfLineNumbers
> > + .long IMAGE_SCN_CNT_INITIALIZED_DATA | \
> > + IMAGE_SCN_MEM_READ | \
> > + IMAGE_SCN_MEM_WRITE //
> > Characteristics
> > +
> > + .set section_count, (. - section_table) / 40
> > +
>
> You dropped the debug header here, which is actually *very* useful if
> you want to single step through the stub from DEBUG edk2 firmware.
>
Ahh I see. I was not sure how to use the debug feature :). Can we do
the same in U-boot ?
How about adding it back later once I can verify it with EDK2 ?
> > + /*
> > + * EFI will load .text onwards at the 4k section alignment
> > + * described in the PE/COFF header. To ensure that
> > instruction
> > + * sequences using an adrp and a :lo12: immediate will
> > function
>
> Surely, this is inaccurate for RISC-V?
Sorry. I should have removed the comment. We keep the the _start and
.head.text section aligned to a PAGE_SIZE anyways using the linker
script.
>
> > + * correctly at this alignment, we must ensure that .text
> > is
> > + * placed at a 4k boundary in the Image to begin with.
> > + */
> > + .align 12
> > +efi_header_end:
> > + .endm
> > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > index ac5b0e0a02f6..835dc76de285 100644
> > --- a/arch/riscv/kernel/head.S
> > +++ b/arch/riscv/kernel/head.S
> > @@ -13,6 +13,7 @@
> > #include <asm/csr.h>
> > #include <asm/hwcap.h>
> > #include <asm/image.h>
> > +#include "efi-header.S"
> >
> > __HEAD
> > ENTRY(_start)
> > @@ -22,10 +23,17 @@ ENTRY(_start)
> > * Do not modify it without modifying the structure and all
> > bootloaders
> > * that expects this header format!!
> > */
> > +#ifdef CONFIG_EFI
> > + /*
> > + * This instruction decodes to "MZ" ASCII required by UEFI.
> > + */
> > + li s4,-13
>
> What happens if you try to do plain boot on an EFI kernel? On ARM and
> x86, we took care to use a 'MZ' opcode that behaves as a pseudo-NOP,
> and jump to start_kernel right after, so if you boot the EFI kernel
> as
> a normal kernel, it still works.
>
There should have been a "j _start_kernel" after the "MZ" ascii. I just
tested EFI kernel can now boot as a normal kernel as well with that
change. Thanks for pointing that out.
> > +#else
> > /* jump to start kernel */
> > j _start_kernel
> > /* reserved */
> > .word 0
> > +#endif
> > .balign 8
> > #if __riscv_xlen == 64
> > /* Image load offset(2MB) from start of RAM */
> > @@ -43,7 +51,14 @@ ENTRY(_start)
> > .ascii RISCV_IMAGE_MAGIC
> > .balign 4
> > .ascii RISCV_IMAGE_MAGIC2
> > +#ifdef CONFIG_EFI
> > + .word pe_head_start - _start
> > +pe_head_start:
> > +
> > + __EFI_PE_HEADER
> > +#else
> > .word 0
> > +#endif
> >
> > .align 2
> > #ifdef CONFIG_MMU
> > diff --git a/arch/riscv/kernel/image-vars.h
> > b/arch/riscv/kernel/image-vars.h
> > new file mode 100644
> > index 000000000000..57abb85065e9
> > --- /dev/null
> > +++ b/arch/riscv/kernel/image-vars.h
> > @@ -0,0 +1,52 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Linker script variables to be set after section resolution, as
> > + * ld.lld does not like variables assigned before SECTIONS is
> > processed.
> > + * Based on arch/arm64/kerne/image-vars.h
> > + */
> > +#ifndef __RISCV_KERNEL_IMAGE_VARS_H
> > +#define __RISCV_KERNEL_IMAGE_VARS_H
> > +
> > +#ifndef LINKER_SCRIPT
> > +#error This file should only be included in vmlinux.lds.S
> > +#endif
> > +
> > +#ifdef CONFIG_EFI
> > +
> > +__efistub_stext_offset = _start_kernel - _start;
> > +
> > +/*
> > + * The EFI stub has its own symbol namespace prefixed by
> > __efistub_, to
> > + * isolate it from the kernel proper. The following symbols are
> > legally
> > + * accessed by the stub, so provide some aliases to make them
> > accessible.
> > + * Only include data symbols here, or text symbols of functions
> > that are
> > + * guaranteed to be safe when executed at another offset than they
> > were
> > + * linked at. The routines below are all implemented in assembler
> > in a
> > + * position independent manner
> > + */
> > +__efistub_memcmp = memcmp;
> > +__efistub_memchr = memchr;
> > +__efistub_memcpy = memcpy;
> > +__efistub_memmove = memmove;
> > +__efistub_memset = memset;
> > +__efistub_strlen = strlen;
> > +__efistub_strnlen = strnlen;
> > +__efistub_strcmp = strcmp;
> > +__efistub_strncmp = strncmp;
> > +__efistub_strrchr = strrchr;
> > +
> > +#ifdef CONFIG_KASAN
> > +__efistub___memcpy = memcpy;
> > +__efistub___memmove = memmove;
> > +__efistub___memset = memset;
> > +#endif
> > +
> > +__efistub__start = _start;
> > +__efistub__start_kernel = _start_kernel;
> > +__efistub__end = _end;
> > +__efistub__edata = _edata;
> > +__efistub_screen_info = screen_info;
> > +
> > +#endif
> > +
> > +#endif /* __RISCV_KERNEL_IMAGE_VARS_H */
> > diff --git a/arch/riscv/kernel/vmlinux.lds.S
> > b/arch/riscv/kernel/vmlinux.lds.S
> > index b32640300d07..933b9e9a4b39 100644
> > --- a/arch/riscv/kernel/vmlinux.lds.S
> > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > @@ -9,6 +9,7 @@
> > #include <asm/page.h>
> > #include <asm/cache.h>
> > #include <asm/thread_info.h>
> > +#include "image-vars.h"
> >
> > #include <linux/sizes.h>
> > OUTPUT_ARCH(riscv)
> > @@ -16,6 +17,14 @@ ENTRY(_start)
> >
> > jiffies = jiffies_64;
> >
> > +PECOFF_FILE_ALIGNMENT = 0x200;
> > +#ifdef CONFIG_EFI
> > +#define PECOFF_EDATA_PADDING \
> > + .pecoff_edata_padding : { BYTE(0); . =
> > ALIGN(PECOFF_FILE_ALIGNMENT); }
> > +#else
> > +#define PECOFF_EDATA_PADDING
> > +#endif
> > +
> > SECTIONS
> > {
> > /* Beginning of code and text segment */
> > @@ -26,12 +35,15 @@ SECTIONS
> >
> > __init_begin = .;
> > INIT_TEXT_SECTION(PAGE_SIZE)
> > +
> > + /* Start of data section */
> > INIT_DATA_SECTION(16)
> > /* we have to discard exit text and such at runtime, not
> > link time */
> > .exit.text :
> > {
> > EXIT_TEXT
> > }
> > +
> > .exit.data :
> > {
> > EXIT_DATA
> > @@ -54,7 +66,8 @@ SECTIONS
> > _etext = .;
> > }
> >
> > - /* Start of data section */
> > + __text_end = .;
> > +
> > _sdata = .;
> > RO_DATA(L1_CACHE_BYTES)
> > .srodata : {
> > @@ -65,19 +78,21 @@ SECTIONS
> > .sdata : {
> > __global_pointer$ = . + 0x800;
> > *(.sdata*)
> > - /* End of data section */
> > - _edata = .;
> > *(.sbss*)
> > }
> > -
> > - BSS_SECTION(PAGE_SIZE, PAGE_SIZE, 0)
> > -
> > + PECOFF_EDATA_PADDING
> > + __data_raw_size = ABSOLUTE(. - __text_end);
> > + /* End of data section */
> > + _edata = .;
> > EXCEPTION_TABLE(0x10)
> >
> > .rel.dyn : {
> > *(.rel.dyn*)
> > }
> >
> > + BSS_SECTION(PAGE_SIZE, PAGE_SIZE, 0)
> > + __data_virt_size = ABSOLUTE(. - __text_end);
> > +
> > _end = .;
> >
> > STABS_DEBUG
> > --
> > 2.24.0
> >
--
Regards,
Atish
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [RFC PATCH 4/5] RISC-V: Add PE/COFF header for EFI stub
2020-02-27 1:29 ` Atish Patra
@ 2020-02-27 7:44 ` Ard Biesheuvel
0 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2020-02-27 7:44 UTC (permalink / raw)
To: Atish Patra
Cc: alexios.zavras, tglx, mchehab+samsung, pbonzini, michal.simek,
linux, abner.chang, linux-riscv, catalin.marinas, linux-kernel,
daniel.schaefer, Anup Patel, kstewart, palmer, aou, arnd, rppt,
linux-efi, bp, greentime.hu, keescook, agraf, will, gregkh,
mingo, allison, han_mao, paul.walmsley, linus.walleij, leif,
akpm
On Thu, 27 Feb 2020 at 02:29, Atish Patra <Atish.Patra@wdc.com> wrote:
>
> On Wed, 2020-02-26 at 08:14 +0100, Ard Biesheuvel wrote:
> > On Wed, 26 Feb 2020 at 02:10, Atish Patra <atish.patra@wdc.com>
> > wrote:
> > > Linux kernel Image can appear as an EFI application With
> > > appropriate
> > > PE/COFF header fields in the beginning of the Image header. An EFI
> > > application loader can directly load a Linux kernel Image and an
> > > EFI
> > > stub residing in kernel can boot Linux kernel directly.
> > >
> > > Add the necessary PE/COFF header.
> > >
> > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > ---
> > > arch/riscv/include/asm/Kbuild | 1 -
> > > arch/riscv/include/asm/sections.h | 13 ++++
> > > arch/riscv/kernel/Makefile | 4 ++
> > > arch/riscv/kernel/efi-header.S | 107
> > > ++++++++++++++++++++++++++++++
> > > arch/riscv/kernel/head.S | 15 +++++
> > > arch/riscv/kernel/image-vars.h | 52 +++++++++++++++
> > > arch/riscv/kernel/vmlinux.lds.S | 27 ++++++--
> > > 7 files changed, 212 insertions(+), 7 deletions(-)
> > > create mode 100644 arch/riscv/include/asm/sections.h
> > > create mode 100644 arch/riscv/kernel/efi-header.S
> > > create mode 100644 arch/riscv/kernel/image-vars.h
> > >
> > > diff --git a/arch/riscv/include/asm/Kbuild
> > > b/arch/riscv/include/asm/Kbuild
> > > index 517394390106..ef797fe44934 100644
> > > --- a/arch/riscv/include/asm/Kbuild
> > > +++ b/arch/riscv/include/asm/Kbuild
> > > @@ -24,7 +24,6 @@ generic-y += local64.h
> > > generic-y += mm-arch-hooks.h
> > > generic-y += percpu.h
> > > generic-y += preempt.h
> > > -generic-y += sections.h
> > > generic-y += serial.h
> > > generic-y += shmparam.h
> > > generic-y += topology.h
> > > diff --git a/arch/riscv/include/asm/sections.h
> > > b/arch/riscv/include/asm/sections.h
> > > new file mode 100644
> > > index 000000000000..3a9971b1210f
> > > --- /dev/null
> > > +++ b/arch/riscv/include/asm/sections.h
> > > @@ -0,0 +1,13 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * Copyright (C) 2020 Western Digital Corporation or its
> > > affiliates.
> > > + */
> > > +#ifndef __ASM_SECTIONS_H
> > > +#define __ASM_SECTIONS_H
> > > +
> > > +#include <asm-generic/sections.h>
> > > +
> > > +extern char _start[];
> > > +extern char _start_kernel[];
> > > +
> > > +#endif /* __ASM_SECTIONS_H */
> > > diff --git a/arch/riscv/kernel/Makefile
> > > b/arch/riscv/kernel/Makefile
> > > index 9601ac907f70..471b1c73f77d 100644
> > > --- a/arch/riscv/kernel/Makefile
> > > +++ b/arch/riscv/kernel/Makefile
> > > @@ -29,6 +29,10 @@ obj-y += cacheinfo.o
> > > obj-$(CONFIG_MMU) += vdso.o vdso/
> > >
> > > obj-$(CONFIG_RISCV_M_MODE) += clint.o
> > > +OBJCOPYFLAGS := --prefix-symbols=__efistub_
> > > +$(obj)/%.stub.o: $(obj)/%.o FORCE
> > > + $(call if_changed,objcopy)
> > > +
> > > obj-$(CONFIG_FPU) += fpu.o
> > > obj-$(CONFIG_SMP) += smpboot.o
> > > obj-$(CONFIG_SMP) += smp.o
> > > diff --git a/arch/riscv/kernel/efi-header.S
> > > b/arch/riscv/kernel/efi-header.S
> > > new file mode 100644
> > > index 000000000000..af959e748d93
> > > --- /dev/null
> > > +++ b/arch/riscv/kernel/efi-header.S
> > > @@ -0,0 +1,107 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * Copyright (C) 2019 Western Digital Corporation or its
> > > affiliates.
> > > + * Adapted from arch/arm64/kernel/efi-header.S
> > > + */
> > > +
> > > +#include <linux/pe.h>
> > > +#include <linux/sizes.h>
> > > +
> > > + .macro __EFI_PE_HEADER
> > > + .long PE_MAGIC
> > > +coff_header:
> > > + .short IMAGE_FILE_MACHINE_RISCV64 // Machine
> > > + .short section_count //
> > > NumberOfSections
> > > + .long 0 //
> > > TimeDateStamp
> > > + .long 0 //
> > > PointerToSymbolTable
> > > + .long 0 //
> > > NumberOfSymbols
> > > + .short section_table - optional_header //
> > > SizeOfOptionalHeader
> > > + .short IMAGE_FILE_DEBUG_STRIPPED | \
> > > + IMAGE_FILE_EXECUTABLE_IMAGE | \
> > > + IMAGE_FILE_LINE_NUMS_STRIPPED //
> > > Characteristics
> > > +
> > > +optional_header:
> > > + .short PE_OPT_MAGIC_PE32PLUS // PE32+
> > > format
> > > + .byte 0x02 //
> > > MajorLinkerVersion
> > > + .byte 0x14 //
> > > MinorLinkerVersion
> > > + .long __text_end - efi_header_end //
> > > SizeOfCode
> > > + .long _end - __text_end //
> > > SizeOfInitializedData
> > > + .long 0 //
> > > SizeOfUninitializedData
> > > + .long __efistub_efi_entry - _start //
> > > AddressOfEntryPoint
> > > + .long efi_header_end - _start //
> > > BaseOfCode
> > > +
> > > +extra_header_fields:
> > > + .quad 0 //
> > > ImageBase
> > > + .long SZ_4K //
> > > SectionAlignment
> > > + .long PECOFF_FILE_ALIGNMENT //
> > > FileAlignment
> > > + .short 0 //
> > > MajorOperatingSystemVersion
> > > + .short 0 //
> > > MinorOperatingSystemVersion
> > > + .short 0 //
> > > MajorImageVersion
> > > + .short 0 //
> > > MinorImageVersion
> >
> > Put LINUX_EFISTUB_MAJOR_VERSION and LINUX_EFISTUB_MINOR_VERSION here
> >
>
> Sure.
>
> > > + .short 0 //
> > > MajorSubsystemVersion
> > > + .short 0 //
> > > MinorSubsystemVersion
> > > + .long 0 //
> > > Win32VersionValue
> > > +
> > > + .long _end - _start //
> > > SizeOfImage
> > > +
> > > + // Everything before the kernel image is considered part of
> > > the header
> > > + .long efi_header_end - _start //
> > > SizeOfHeaders
> > > + .long 0 // CheckSum
> > > + .short IMAGE_SUBSYSTEM_EFI_APPLICATION //
> > > Subsystem
> > > + .short 0 //
> > > DllCharacteristics
> > > + .quad 0 //
> > > SizeOfStackReserve
> > > + .quad 0 //
> > > SizeOfStackCommit
> > > + .quad 0 //
> > > SizeOfHeapReserve
> > > + .quad 0 //
> > > SizeOfHeapCommit
> > > + .long 0 //
> > > LoaderFlags
> > > + .long (section_table - .) / 8 //
> > > NumberOfRvaAndSizes
> > > +
> > > + .quad 0 //
> > > ExportTable
> > > + .quad 0 //
> > > ImportTable
> > > + .quad 0 //
> > > ResourceTable
> > > + .quad 0 //
> > > ExceptionTable
> > > + .quad 0 //
> > > CertificationTable
> > > + .quad 0 //
> > > BaseRelocationTable
> > > +
> > > + // Section table
> > > +section_table:
> > > + .ascii ".text\0\0\0"
> > > + .long __text_end - efi_header_end //
> > > VirtualSize
> > > + .long efi_header_end - _start //
> > > VirtualAddress
> > > + .long __text_end - efi_header_end //
> > > SizeOfRawData
> > > + .long efi_header_end - _start //
> > > PointerToRawData
> > > +
> > > + .long 0 //
> > > PointerToRelocations
> > > + .long 0 //
> > > PointerToLineNumbers
> > > + .short 0 //
> > > NumberOfRelocations
> > > + .short 0 //
> > > NumberOfLineNumbers
> > > + .long IMAGE_SCN_CNT_CODE | \
> > > + IMAGE_SCN_MEM_READ | \
> > > + IMAGE_SCN_MEM_EXECUTE //
> > > Characteristics
> > > +
> > > + .ascii ".data\0\0\0"
> > > + .long __data_virt_size //
> > > VirtualSize
> > > + .long __text_end - _start //
> > > VirtualAddress
> > > + .long __data_raw_size //
> > > SizeOfRawData
> > > + .long __text_end - _start //
> > > PointerToRawData
> > > +
> > > + .long 0 //
> > > PointerToRelocations
> > > + .long 0 //
> > > PointerToLineNumbers
> > > + .short 0 //
> > > NumberOfRelocations
> > > + .short 0 //
> > > NumberOfLineNumbers
> > > + .long IMAGE_SCN_CNT_INITIALIZED_DATA | \
> > > + IMAGE_SCN_MEM_READ | \
> > > + IMAGE_SCN_MEM_WRITE //
> > > Characteristics
> > > +
> > > + .set section_count, (. - section_table) / 40
> > > +
> >
> > You dropped the debug header here, which is actually *very* useful if
> > you want to single step through the stub from DEBUG edk2 firmware.
> >
>
> Ahh I see. I was not sure how to use the debug feature :). Can we do
> the same in U-boot ?
>
You could, but I doubt it has been implemented yet.
Adding the debug header will cause the firmware to record the image
path and its load address in a special debug table, which the firmware
can use to expose the load address and path of the image as it is
loaded (i.e. before moving itself into place)
When I run this under a DEBUG build of EDK2, it prints a line like
add-symbol-file /home/ardbie01/linux-build-arm64/vmlinux 0x74E0B000
which I can paste into the GDB command window, set a breakpoint on
efi_entry(), and single step through the stub.
> How about adding it back later once I can verify it with EDK2 ?
>
Sure, that is fine
> > > + /*
> > > + * EFI will load .text onwards at the 4k section alignment
> > > + * described in the PE/COFF header. To ensure that
> > > instruction
> > > + * sequences using an adrp and a :lo12: immediate will
> > > function
> >
> > Surely, this is inaccurate for RISC-V?
>
> Sorry. I should have removed the comment. We keep the the _start and
> .head.text section aligned to a PAGE_SIZE anyways using the linker
> script.
ok
> >
> > > + * correctly at this alignment, we must ensure that .text
> > > is
> > > + * placed at a 4k boundary in the Image to begin with.
> > > + */
> > > + .align 12
> > > +efi_header_end:
> > > + .endm
> > > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > > index ac5b0e0a02f6..835dc76de285 100644
> > > --- a/arch/riscv/kernel/head.S
> > > +++ b/arch/riscv/kernel/head.S
> > > @@ -13,6 +13,7 @@
> > > #include <asm/csr.h>
> > > #include <asm/hwcap.h>
> > > #include <asm/image.h>
> > > +#include "efi-header.S"
> > >
> > > __HEAD
> > > ENTRY(_start)
> > > @@ -22,10 +23,17 @@ ENTRY(_start)
> > > * Do not modify it without modifying the structure and all
> > > bootloaders
> > > * that expects this header format!!
> > > */
> > > +#ifdef CONFIG_EFI
> > > + /*
> > > + * This instruction decodes to "MZ" ASCII required by UEFI.
> > > + */
> > > + li s4,-13
> >
> > What happens if you try to do plain boot on an EFI kernel? On ARM and
> > x86, we took care to use a 'MZ' opcode that behaves as a pseudo-NOP,
> > and jump to start_kernel right after, so if you boot the EFI kernel
> > as
> > a normal kernel, it still works.
> >
>
> There should have been a "j _start_kernel" after the "MZ" ascii. I just
> tested EFI kernel can now boot as a normal kernel as well with that
> change. Thanks for pointing that out.
>
Sure
> > > +#else
> > > /* jump to start kernel */
> > > j _start_kernel
> > > /* reserved */
> > > .word 0
> > > +#endif
> > > .balign 8
> > > #if __riscv_xlen == 64
> > > /* Image load offset(2MB) from start of RAM */
> > > @@ -43,7 +51,14 @@ ENTRY(_start)
> > > .ascii RISCV_IMAGE_MAGIC
> > > .balign 4
> > > .ascii RISCV_IMAGE_MAGIC2
> > > +#ifdef CONFIG_EFI
> > > + .word pe_head_start - _start
> > > +pe_head_start:
> > > +
> > > + __EFI_PE_HEADER
> > > +#else
> > > .word 0
> > > +#endif
> > >
> > > .align 2
> > > #ifdef CONFIG_MMU
> > > diff --git a/arch/riscv/kernel/image-vars.h
> > > b/arch/riscv/kernel/image-vars.h
> > > new file mode 100644
> > > index 000000000000..57abb85065e9
> > > --- /dev/null
> > > +++ b/arch/riscv/kernel/image-vars.h
> > > @@ -0,0 +1,52 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * Linker script variables to be set after section resolution, as
> > > + * ld.lld does not like variables assigned before SECTIONS is
> > > processed.
> > > + * Based on arch/arm64/kerne/image-vars.h
> > > + */
> > > +#ifndef __RISCV_KERNEL_IMAGE_VARS_H
> > > +#define __RISCV_KERNEL_IMAGE_VARS_H
> > > +
> > > +#ifndef LINKER_SCRIPT
> > > +#error This file should only be included in vmlinux.lds.S
> > > +#endif
> > > +
> > > +#ifdef CONFIG_EFI
> > > +
> > > +__efistub_stext_offset = _start_kernel - _start;
> > > +
> > > +/*
> > > + * The EFI stub has its own symbol namespace prefixed by
> > > __efistub_, to
> > > + * isolate it from the kernel proper. The following symbols are
> > > legally
> > > + * accessed by the stub, so provide some aliases to make them
> > > accessible.
> > > + * Only include data symbols here, or text symbols of functions
> > > that are
> > > + * guaranteed to be safe when executed at another offset than they
> > > were
> > > + * linked at. The routines below are all implemented in assembler
> > > in a
> > > + * position independent manner
> > > + */
> > > +__efistub_memcmp = memcmp;
> > > +__efistub_memchr = memchr;
> > > +__efistub_memcpy = memcpy;
> > > +__efistub_memmove = memmove;
> > > +__efistub_memset = memset;
> > > +__efistub_strlen = strlen;
> > > +__efistub_strnlen = strnlen;
> > > +__efistub_strcmp = strcmp;
> > > +__efistub_strncmp = strncmp;
> > > +__efistub_strrchr = strrchr;
> > > +
> > > +#ifdef CONFIG_KASAN
> > > +__efistub___memcpy = memcpy;
> > > +__efistub___memmove = memmove;
> > > +__efistub___memset = memset;
> > > +#endif
> > > +
> > > +__efistub__start = _start;
> > > +__efistub__start_kernel = _start_kernel;
> > > +__efistub__end = _end;
> > > +__efistub__edata = _edata;
> > > +__efistub_screen_info = screen_info;
> > > +
> > > +#endif
> > > +
> > > +#endif /* __RISCV_KERNEL_IMAGE_VARS_H */
> > > diff --git a/arch/riscv/kernel/vmlinux.lds.S
> > > b/arch/riscv/kernel/vmlinux.lds.S
> > > index b32640300d07..933b9e9a4b39 100644
> > > --- a/arch/riscv/kernel/vmlinux.lds.S
> > > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > > @@ -9,6 +9,7 @@
> > > #include <asm/page.h>
> > > #include <asm/cache.h>
> > > #include <asm/thread_info.h>
> > > +#include "image-vars.h"
> > >
> > > #include <linux/sizes.h>
> > > OUTPUT_ARCH(riscv)
> > > @@ -16,6 +17,14 @@ ENTRY(_start)
> > >
> > > jiffies = jiffies_64;
> > >
> > > +PECOFF_FILE_ALIGNMENT = 0x200;
> > > +#ifdef CONFIG_EFI
> > > +#define PECOFF_EDATA_PADDING \
> > > + .pecoff_edata_padding : { BYTE(0); . =
> > > ALIGN(PECOFF_FILE_ALIGNMENT); }
> > > +#else
> > > +#define PECOFF_EDATA_PADDING
> > > +#endif
> > > +
> > > SECTIONS
> > > {
> > > /* Beginning of code and text segment */
> > > @@ -26,12 +35,15 @@ SECTIONS
> > >
> > > __init_begin = .;
> > > INIT_TEXT_SECTION(PAGE_SIZE)
> > > +
> > > + /* Start of data section */
> > > INIT_DATA_SECTION(16)
> > > /* we have to discard exit text and such at runtime, not
> > > link time */
> > > .exit.text :
> > > {
> > > EXIT_TEXT
> > > }
> > > +
> > > .exit.data :
> > > {
> > > EXIT_DATA
> > > @@ -54,7 +66,8 @@ SECTIONS
> > > _etext = .;
> > > }
> > >
> > > - /* Start of data section */
> > > + __text_end = .;
> > > +
> > > _sdata = .;
> > > RO_DATA(L1_CACHE_BYTES)
> > > .srodata : {
> > > @@ -65,19 +78,21 @@ SECTIONS
> > > .sdata : {
> > > __global_pointer$ = . + 0x800;
> > > *(.sdata*)
> > > - /* End of data section */
> > > - _edata = .;
> > > *(.sbss*)
> > > }
> > > -
> > > - BSS_SECTION(PAGE_SIZE, PAGE_SIZE, 0)
> > > -
> > > + PECOFF_EDATA_PADDING
> > > + __data_raw_size = ABSOLUTE(. - __text_end);
> > > + /* End of data section */
> > > + _edata = .;
> > > EXCEPTION_TABLE(0x10)
> > >
> > > .rel.dyn : {
> > > *(.rel.dyn*)
> > > }
> > >
> > > + BSS_SECTION(PAGE_SIZE, PAGE_SIZE, 0)
> > > + __data_virt_size = ABSOLUTE(. - __text_end);
> > > +
> > > _end = .;
> > >
> > > STABS_DEBUG
> > > --
> > > 2.24.0
> > >
>
> --
> Regards,
> Atish
^ permalink raw reply [flat|nested] 26+ messages in thread
* [RFC PATCH 5/5] RISC-V: Add EFI stub support.
2020-02-26 1:10 [RFC PATCH 0/5] Add UEFI support for RISC-V Atish Patra
` (3 preceding siblings ...)
2020-02-26 1:10 ` [RFC PATCH 4/5] RISC-V: Add PE/COFF header for EFI stub Atish Patra
@ 2020-02-26 1:10 ` Atish Patra
2020-02-26 7:28 ` Ard Biesheuvel
2020-02-26 23:46 ` [RFC PATCH 0/5] Add UEFI support for RISC-V Palmer Dabbelt
5 siblings, 1 reply; 26+ messages in thread
From: Atish Patra @ 2020-02-26 1:10 UTC (permalink / raw)
To: linux-kernel
Cc: Atish Patra, Albert Ou, Alexios Zavras, Allison Randal,
Andrew Morton, Anup Patel, Ard Biesheuvel, Arnd Bergmann,
Borislav Petkov, Catalin Marinas, Greentime Hu,
Greg Kroah-Hartman, Ingo Molnar, Kate Stewart, Kees Cook,
Linus Walleij, linux-efi, linux-riscv, Mao Han,
Mauro Carvalho Chehab, Michal Simek, Mike Rapoport,
Palmer Dabbelt, Paolo Bonzini, Paul Walmsley, Russell King,
Thomas Gleixner, Will Deacon, Alexander Graf, leif, Chang,
Abner (HPS SW/FW Technologist),
daniel.schaefer
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)
+#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;
+}
--
2.24.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [RFC PATCH 5/5] RISC-V: Add EFI stub support.
2020-02-26 1:10 ` [RFC PATCH 5/5] RISC-V: Add EFI stub support Atish Patra
@ 2020-02-26 7:28 ` Ard Biesheuvel
2020-02-27 19:53 ` Atish Patra
0 siblings, 1 reply; 26+ messages in thread
From: Ard Biesheuvel @ 2020-02-26 7:28 UTC (permalink / raw)
To: Atish Patra
Cc: Linux Kernel Mailing List, Albert Ou, Alexios Zavras,
Allison Randal, Andrew Morton, Anup Patel, Arnd Bergmann,
Borislav Petkov, Catalin Marinas, Greentime Hu,
Greg Kroah-Hartman, Ingo Molnar, Kate Stewart, Kees Cook,
Linus Walleij, linux-efi, linux-riscv, Mao Han,
Mauro Carvalho Chehab, Michal Simek, Mike Rapoport,
Palmer Dabbelt, Paolo Bonzini, Paul Walmsley, Russell King,
Thomas Gleixner, Will Deacon, Alexander Graf, leif, Chang,
Abner (HPS SW/FW Technologist), Schaefer, Daniel (DualStudy)
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
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [RFC PATCH 5/5] RISC-V: Add EFI stub support.
2020-02-26 7:28 ` Ard Biesheuvel
@ 2020-02-27 19:53 ` Atish Patra
2020-02-27 19:59 ` Ard Biesheuvel
0 siblings, 1 reply; 26+ messages in thread
From: Atish Patra @ 2020-02-27 19:53 UTC (permalink / raw)
To: ardb
Cc: alexios.zavras, tglx, mchehab+samsung, pbonzini, michal.simek,
linux, abner.chang, linux-riscv, catalin.marinas, linux-kernel,
daniel.schaefer, Anup Patel, kstewart, palmer, aou, arnd, rppt,
linux-efi, bp, greentime.hu, keescook, agraf, will, gregkh,
mingo, allison, han_mao, paul.walmsley, linus.walleij, leif,
akpm
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
> >
--
Regards,
Atish
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [RFC PATCH 5/5] RISC-V: Add EFI stub support.
2020-02-27 19:53 ` Atish Patra
@ 2020-02-27 19:59 ` Ard Biesheuvel
2020-02-28 1:05 ` Atish Patra
0 siblings, 1 reply; 26+ messages in thread
From: Ard Biesheuvel @ 2020-02-27 19:59 UTC (permalink / raw)
To: Atish Patra
Cc: alexios.zavras, tglx, mchehab+samsung, pbonzini, michal.simek,
linux, abner.chang, linux-riscv, catalin.marinas, linux-kernel,
daniel.schaefer, Anup Patel, kstewart, palmer, aou, arnd, rppt,
linux-efi, bp, greentime.hu, keescook, agraf, will, gregkh,
mingo, allison, han_mao, paul.walmsley, linus.walleij, leif,
akpm
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
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [RFC PATCH 5/5] RISC-V: Add EFI stub support.
2020-02-27 19:59 ` Ard Biesheuvel
@ 2020-02-28 1:05 ` Atish Patra
2020-02-28 6:57 ` Ard Biesheuvel
0 siblings, 1 reply; 26+ messages in thread
From: Atish Patra @ 2020-02-28 1:05 UTC (permalink / raw)
To: ardb
Cc: alexios.zavras, tglx, mchehab+samsung, pbonzini, linux,
michal.simek, abner.chang, linux-riscv, catalin.marinas,
linux-kernel, daniel.schaefer, Anup Patel, kstewart, palmer, aou,
arnd, rppt, bp, linux-efi, greentime.hu, keescook, agraf, will,
gregkh, mingo, allison, han_mao, paul.walmsley, leif,
linus.walleij, akpm
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
--
Regards,
Atish
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [RFC PATCH 5/5] RISC-V: Add EFI stub support.
2020-02-28 1:05 ` Atish Patra
@ 2020-02-28 6:57 ` Ard Biesheuvel
2020-03-10 7:08 ` Atish Patra
0 siblings, 1 reply; 26+ messages in thread
From: Ard Biesheuvel @ 2020-02-28 6:57 UTC (permalink / raw)
To: Atish Patra
Cc: alexios.zavras, tglx, mchehab+samsung, pbonzini, linux,
michal.simek, abner.chang, linux-riscv, catalin.marinas,
linux-kernel, daniel.schaefer, Anup Patel, kstewart, palmer, aou,
arnd, rppt, bp, linux-efi, greentime.hu, keescook, agraf, will,
gregkh, mingo, allison, han_mao, paul.walmsley, leif,
linus.walleij, akpm
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.
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH 5/5] RISC-V: Add EFI stub support.
2020-02-28 6:57 ` Ard Biesheuvel
@ 2020-03-10 7:08 ` Atish Patra
2020-03-10 7:38 ` Anup Patel
2020-03-10 13:18 ` Ard Biesheuvel
0 siblings, 2 replies; 26+ messages in thread
From: Atish Patra @ 2020-03-10 7:08 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Atish Patra, linux, abner.chang, linux-riscv, linux-kernel,
daniel.schaefer, Anup Patel, palmer, linux-efi, agraf,
paul.walmsley, leif
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.
--
Regards,
Atish
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH 5/5] RISC-V: Add EFI stub support.
2020-03-10 7:08 ` Atish Patra
@ 2020-03-10 7:38 ` Anup Patel
2020-03-10 13:18 ` Ard Biesheuvel
1 sibling, 0 replies; 26+ messages in thread
From: Anup Patel @ 2020-03-10 7:38 UTC (permalink / raw)
To: Atish Patra
Cc: Ard Biesheuvel, Atish Patra, linux, abner.chang, linux-riscv,
linux-kernel, daniel.schaefer, Anup Patel, palmer, linux-efi,
agraf, paul.walmsley, leif
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH 5/5] RISC-V: Add EFI stub support.
2020-03-10 7:08 ` Atish Patra
2020-03-10 7:38 ` Anup Patel
@ 2020-03-10 13:18 ` Ard Biesheuvel
1 sibling, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2020-03-10 13:18 UTC (permalink / raw)
To: Atish Patra
Cc: Atish Patra, linux, abner.chang, linux-riscv, linux-kernel,
daniel.schaefer, Anup Patel, palmer, linux-efi, agraf,
paul.walmsley, leif
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
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [RFC PATCH 0/5] Add UEFI support for RISC-V
2020-02-26 1:10 [RFC PATCH 0/5] Add UEFI support for RISC-V Atish Patra
` (4 preceding siblings ...)
2020-02-26 1:10 ` [RFC PATCH 5/5] RISC-V: Add EFI stub support Atish Patra
@ 2020-02-26 23:46 ` Palmer Dabbelt
2020-02-27 2:09 ` Atish Patra
5 siblings, 1 reply; 26+ messages in thread
From: Palmer Dabbelt @ 2020-02-26 23:46 UTC (permalink / raw)
To: Atish Patra
Cc: linux-kernel, Atish Patra, aou, alexios.zavras, allison, akpm,
Anup Patel, ardb, Arnd Bergmann, bp, catalin.marinas,
greentime.hu, Greg KH, mingo, kstewart, keescook, linus.walleij,
linux-efi, linux-riscv, han_mao, mchehab+samsung, michal.simek,
rppt, pbonzini, Paul Walmsley, linux, tglx, will, agraf, leif,
abner.chang, daniel.schaefer
On Tue, 25 Feb 2020 17:10:32 PST (-0800), Atish Patra wrote:
> This series adds UEFI support for RISC-V. Currently, only boot time
> services have been added. Runtime services will be added in a separate
> series. This series depends on some core EFI patches
> present in current in efi-next and following other patches.
>
> U-Boot: Adds the boot hartid under chosen node.
> https://lists.denx.de/pipermail/u-boot/2020-February/401227.html
>
> Linux kernel: SBI v0.2 and HSM extension support. This series is a mandatory
> pre-requisite for UEFI support as only single core can boot EFI stub and
> Linux via UEFI. All other cores are brought up using SBI HSM extension.
> http://lists.infradead.org/pipermail/linux-riscv/2020-February/008513.html
>
> OpenSBI: master (commit: ge3f69fc1e934)
>
> Patch 1 just moves arm-stub code to a generic code so that it can be used
> across different architecture.
>
> Patch 3 adds fixmap bindings so that CONFIG_EFI can be compiled and we do not
> have create separate config to enable boot time services.
> As runtime services are not enabled at this time, full generic early ioremap
> support is also not added in this series.
>
> Patch 4 and 5 adds the PE/COFF header and EFI stub code support for RISC-V
> respectively.
>
> The patches can also be found in following git repo.
>
> https://github.com/atishp04/linux/tree/wip_uefi_riscv
>
> The patches have been verified on Qemu using bootefi command in U-Boot.
> Here is a boot log.
>
> Atish Patra (5):
> efi: Move arm-stub to a common file
> include: pe.h: Add RISC-V related PE definition
> RISC-V: Define fixmap bindings for generic early ioremap support
> RISC-V: Add PE/COFF header for EFI stub
> RISC-V: Add EFI stub support.
>
> arch/arm/Kconfig | 2 +-
> arch/arm64/Kconfig | 2 +-
> arch/riscv/Kconfig | 21 +++
> arch/riscv/Makefile | 1 +
> arch/riscv/configs/defconfig | 1 +
> arch/riscv/include/asm/Kbuild | 2 +-
> arch/riscv/include/asm/fixmap.h | 21 ++-
> arch/riscv/include/asm/io.h | 1 +
> arch/riscv/include/asm/sections.h | 13 ++
> arch/riscv/kernel/Makefile | 4 +
> arch/riscv/kernel/efi-header.S | 107 ++++++++++++++
> arch/riscv/kernel/head.S | 15 ++
> arch/riscv/kernel/image-vars.h | 52 +++++++
> arch/riscv/kernel/vmlinux.lds.S | 27 +++-
> drivers/firmware/efi/Kconfig | 6 +-
> drivers/firmware/efi/libstub/Makefile | 20 ++-
> .../efi/libstub/{arm-stub.c => efi-stub.c} | 7 +-
> drivers/firmware/efi/libstub/riscv-stub.c | 135 ++++++++++++++++++
> include/linux/pe.h | 3 +
> 19 files changed, 420 insertions(+), 20 deletions(-)
> create mode 100644 arch/riscv/include/asm/sections.h
> create mode 100644 arch/riscv/kernel/efi-header.S
> create mode 100644 arch/riscv/kernel/image-vars.h
> rename drivers/firmware/efi/libstub/{arm-stub.c => efi-stub.c} (98%)
> create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c
I'm in favor of adding EFI support, and I'd rather have it sooner than later so
we don't paint ourselves into a corner. I'm not sure what happened to the
RISC-V EFI spec process, though, which would be my only worry here (also I
haven't looked at the code :)). Do we have enough of a spec through the EFI
process that this is all kosher on their end?
Given that this definately isn't for these RCs, I'm going to leave it in my
review queue. It might be best to get the "move stuff to generic" work merged
on its own, as then we can carry less diff around.
Thanks!
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [RFC PATCH 0/5] Add UEFI support for RISC-V
2020-02-26 23:46 ` [RFC PATCH 0/5] Add UEFI support for RISC-V Palmer Dabbelt
@ 2020-02-27 2:09 ` Atish Patra
0 siblings, 0 replies; 26+ messages in thread
From: Atish Patra @ 2020-02-27 2:09 UTC (permalink / raw)
To: palmer
Cc: alexios.zavras, tglx, mchehab+samsung, pbonzini, michal.simek,
linux, abner.chang, linux-riscv, catalin.marinas, linux-kernel,
daniel.schaefer, Anup Patel, kstewart, aou, arnd, rppt, bp,
linux-efi, greentime.hu, keescook, agraf, will, gregkh, mingo,
allison, han_mao, paul.walmsley, ardb, linus.walleij, leif, akpm
On Wed, 2020-02-26 at 15:46 -0800, Palmer Dabbelt wrote:
> On Tue, 25 Feb 2020 17:10:32 PST (-0800), Atish Patra wrote:
> > This series adds UEFI support for RISC-V. Currently, only boot time
> > services have been added. Runtime services will be added in a
> > separate
> > series. This series depends on some core EFI patches
> > present in current in efi-next and following other patches.
> >
> > U-Boot: Adds the boot hartid under chosen node.
> > https://lists.denx.de/pipermail/u-boot/2020-February/401227.html
> >
> > Linux kernel: SBI v0.2 and HSM extension support. This series is a
> > mandatory
> > pre-requisite for UEFI support as only single core can boot EFI
> > stub and
> > Linux via UEFI. All other cores are brought up using SBI HSM
> > extension.
> > http://lists.infradead.org/pipermail/linux-riscv/2020-February/008513.html
> >
> > OpenSBI: master (commit: ge3f69fc1e934)
> >
> > Patch 1 just moves arm-stub code to a generic code so that it can
> > be used
> > across different architecture.
> >
> > Patch 3 adds fixmap bindings so that CONFIG_EFI can be compiled and
> > we do not
> > have create separate config to enable boot time services.
> > As runtime services are not enabled at this time, full generic
> > early ioremap
> > support is also not added in this series.
> >
> > Patch 4 and 5 adds the PE/COFF header and EFI stub code support for
> > RISC-V
> > respectively.
> >
> > The patches can also be found in following git repo.
> >
> > https://github.com/atishp04/linux/tree/wip_uefi_riscv
> >
> > The patches have been verified on Qemu using bootefi command in U-
> > Boot.
> > Here is a boot log.
> >
> > Atish Patra (5):
> > efi: Move arm-stub to a common file
> > include: pe.h: Add RISC-V related PE definition
> > RISC-V: Define fixmap bindings for generic early ioremap support
> > RISC-V: Add PE/COFF header for EFI stub
> > RISC-V: Add EFI stub support.
> >
> > arch/arm/Kconfig | 2 +-
> > arch/arm64/Kconfig | 2 +-
> > arch/riscv/Kconfig | 21 +++
> > arch/riscv/Makefile | 1 +
> > arch/riscv/configs/defconfig | 1 +
> > arch/riscv/include/asm/Kbuild | 2 +-
> > arch/riscv/include/asm/fixmap.h | 21 ++-
> > arch/riscv/include/asm/io.h | 1 +
> > arch/riscv/include/asm/sections.h | 13 ++
> > arch/riscv/kernel/Makefile | 4 +
> > arch/riscv/kernel/efi-header.S | 107 ++++++++++++++
> > arch/riscv/kernel/head.S | 15 ++
> > arch/riscv/kernel/image-vars.h | 52 +++++++
> > arch/riscv/kernel/vmlinux.lds.S | 27 +++-
> > drivers/firmware/efi/Kconfig | 6 +-
> > drivers/firmware/efi/libstub/Makefile | 20 ++-
> > .../efi/libstub/{arm-stub.c => efi-stub.c} | 7 +-
> > drivers/firmware/efi/libstub/riscv-stub.c | 135
> > ++++++++++++++++++
> > include/linux/pe.h | 3 +
> > 19 files changed, 420 insertions(+), 20 deletions(-)
> > create mode 100644 arch/riscv/include/asm/sections.h
> > create mode 100644 arch/riscv/kernel/efi-header.S
> > create mode 100644 arch/riscv/kernel/image-vars.h
> > rename drivers/firmware/efi/libstub/{arm-stub.c => efi-stub.c}
> > (98%)
> > create mode 100644 drivers/firmware/efi/libstub/riscv-stub.c
>
> I'm in favor of adding EFI support, and I'd rather have it sooner
> than later so
> we don't paint ourselves into a corner. I'm not sure what happened
> to the
> RISC-V EFI spec process, though, which would be my only worry here
> (also I
> haven't looked at the code :)). Do we have enough of a spec through
> the EFI
> process that this is all kosher on their end?
>
The RISC-V platform is already merged in UEFI spec. The latest UEFI
spec can be found here.
https://uefi.org/sites/default/files/resources/UEFI_Spec_2_8_A_Feb14.pdf
Section 2.3.7: RISC-V Platforms
There are some modification required in the current spec. For example:
The current spec assumes that UEFI will execute in M mode only and
should perform handoff control to OS in M-mode as well. This is not
entirely correct as we can do this in S-mode as well.
We are in the process of creating an ECR so that next release will have
the correct information.
> Given that this definately isn't for these RCs, I'm going to leave it
> in my
> review queue. It might be best to get the "move stuff to generic"
> work merged
> on its own, as then we can carry less diff around.
>
>
Yup. It's definitely not rc material. Given that RISC-V specific
section is already merged in UEFI spec, can we aim for 5.7 merge window?
> Thanks!
--
Regards,
Atish
^ permalink raw reply [flat|nested] 26+ messages in thread