All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V3] LoongArch: Add efistub booting support
@ 2022-08-19 10:20 Huacai Chen
  2022-08-22 10:44 ` Ard Biesheuvel
  2022-08-27  4:41 ` Xi Ruoyao
  0 siblings, 2 replies; 26+ messages in thread
From: Huacai Chen @ 2022-08-19 10:20 UTC (permalink / raw)
  To: Arnd Bergmann, Huacai Chen
  Cc: loongarch, linux-arch, Xuefeng Li, Guo Ren, Xuerui Wang,
	Jiaxun Yang, Ard Biesheuvel, linux-efi, linux-kernel,
	Huacai Chen, Xi Ruoyao

This patch adds efistub booting support, which is the standard UEFI boot
protocol for us to use.

We use generic efistub, which means we can pass boot information (i.e.,
system table, memory map, kernel command line, initrd) via a light FDT
and drop a lot of non-standard code.

We use a flat mapping to map the efi runtime in the kernel's address
space. In efi, VA = PA; in kernel, VA = PA + PAGE_OFFSET. As a result,
flat mapping is not identity mapping, SetVirtualAddressMap() is still
needed for the efi runtime.

Tested-by: Xi Ruoyao <xry111@xry111.site>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
V1 --> V2:
1, Call SetVirtualAddressMap() in stub;
2, Use core kernel data directly in alloc_screen_info();
3, Remove the magic number in MS-DOS header;
4, Disable EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER;
5, Some other small changes suggested by Ard Biesheuvel.

V2 --> V3:
1, Adjust Makefile to adapt zboot;
2, Introduce EFI_RT_VIRTUAL_OFFSET instead of changing flat_va_mapping.

 arch/loongarch/Kconfig                        |  9 ++
 arch/loongarch/Makefile                       | 13 ++-
 arch/loongarch/boot/Makefile                  |  8 +-
 arch/loongarch/include/asm/efi.h              | 11 ++-
 arch/loongarch/kernel/efi-header.S            | 99 +++++++++++++++++++
 arch/loongarch/kernel/efi.c                   |  3 +
 arch/loongarch/kernel/head.S                  | 20 ++++
 arch/loongarch/kernel/image-vars.h            | 30 ++++++
 arch/loongarch/kernel/setup.c                 | 12 +--
 arch/loongarch/kernel/vmlinux.lds.S           |  1 +
 drivers/firmware/efi/Kconfig                  |  4 +-
 drivers/firmware/efi/libstub/Makefile         | 10 ++
 drivers/firmware/efi/libstub/efi-stub.c       | 16 +--
 drivers/firmware/efi/libstub/loongarch-stub.c | 60 +++++++++++
 include/linux/pe.h                            |  2 +
 15 files changed, 272 insertions(+), 26 deletions(-)
 create mode 100644 arch/loongarch/kernel/efi-header.S
 create mode 100644 arch/loongarch/kernel/image-vars.h
 create mode 100644 drivers/firmware/efi/libstub/loongarch-stub.c

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index 9478f9646fa5..4cb412a82afa 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -324,6 +324,15 @@ config EFI
 	  This enables the kernel to use EFI runtime services that are
 	  available (such as the EFI variable services).
 
+config EFI_STUB
+	bool "EFI boot stub support"
+	default y
+	depends on EFI
+	select EFI_GENERIC_STUB
+	help
+	  This kernel feature allows the kernel to be loaded directly by
+	  EFI firmware without the use of a bootloader.
+
 config SMP
 	bool "Multi-Processing support"
 	help
diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index ec3de6191276..4bc47f47cfd8 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -7,7 +7,11 @@ boot	:= arch/loongarch/boot
 
 KBUILD_DEFCONFIG := loongson3_defconfig
 
-KBUILD_IMAGE	= $(boot)/vmlinux
+ifndef CONFIG_EFI_STUB
+KBUILD_IMAGE	:= $(boot)/vmlinux.elf
+else
+KBUILD_IMAGE	:= $(boot)/vmlinux.efi
+endif
 
 #
 # Select the object file format to substitute into the linker script.
@@ -75,6 +79,7 @@ endif
 head-y := arch/loongarch/kernel/head.o
 
 libs-y += arch/loongarch/lib/
+libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
 
 ifeq ($(KBUILD_EXTMOD),)
 prepare: vdso_prepare
@@ -86,10 +91,10 @@ PHONY += vdso_install
 vdso_install:
 	$(Q)$(MAKE) $(build)=arch/loongarch/vdso $@
 
-all:	$(KBUILD_IMAGE)
+all:	$(notdir $(KBUILD_IMAGE))
 
-$(KBUILD_IMAGE): vmlinux
-	$(Q)$(MAKE) $(build)=$(boot) $(bootvars-y) $@
+vmlinux.elf vmlinux.efi: vmlinux
+	$(Q)$(MAKE) $(build)=$(boot) $(bootvars-y) $(boot)/$@
 
 install:
 	$(Q)install -D -m 755 $(KBUILD_IMAGE) $(INSTALL_PATH)/vmlinux-$(KERNELRELEASE)
diff --git a/arch/loongarch/boot/Makefile b/arch/loongarch/boot/Makefile
index 0125b17edc98..fecf34f50e56 100644
--- a/arch/loongarch/boot/Makefile
+++ b/arch/loongarch/boot/Makefile
@@ -8,9 +8,13 @@ drop-sections := .comment .note .options .note.gnu.build-id
 strip-flags   := $(addprefix --remove-section=,$(drop-sections)) -S
 OBJCOPYFLAGS_vmlinux.efi := -O binary $(strip-flags)
 
-targets := vmlinux
 quiet_cmd_strip = STRIP	  $@
       cmd_strip = $(STRIP) -s -o $@ $<
 
-$(obj)/vmlinux: vmlinux FORCE
+targets := vmlinux.elf
+$(obj)/vmlinux.elf: vmlinux FORCE
 	$(call if_changed,strip)
+
+targets += vmlinux.efi
+$(obj)/vmlinux.efi: vmlinux FORCE
+	$(call if_changed,objcopy)
diff --git a/arch/loongarch/include/asm/efi.h b/arch/loongarch/include/asm/efi.h
index 9d44c6948be1..50c54b4174eb 100644
--- a/arch/loongarch/include/asm/efi.h
+++ b/arch/loongarch/include/asm/efi.h
@@ -17,9 +17,16 @@ void efifb_setup_from_dmi(struct screen_info *si, const char *opt);
 #define arch_efi_call_virt_teardown()
 
 #define EFI_ALLOC_ALIGN		SZ_64K
+#define EFI_RT_VIRTUAL_OFFSET	CSR_DMW0_BASE
 
-struct screen_info *alloc_screen_info(void);
-void free_screen_info(struct screen_info *si);
+static inline struct screen_info *alloc_screen_info(void)
+{
+	return &screen_info;
+}
+
+static inline void free_screen_info(struct screen_info *si)
+{
+}
 
 static inline unsigned long efi_get_max_initrd_addr(unsigned long image_addr)
 {
diff --git a/arch/loongarch/kernel/efi-header.S b/arch/loongarch/kernel/efi-header.S
new file mode 100644
index 000000000000..8c1d229a2afa
--- /dev/null
+++ b/arch/loongarch/kernel/efi-header.S
@@ -0,0 +1,99 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020-2022 Loongson Technology Corporation Limited
+ */
+
+#include <linux/pe.h>
+#include <linux/sizes.h>
+
+	.macro	__EFI_PE_HEADER
+	.long	PE_MAGIC
+.Lcoff_header:
+	.short	IMAGE_FILE_MACHINE_LOONGARCH64		/* Machine */
+	.short	.Lsection_count				/* NumberOfSections */
+	.long	0 					/* TimeDateStamp */
+	.long	0					/* PointerToSymbolTable */
+	.long	0					/* NumberOfSymbols */
+	.short	.Lsection_table - .Loptional_header	/* SizeOfOptionalHeader */
+	.short	IMAGE_FILE_DEBUG_STRIPPED | \
+		IMAGE_FILE_EXECUTABLE_IMAGE | \
+		IMAGE_FILE_LINE_NUMS_STRIPPED		/* Characteristics */
+
+.Loptional_header:
+	.short	PE_OPT_MAGIC_PE32PLUS			/* PE32+ format */
+	.byte	0x02					/* MajorLinkerVersion */
+	.byte	0x14					/* MinorLinkerVersion */
+	.long	__inittext_end - .Lefi_header_end	/* SizeOfCode */
+	.long	_end - __initdata_begin			/* SizeOfInitializedData */
+	.long	0					/* SizeOfUninitializedData */
+	.long	__efistub_efi_pe_entry - _head		/* AddressOfEntryPoint */
+	.long	.Lefi_header_end - _head		/* BaseOfCode */
+
+.Lextra_header_fields:
+	.quad	0					/* ImageBase */
+	.long	PECOFF_SEGMENT_ALIGN			/* SectionAlignment */
+	.long	PECOFF_FILE_ALIGN			/* FileAlignment */
+	.short	0					/* MajorOperatingSystemVersion */
+	.short	0					/* MinorOperatingSystemVersion */
+	.short	LINUX_EFISTUB_MAJOR_VERSION		/* MajorImageVersion */
+	.short	LINUX_EFISTUB_MINOR_VERSION		/* MinorImageVersion */
+	.short	0					/* MajorSubsystemVersion */
+	.short	0					/* MinorSubsystemVersion */
+	.long	0					/* Win32VersionValue */
+
+	.long	_end - _head				/* SizeOfImage */
+
+	/* Everything before the kernel image is considered part of the header */
+	.long	.Lefi_header_end - _head		/* 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	(.Lsection_table - .) / 8		/* NumberOfRvaAndSizes */
+
+	.quad	0					/* ExportTable */
+	.quad	0					/* ImportTable */
+	.quad	0					/* ResourceTable */
+	.quad	0					/* ExceptionTable */
+	.quad	0					/* CertificationTable */
+	.quad	0					/* BaseRelocationTable */
+
+	/* Section table */
+.Lsection_table:
+	.ascii	".text\0\0\0"
+	.long	__inittext_end - .Lefi_header_end	/* VirtualSize */
+	.long	.Lefi_header_end - _head		/* VirtualAddress */
+	.long	__inittext_end - .Lefi_header_end	/* SizeOfRawData */
+	.long	.Lefi_header_end - _head		/* 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	_end - __initdata_begin			/* VirtualSize */
+	.long	__initdata_begin - _head		/* VirtualAddress */
+	.long	_edata - __initdata_begin		/* SizeOfRawData */
+	.long	__initdata_begin - _head		/* 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	.Lsection_count, (. - .Lsection_table) / 40
+
+	.balign	0x10000					/* PECOFF_SEGMENT_ALIGN */
+.Lefi_header_end:
+	.endm
diff --git a/arch/loongarch/kernel/efi.c b/arch/loongarch/kernel/efi.c
index a50b60c587fa..1f1f755fb425 100644
--- a/arch/loongarch/kernel/efi.c
+++ b/arch/loongarch/kernel/efi.c
@@ -69,4 +69,7 @@ void __init efi_init(void)
 	config_tables = early_memremap(efi_config_table, efi_nr_tables * size);
 	efi_config_parse_tables(config_tables, efi_systab->nr_tables, arch_tables);
 	early_memunmap(config_tables, efi_nr_tables * size);
+
+	if (screen_info.orig_video_isVGA == VIDEO_TYPE_EFI)
+		memblock_reserve(screen_info.lfb_base, screen_info.lfb_size);
 }
diff --git a/arch/loongarch/kernel/head.S b/arch/loongarch/kernel/head.S
index c60eb66793e3..01bac62a6442 100644
--- a/arch/loongarch/kernel/head.S
+++ b/arch/loongarch/kernel/head.S
@@ -12,6 +12,26 @@
 #include <asm/loongarch.h>
 #include <asm/stackframe.h>
 
+#ifdef CONFIG_EFI_STUB
+
+#include "efi-header.S"
+
+	__HEAD
+
+_head:
+	.word	MZ_MAGIC		/* "MZ", MS-DOS header */
+	.org	0x3c			/* 0x04 ~ 0x3b reserved */
+	.long	pe_header - _head	/* Offset to the PE header */
+
+pe_header:
+	__EFI_PE_HEADER
+
+SYM_DATA(kernel_asize, .long _end - _text);
+SYM_DATA(kernel_fsize, .long _edata - _text);
+SYM_DATA(kernel_offset, .long kernel_offset - _text);
+
+#endif
+
 	__REF
 
 SYM_CODE_START(kernel_entry)			# kernel entry point
diff --git a/arch/loongarch/kernel/image-vars.h b/arch/loongarch/kernel/image-vars.h
new file mode 100644
index 000000000000..c901ebb903f2
--- /dev/null
+++ b/arch/loongarch/kernel/image-vars.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020-2022 Loongson Technology Corporation Limited
+ */
+#ifndef __LOONGARCH_KERNEL_IMAGE_VARS_H
+#define __LOONGARCH_KERNEL_IMAGE_VARS_H
+
+#ifdef CONFIG_EFI_STUB
+
+__efistub_memcmp		= memcmp;
+__efistub_memchr		= memchr;
+__efistub_memcpy		= memcpy;
+__efistub_memmove		= memmove;
+__efistub_memset		= memset;
+__efistub_strcat		= strcat;
+__efistub_strcmp		= strcmp;
+__efistub_strlen		= strlen;
+__efistub_strncat		= strncat;
+__efistub_strnstr		= strnstr;
+__efistub_strnlen		= strnlen;
+__efistub_strrchr		= strrchr;
+__efistub_kernel_entry		= kernel_entry;
+__efistub_kernel_asize		= kernel_asize;
+__efistub_kernel_fsize		= kernel_fsize;
+__efistub_kernel_offset		= kernel_offset;
+__efistub_screen_info		= screen_info;
+
+#endif
+
+#endif /* __LOONGARCH_KERNEL_IMAGE_VARS_H */
diff --git a/arch/loongarch/kernel/setup.c b/arch/loongarch/kernel/setup.c
index 23ee293e1cd2..f938aae3e92c 100644
--- a/arch/loongarch/kernel/setup.c
+++ b/arch/loongarch/kernel/setup.c
@@ -49,9 +49,7 @@
 #define SMBIOS_CORE_PACKAGE_OFFSET	0x23
 #define LOONGSON_EFI_ENABLE		(1 << 3)
 
-#ifdef CONFIG_VT
-struct screen_info screen_info;
-#endif
+struct screen_info screen_info __section(".data");
 
 unsigned long fw_arg0, fw_arg1;
 DEFINE_PER_CPU(unsigned long, kernelsp);
@@ -122,16 +120,9 @@ static void __init parse_cpu_table(const struct dmi_header *dm)
 
 static void __init parse_bios_table(const struct dmi_header *dm)
 {
-	int bios_extern;
 	char *dmi_data = (char *)dm;
 
-	bios_extern = *(dmi_data + SMBIOS_BIOSEXTERN_OFFSET);
 	b_info.bios_size = (*(dmi_data + SMBIOS_BIOSSIZE_OFFSET) + 1) << 6;
-
-	if (bios_extern & LOONGSON_EFI_ENABLE)
-		set_bit(EFI_BOOT, &efi.flags);
-	else
-		clear_bit(EFI_BOOT, &efi.flags);
 }
 
 static void __init find_tokens(const struct dmi_header *dm, void *dummy)
@@ -145,6 +136,7 @@ static void __init find_tokens(const struct dmi_header *dm, void *dummy)
 		break;
 	}
 }
+
 static void __init smbios_parse(void)
 {
 	b_info.bios_vendor = (void *)dmi_get_system_info(DMI_BIOS_VENDOR);
diff --git a/arch/loongarch/kernel/vmlinux.lds.S b/arch/loongarch/kernel/vmlinux.lds.S
index 69c76f26c1c5..36d042739f3c 100644
--- a/arch/loongarch/kernel/vmlinux.lds.S
+++ b/arch/loongarch/kernel/vmlinux.lds.S
@@ -12,6 +12,7 @@
 #define BSS_FIRST_SECTIONS *(.bss..swapper_pg_dir)
 
 #include <asm-generic/vmlinux.lds.h>
+#include "image-vars.h"
 
 /*
  * Max avaliable Page Size is 64K, so we set SectionAlignment
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 6cb7384ad2ac..cbf1c55dc224 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -107,7 +107,7 @@ config EFI_GENERIC_STUB
 
 config EFI_ARMSTUB_DTB_LOADER
 	bool "Enable the DTB loader"
-	depends on EFI_GENERIC_STUB && !RISCV
+	depends on EFI_GENERIC_STUB && !RISCV && !LOONGARCH
 	default y
 	help
 	  Select this config option to add support for the dtb= command
@@ -124,7 +124,7 @@ config EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER
 	bool "Enable the command line initrd loader" if !X86
 	depends on EFI_STUB && (EFI_GENERIC_STUB || X86)
 	default y if X86
-	depends on !RISCV
+	depends on !RISCV && !LOONGARCH
 	help
 	  Select this config option to add support for the initrd= command
 	  line parameter, allowing an initrd that resides on the same volume
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index d0537573501e..1588c61939e7 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -26,6 +26,8 @@ cflags-$(CONFIG_ARM)		:= $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \
 				   $(call cc-option,-mno-single-pic-base)
 cflags-$(CONFIG_RISCV)		:= $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \
 				   -fpic
+cflags-$(CONFIG_LOONGARCH)	:= $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) \
+				   -fpic
 
 cflags-$(CONFIG_EFI_GENERIC_STUB) += -I$(srctree)/scripts/dtc/libfdt
 
@@ -70,6 +72,8 @@ lib-$(CONFIG_ARM)		+= arm32-stub.o
 lib-$(CONFIG_ARM64)		+= arm64-stub.o
 lib-$(CONFIG_X86)		+= x86-stub.o
 lib-$(CONFIG_RISCV)		+= riscv-stub.o
+lib-$(CONFIG_LOONGARCH)		+= loongarch-stub.o
+
 CFLAGS_arm32-stub.o		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
 
 # Even when -mbranch-protection=none is set, Clang will generate a
@@ -125,6 +129,12 @@ STUBCOPY_FLAGS-$(CONFIG_RISCV)	+= --prefix-alloc-sections=.init \
 				   --prefix-symbols=__efistub_
 STUBCOPY_RELOC-$(CONFIG_RISCV)	:= R_RISCV_HI20
 
+# For LoongArch, keep all the symbols in .init section and make sure that no
+# absolute symbols references doesn't exist.
+STUBCOPY_FLAGS-$(CONFIG_LOONGARCH)	+= --prefix-alloc-sections=.init \
+					   --prefix-symbols=__efistub_
+STUBCOPY_RELOC-$(CONFIG_LOONGARCH)	:= R_LARCH_MARK_LA
+
 $(obj)/%.stub.o: $(obj)/%.o FORCE
 	$(call if_changed,stubcopy)
 
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index f515394cce6e..90f1335bc46e 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -40,14 +40,18 @@
 
 #ifdef CONFIG_ARM64
 # define EFI_RT_VIRTUAL_LIMIT	DEFAULT_MAP_WINDOW_64
-#elif defined(CONFIG_RISCV)
+#elif defined(CONFIG_RISCV) || defined(CONFIG_LOONGARCH)
 # define EFI_RT_VIRTUAL_LIMIT	TASK_SIZE_MIN
-#else
+#else /* Only if TASK_SIZE is a constant */
 # define EFI_RT_VIRTUAL_LIMIT	TASK_SIZE
 #endif
 
+#ifndef EFI_RT_VIRTUAL_OFFSET
+#define EFI_RT_VIRTUAL_OFFSET	0
+#endif
+
 static u64 virtmap_base = EFI_RT_VIRTUAL_BASE;
-static bool flat_va_mapping;
+static bool flat_va_mapping = !!EFI_RT_VIRTUAL_OFFSET;
 
 const efi_system_table_t *efi_system_table;
 
@@ -254,8 +258,8 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
 	 * The easiest way to achieve that is to simply use a 1:1 mapping.
 	 */
 	prop_tbl = get_efi_config_table(EFI_PROPERTIES_TABLE_GUID);
-	flat_va_mapping = prop_tbl &&
-			  (prop_tbl->memory_protection_attribute &
+	flat_va_mapping |= prop_tbl &&
+			   (prop_tbl->memory_protection_attribute &
 			   EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA);
 
 	/* force efi_novamap if SetVirtualAddressMap() is unsupported */
@@ -338,7 +342,7 @@ void efi_get_virtmap(efi_memory_desc_t *memory_map, unsigned long map_size,
 		paddr = in->phys_addr;
 		size = in->num_pages * EFI_PAGE_SIZE;
 
-		in->virt_addr = in->phys_addr;
+		in->virt_addr = in->phys_addr + EFI_RT_VIRTUAL_OFFSET;
 		if (efi_novamap) {
 			continue;
 		}
diff --git a/drivers/firmware/efi/libstub/loongarch-stub.c b/drivers/firmware/efi/libstub/loongarch-stub.c
new file mode 100644
index 000000000000..b7ef8d2df59e
--- /dev/null
+++ b/drivers/firmware/efi/libstub/loongarch-stub.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Author: Yun Liu <liuyun@loongson.cn>
+ *         Huacai Chen <chenhuacai@loongson.cn>
+ * Copyright (C) 2020-2022 Loongson Technology Corporation Limited
+ */
+
+#include <asm/efi.h>
+#include <asm/addrspace.h>
+#include "efistub.h"
+
+typedef void __noreturn (*kernel_entry_t)(bool efi, unsigned long fdt);
+
+extern int kernel_asize;
+extern int kernel_fsize;
+extern int kernel_offset;
+extern kernel_entry_t kernel_entry;
+
+efi_status_t check_platform_features(void)
+{
+	return EFI_SUCCESS;
+}
+
+efi_status_t handle_kernel_image(unsigned long *image_addr,
+				 unsigned long *image_size,
+				 unsigned long *reserve_addr,
+				 unsigned long *reserve_size,
+				 efi_loaded_image_t *image,
+				 efi_handle_t image_handle)
+{
+	efi_status_t status;
+	unsigned long kernel_addr = 0;
+
+	kernel_addr = (unsigned long)&kernel_offset - kernel_offset;
+
+	status = efi_relocate_kernel(&kernel_addr, kernel_fsize, kernel_asize,
+				     PHYSADDR(VMLINUX_LOAD_ADDRESS), SZ_2M, 0x0);
+
+	*image_addr = kernel_addr;
+	*image_size = kernel_asize;
+
+	return status;
+}
+
+void __noreturn efi_enter_kernel(unsigned long entrypoint, unsigned long fdt, unsigned long fdt_size)
+{
+	kernel_entry_t real_kernel_entry;
+
+	/* Config Direct Mapping */
+	csr_write64(CSR_DMW0_INIT, LOONGARCH_CSR_DMWIN0);
+	csr_write64(CSR_DMW1_INIT, LOONGARCH_CSR_DMWIN1);
+
+	real_kernel_entry = (kernel_entry_t)
+		((unsigned long)&kernel_entry - entrypoint + VMLINUX_LOAD_ADDRESS);
+
+	if (!efi_novamap)
+		real_kernel_entry(true, fdt);
+	else
+		real_kernel_entry(false, fdt);
+}
diff --git a/include/linux/pe.h b/include/linux/pe.h
index daf09ffffe38..1d3836ef9d92 100644
--- a/include/linux/pe.h
+++ b/include/linux/pe.h
@@ -65,6 +65,8 @@
 #define	IMAGE_FILE_MACHINE_SH5		0x01a8
 #define	IMAGE_FILE_MACHINE_THUMB	0x01c2
 #define	IMAGE_FILE_MACHINE_WCEMIPSV2	0x0169
+#define	IMAGE_FILE_MACHINE_LOONGARCH32	0x6232
+#define	IMAGE_FILE_MACHINE_LOONGARCH64	0x6264
 
 /* flags */
 #define IMAGE_FILE_RELOCS_STRIPPED           0x0001
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-08-19 10:20 [PATCH V3] LoongArch: Add efistub booting support Huacai Chen
@ 2022-08-22 10:44 ` Ard Biesheuvel
  2022-08-22 18:03   ` Ard Biesheuvel
  2022-08-23  2:04   ` Huacai Chen
  2022-08-27  4:41 ` Xi Ruoyao
  1 sibling, 2 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2022-08-22 10:44 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Arnd Bergmann, Huacai Chen, loongarch, linux-arch, Xuefeng Li,
	Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi, linux-kernel,
	Xi Ruoyao

On Fri, 19 Aug 2022 at 12:20, Huacai Chen <chenhuacai@loongson.cn> wrote:
>
> This patch adds efistub booting support, which is the standard UEFI boot
> protocol for us to use.
>
> We use generic efistub, which means we can pass boot information (i.e.,
> system table, memory map, kernel command line, initrd) via a light FDT
> and drop a lot of non-standard code.
>
> We use a flat mapping to map the efi runtime in the kernel's address
> space. In efi, VA = PA; in kernel, VA = PA + PAGE_OFFSET. As a result,
> flat mapping is not identity mapping, SetVirtualAddressMap() is still
> needed for the efi runtime.
>
> Tested-by: Xi Ruoyao <xry111@xry111.site>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> V1 --> V2:
> 1, Call SetVirtualAddressMap() in stub;
> 2, Use core kernel data directly in alloc_screen_info();
> 3, Remove the magic number in MS-DOS header;
> 4, Disable EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER;
> 5, Some other small changes suggested by Ard Biesheuvel.
>
> V2 --> V3:
> 1, Adjust Makefile to adapt zboot;
> 2, Introduce EFI_RT_VIRTUAL_OFFSET instead of changing flat_va_mapping.
>

Thanks for the update.

I am going to queue this up in the efi/next tree. However, due to the
many changes to arch/loongarch in this patch, conflicts are not
unlikely, so I created a signed stable tag for the patch that you can
merge into the loongarch arch tree if you want.

*However*, you must *not* rebase your tree after merging this tag.
Therefore, it is probably best that the merge of this tag appears as
the very first change on your PR to Linus for v6.1. Everything after
can be rebased at will (assuming there are no other impediments to
doing so)

You can fetch it and merge it like so:

git fetch -t git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
git verify-tag efi-loongarch-for-v6.1 # if you like
git merge efi-loongarch-for-v6.1

and all your other v6.1 changes can go on top.

This way, you can resolve conflicts locally without affecting the EFI
changes going via the other tree. The EFI stub for LoongArch change
will arrive into Linus's tree via whichever tree he pulls first: the
LoongArch one or the EFI one.

I will rebase my zboot decompressor changes on top of this - I will cc
you again, as the LoongArch builds ok but still does not boot.

Thanks,
Ard.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-08-22 10:44 ` Ard Biesheuvel
@ 2022-08-22 18:03   ` Ard Biesheuvel
  2022-08-23  3:11     ` Huacai Chen
  2022-08-23  2:04   ` Huacai Chen
  1 sibling, 1 reply; 26+ messages in thread
From: Ard Biesheuvel @ 2022-08-22 18:03 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Arnd Bergmann, Huacai Chen, loongarch, linux-arch, Xuefeng Li,
	Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi, linux-kernel,
	Xi Ruoyao

On Mon, 22 Aug 2022 at 12:44, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 19 Aug 2022 at 12:20, Huacai Chen <chenhuacai@loongson.cn> wrote:
> >
> > This patch adds efistub booting support, which is the standard UEFI boot
> > protocol for us to use.
> >
> > We use generic efistub, which means we can pass boot information (i.e.,
> > system table, memory map, kernel command line, initrd) via a light FDT
> > and drop a lot of non-standard code.
> >
> > We use a flat mapping to map the efi runtime in the kernel's address
> > space. In efi, VA = PA; in kernel, VA = PA + PAGE_OFFSET. As a result,
> > flat mapping is not identity mapping, SetVirtualAddressMap() is still
> > needed for the efi runtime.
> >
> > Tested-by: Xi Ruoyao <xry111@xry111.site>
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> > V1 --> V2:
> > 1, Call SetVirtualAddressMap() in stub;
> > 2, Use core kernel data directly in alloc_screen_info();
> > 3, Remove the magic number in MS-DOS header;
> > 4, Disable EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER;
> > 5, Some other small changes suggested by Ard Biesheuvel.
> >
> > V2 --> V3:
> > 1, Adjust Makefile to adapt zboot;
> > 2, Introduce EFI_RT_VIRTUAL_OFFSET instead of changing flat_va_mapping.
> >
>
> Thanks for the update.
>
> I am going to queue this up in the efi/next tree. However, due to the
> many changes to arch/loongarch in this patch, conflicts are not
> unlikely, so I created a signed stable tag for the patch that you can
> merge into the loongarch arch tree if you want.
>
> *However*, you must *not* rebase your tree after merging this tag.
> Therefore, it is probably best that the merge of this tag appears as
> the very first change on your PR to Linus for v6.1. Everything after
> can be rebased at will (assuming there are no other impediments to
> doing so)
>
> You can fetch it and merge it like so:
>
> git fetch -t git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> git verify-tag efi-loongarch-for-v6.1 # if you like
> git merge efi-loongarch-for-v6.1
>
> and all your other v6.1 changes can go on top.
>
> This way, you can resolve conflicts locally without affecting the EFI
> changes going via the other tree. The EFI stub for LoongArch change
> will arrive into Linus's tree via whichever tree he pulls first: the
> LoongArch one or the EFI one.
>
> I will rebase my zboot decompressor changes on top of this - I will cc
> you again, as the LoongArch builds ok but still does not boot.
>

I have pushed a branch here that includes EFI decompressor support for LoongArch

https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4

You will need to enable CONFIG_EFI_ZBOOT and build the zImage.efi
target. The resulting image should be bootable jus tlike the
vmlinux.efi but for some reason, it produces the crash I reported
earlier.

Please give it a try, and if you manage to figure out what's wrong
with my code, please let me know :-)

Thanks,
Ard.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-08-22 10:44 ` Ard Biesheuvel
  2022-08-22 18:03   ` Ard Biesheuvel
@ 2022-08-23  2:04   ` Huacai Chen
  1 sibling, 0 replies; 26+ messages in thread
From: Huacai Chen @ 2022-08-23  2:04 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Huacai Chen, Arnd Bergmann, loongarch, linux-arch, Xuefeng Li,
	Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi, LKML, Xi Ruoyao

Hi, Ard,

On Mon, Aug 22, 2022 at 6:44 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 19 Aug 2022 at 12:20, Huacai Chen <chenhuacai@loongson.cn> wrote:
> >
> > This patch adds efistub booting support, which is the standard UEFI boot
> > protocol for us to use.
> >
> > We use generic efistub, which means we can pass boot information (i.e.,
> > system table, memory map, kernel command line, initrd) via a light FDT
> > and drop a lot of non-standard code.
> >
> > We use a flat mapping to map the efi runtime in the kernel's address
> > space. In efi, VA = PA; in kernel, VA = PA + PAGE_OFFSET. As a result,
> > flat mapping is not identity mapping, SetVirtualAddressMap() is still
> > needed for the efi runtime.
> >
> > Tested-by: Xi Ruoyao <xry111@xry111.site>
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> > V1 --> V2:
> > 1, Call SetVirtualAddressMap() in stub;
> > 2, Use core kernel data directly in alloc_screen_info();
> > 3, Remove the magic number in MS-DOS header;
> > 4, Disable EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER;
> > 5, Some other small changes suggested by Ard Biesheuvel.
> >
> > V2 --> V3:
> > 1, Adjust Makefile to adapt zboot;
> > 2, Introduce EFI_RT_VIRTUAL_OFFSET instead of changing flat_va_mapping.
> >
>
> Thanks for the update.
>
> I am going to queue this up in the efi/next tree. However, due to the
> many changes to arch/loongarch in this patch, conflicts are not
> unlikely, so I created a signed stable tag for the patch that you can
> merge into the loongarch arch tree if you want.
>
> *However*, you must *not* rebase your tree after merging this tag.
> Therefore, it is probably best that the merge of this tag appears as
> the very first change on your PR to Linus for v6.1. Everything after
> can be rebased at will (assuming there are no other impediments to
> doing so)
>
> You can fetch it and merge it like so:
>
> git fetch -t git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> git verify-tag efi-loongarch-for-v6.1 # if you like
> git merge efi-loongarch-for-v6.1
>
> and all your other v6.1 changes can go on top.
>
> This way, you can resolve conflicts locally without affecting the EFI
> changes going via the other tree. The EFI stub for LoongArch change
> will arrive into Linus's tree via whichever tree he pulls first: the
> LoongArch one or the EFI one.
>
> I will rebase my zboot decompressor changes on top of this - I will cc
> you again, as the LoongArch builds ok but still does not boot.
Thank you very much. There are several bugs that need to be fixed
first for the 6.0 cycle, I will merge the efi-loongarch-for-v6.1 tag
after that.

Huacai
>
> Thanks,
> Ard.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-08-22 18:03   ` Ard Biesheuvel
@ 2022-08-23  3:11     ` Huacai Chen
       [not found]       ` <CAAhV-H7-JGGjcFBenpwUz1itZvFK9f1pH88b1dcRtPcGKToojA@mail.gmail.com>
  0 siblings, 1 reply; 26+ messages in thread
From: Huacai Chen @ 2022-08-23  3:11 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Huacai Chen, Arnd Bergmann, loongarch, linux-arch, Xuefeng Li,
	Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi, LKML, Xi Ruoyao

Hi, Ard,

On Tue, Aug 23, 2022 at 2:03 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 22 Aug 2022 at 12:44, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 19 Aug 2022 at 12:20, Huacai Chen <chenhuacai@loongson.cn> wrote:
> > >
> > > This patch adds efistub booting support, which is the standard UEFI boot
> > > protocol for us to use.
> > >
> > > We use generic efistub, which means we can pass boot information (i.e.,
> > > system table, memory map, kernel command line, initrd) via a light FDT
> > > and drop a lot of non-standard code.
> > >
> > > We use a flat mapping to map the efi runtime in the kernel's address
> > > space. In efi, VA = PA; in kernel, VA = PA + PAGE_OFFSET. As a result,
> > > flat mapping is not identity mapping, SetVirtualAddressMap() is still
> > > needed for the efi runtime.
> > >
> > > Tested-by: Xi Ruoyao <xry111@xry111.site>
> > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > > ---
> > > V1 --> V2:
> > > 1, Call SetVirtualAddressMap() in stub;
> > > 2, Use core kernel data directly in alloc_screen_info();
> > > 3, Remove the magic number in MS-DOS header;
> > > 4, Disable EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER;
> > > 5, Some other small changes suggested by Ard Biesheuvel.
> > >
> > > V2 --> V3:
> > > 1, Adjust Makefile to adapt zboot;
> > > 2, Introduce EFI_RT_VIRTUAL_OFFSET instead of changing flat_va_mapping.
> > >
> >
> > Thanks for the update.
> >
> > I am going to queue this up in the efi/next tree. However, due to the
> > many changes to arch/loongarch in this patch, conflicts are not
> > unlikely, so I created a signed stable tag for the patch that you can
> > merge into the loongarch arch tree if you want.
> >
> > *However*, you must *not* rebase your tree after merging this tag.
> > Therefore, it is probably best that the merge of this tag appears as
> > the very first change on your PR to Linus for v6.1. Everything after
> > can be rebased at will (assuming there are no other impediments to
> > doing so)
> >
> > You can fetch it and merge it like so:
> >
> > git fetch -t git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> > git verify-tag efi-loongarch-for-v6.1 # if you like
> > git merge efi-loongarch-for-v6.1
> >
> > and all your other v6.1 changes can go on top.
> >
> > This way, you can resolve conflicts locally without affecting the EFI
> > changes going via the other tree. The EFI stub for LoongArch change
> > will arrive into Linus's tree via whichever tree he pulls first: the
> > LoongArch one or the EFI one.
> >
> > I will rebase my zboot decompressor changes on top of this - I will cc
> > you again, as the LoongArch builds ok but still does not boot.
> >
>
> I have pushed a branch here that includes EFI decompressor support for LoongArch
>
> https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
>
> You will need to enable CONFIG_EFI_ZBOOT and build the zImage.efi
> target. The resulting image should be bootable jus tlike the
> vmlinux.efi but for some reason, it produces the crash I reported
> earlier.
>
> Please give it a try, and if you manage to figure out what's wrong
> with my code, please let me know :-)
I will try zboot on my real machine. For the code, I prefer
vmlinuz.efi rather than zImage.efi for LoongArch since it keeps the
naming consistency.

Huacai
>
> Thanks,
> Ard.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
       [not found]             ` <CAMj1kXF8u=M6R_9pCciY+5oWj6VbLcMEFFC=JYQm5aq1-c8eRg@mail.gmail.com>
@ 2022-08-25  9:47               ` Huacai Chen
  2022-08-25  9:51                 ` Ard Biesheuvel
  0 siblings, 1 reply; 26+ messages in thread
From: Huacai Chen @ 2022-08-25  9:47 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Huacai Chen, Arnd Bergmann, Xuefeng Li, Xuerui Wang, Xi Ruoyao,
	Jiaxun Yang, loongarch

Hi, Ard,

On Wed, Aug 24, 2022 at 9:28 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 24 Aug 2022 at 14:43, Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > Hi, Ard,
> >
> > On Wed, Aug 24, 2022 at 6:26 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Tue, 23 Aug 2022 at 05:15, Huacai Chen <chenhuacai@kernel.org> wrote:
> > > >
> > > > On Tue, Aug 23, 2022 at 11:11 AM Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > >
> > > > > Hi, Ard,
> > > > >
> > > > > On Tue, Aug 23, 2022 at 2:03 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > >
> > > > > > On Mon, 22 Aug 2022 at 12:44, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > >
> > > > > > > On Fri, 19 Aug 2022 at 12:20, Huacai Chen <chenhuacai@loongson.cn> wrote:
> > > > > > > >
> > > > > > > > This patch adds efistub booting support, which is the standard UEFI boot
> > > > > > > > protocol for us to use.
> > > > > > > >
> > > > > > > > We use generic efistub, which means we can pass boot information (i.e.,
> > > > > > > > system table, memory map, kernel command line, initrd) via a light FDT
> > > > > > > > and drop a lot of non-standard code.
> > > > > > > >
> > > > > > > > We use a flat mapping to map the efi runtime in the kernel's address
> > > > > > > > space. In efi, VA = PA; in kernel, VA = PA + PAGE_OFFSET. As a result,
> > > > > > > > flat mapping is not identity mapping, SetVirtualAddressMap() is still
> > > > > > > > needed for the efi runtime.
> > > > > > > >
> > > > > > > > Tested-by: Xi Ruoyao <xry111@xry111.site>
> > > > > > > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > > > > > > > ---
> > > > > > > > V1 --> V2:
> > > > > > > > 1, Call SetVirtualAddressMap() in stub;
> > > > > > > > 2, Use core kernel data directly in alloc_screen_info();
> > > > > > > > 3, Remove the magic number in MS-DOS header;
> > > > > > > > 4, Disable EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER;
> > > > > > > > 5, Some other small changes suggested by Ard Biesheuvel.
> > > > > > > >
> > > > > > > > V2 --> V3:
> > > > > > > > 1, Adjust Makefile to adapt zboot;
> > > > > > > > 2, Introduce EFI_RT_VIRTUAL_OFFSET instead of changing flat_va_mapping.
> > > > > > > >
> > > > > > >
> > > > > > > Thanks for the update.
> > > > > > >
> > > > > > > I am going to queue this up in the efi/next tree. However, due to the
> > > > > > > many changes to arch/loongarch in this patch, conflicts are not
> > > > > > > unlikely, so I created a signed stable tag for the patch that you can
> > > > > > > merge into the loongarch arch tree if you want.
> > > > > > >
> > > > > > > *However*, you must *not* rebase your tree after merging this tag.
> > > > > > > Therefore, it is probably best that the merge of this tag appears as
> > > > > > > the very first change on your PR to Linus for v6.1. Everything after
> > > > > > > can be rebased at will (assuming there are no other impediments to
> > > > > > > doing so)
> > > > > > >
> > > > > > > You can fetch it and merge it like so:
> > > > > > >
> > > > > > > git fetch -t git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> > > > > > > git verify-tag efi-loongarch-for-v6.1 # if you like
> > > > > > > git merge efi-loongarch-for-v6.1
> > > > > > >
> > > > > > > and all your other v6.1 changes can go on top.
> > > > > > >
> > > > > > > This way, you can resolve conflicts locally without affecting the EFI
> > > > > > > changes going via the other tree. The EFI stub for LoongArch change
> > > > > > > will arrive into Linus's tree via whichever tree he pulls first: the
> > > > > > > LoongArch one or the EFI one.
> > > > > > >
> > > > > > > I will rebase my zboot decompressor changes on top of this - I will cc
> > > > > > > you again, as the LoongArch builds ok but still does not boot.
> > > > > > >
> > > > > >
> > > > > > I have pushed a branch here that includes EFI decompressor support for LoongArch
> > > > > >
> > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
> > > > > >
> > > > > > You will need to enable CONFIG_EFI_ZBOOT and build the zImage.efi
> > > > > > target. The resulting image should be bootable jus tlike the
> > > > > > vmlinux.efi but for some reason, it produces the crash I reported
> > > > > > earlier.
> > > > > >
> > > > > > Please give it a try, and if you manage to figure out what's wrong
> > > > > > with my code, please let me know :-)
> > > > > I will try zboot on my real machine. For the code, I prefer
> > > > > vmlinuz.efi rather than zImage.efi for LoongArch since it keeps the
> > > > > naming consistency.
> > >
> > > OK, I will rename it to vmlinuz.efi for all architectures then.
> > I found RISC-V and ARM64 use Image for the normal kernel, so zImage
> > may be a suitable name for them, can we use a variable for the
> > compressed kernel? For example, use KBUILD_IMAGE directly, then we
> > don't need to use the same name for all architectures.
> >
>
> No. This is supposed to be a generic EFI boot method, and using
> different names for the same thing on different architectures
> interferes with that.
We have already used a EFI_ZBOOT_PAYLOAD which is different across
architectures, so using another KBUILD_IMAGE may have no problem? This
can also let "make" rather than "make zImage.efi" to build the zboot
kernel.

>
> > And I have tried your zboot series on a real machine, it has the same
> > problem as you tried in QEMU. I will try to find the root cause with
> > my colleagues.
> >
>
> Yes please.
We made the following changes and it works on LoongArch. But we don't
know why the existing method didn't work. Our guess is that the linker
calculates the address incorrectly.

diff --git a/drivers/firmware/efi/libstub/zboot.c
b/drivers/firmware/efi/libstub/zboot.c
index f8eb4c7b41d1..ce3bd1affa1a 100644
--- a/drivers/firmware/efi/libstub/zboot.c
+++ b/drivers/firmware/efi/libstub/zboot.c
@@ -29,7 +29,6 @@ static unsigned long free_mem_ptr, free_mem_end_ptr;
 #endif

 extern char _gzdata_start[], _gzdata_end[];
-extern u32 uncompressed_size __aligned(1);

 static void log(efi_char16_t str[])
 {
@@ -53,6 +52,7 @@ efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
        unsigned long image_buffer;
        efi_handle_t child_handle;
        efi_char16_t *exit_data;
+       u32 uncompressed_size;
        efi_status_t status;
        int ret;

@@ -68,6 +68,9 @@ efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
                return status;
        }

+       // The uncompressed size is appended at the end of the image
+       uncompressed_size = *(u32 *)(_gzdata_end - 4);
+
        status = efi_allocate_pages(uncompressed_size, &image_buffer,
ULONG_MAX);
        if (status != EFI_SUCCESS) {
                log(L"Failed to allocate memory\n");
diff --git a/drivers/firmware/efi/libstub/zboot.lds
b/drivers/firmware/efi/libstub/zboot.lds
index d6ba89a0c294..7a1059a8a47b 100644
--- a/drivers/firmware/efi/libstub/zboot.lds
+++ b/drivers/firmware/efi/libstub/zboot.lds
@@ -33,7 +33,5 @@ SECTIONS

 PROVIDE(__efistub__gzdata_size = ABSOLUTE(. - __efistub__gzdata_start));

-PROVIDE(__efistub_uncompressed_size = __efistub__gzdata_end - 4);
-
 PROVIDE(__data_rawsize = ABSOLUTE(_edata - _etext));
 PROVIDE(__data_size = ABSOLUTE(_end - _etext));

Huacai

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-08-25  9:47               ` Huacai Chen
@ 2022-08-25  9:51                 ` Ard Biesheuvel
  2022-08-25 10:13                   ` Feiyang Chen
  2022-08-25 11:54                   ` Huacai Chen
  0 siblings, 2 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2022-08-25  9:51 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Huacai Chen, Arnd Bergmann, Xuefeng Li, Xuerui Wang, Xi Ruoyao,
	Jiaxun Yang, loongarch

On Thu, 25 Aug 2022 at 11:47, Huacai Chen <chenhuacai@kernel.org> wrote:
>
> Hi, Ard,
>
> On Wed, Aug 24, 2022 at 9:28 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Wed, 24 Aug 2022 at 14:43, Huacai Chen <chenhuacai@kernel.org> wrote:
> > >
> > > Hi, Ard,
> > >
> > > On Wed, Aug 24, 2022 at 6:26 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > On Tue, 23 Aug 2022 at 05:15, Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > >
> > > > > On Tue, Aug 23, 2022 at 11:11 AM Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > > >
> > > > > > Hi, Ard,
> > > > > >
> > > > > > On Tue, Aug 23, 2022 at 2:03 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > >
> > > > > > > On Mon, 22 Aug 2022 at 12:44, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > > >
> > > > > > > > On Fri, 19 Aug 2022 at 12:20, Huacai Chen <chenhuacai@loongson.cn> wrote:
> > > > > > > > >
> > > > > > > > > This patch adds efistub booting support, which is the standard UEFI boot
> > > > > > > > > protocol for us to use.
> > > > > > > > >
> > > > > > > > > We use generic efistub, which means we can pass boot information (i.e.,
> > > > > > > > > system table, memory map, kernel command line, initrd) via a light FDT
> > > > > > > > > and drop a lot of non-standard code.
> > > > > > > > >
> > > > > > > > > We use a flat mapping to map the efi runtime in the kernel's address
> > > > > > > > > space. In efi, VA = PA; in kernel, VA = PA + PAGE_OFFSET. As a result,
> > > > > > > > > flat mapping is not identity mapping, SetVirtualAddressMap() is still
> > > > > > > > > needed for the efi runtime.
> > > > > > > > >
> > > > > > > > > Tested-by: Xi Ruoyao <xry111@xry111.site>
> > > > > > > > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > > > > > > > > ---
> > > > > > > > > V1 --> V2:
> > > > > > > > > 1, Call SetVirtualAddressMap() in stub;
> > > > > > > > > 2, Use core kernel data directly in alloc_screen_info();
> > > > > > > > > 3, Remove the magic number in MS-DOS header;
> > > > > > > > > 4, Disable EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER;
> > > > > > > > > 5, Some other small changes suggested by Ard Biesheuvel.
> > > > > > > > >
> > > > > > > > > V2 --> V3:
> > > > > > > > > 1, Adjust Makefile to adapt zboot;
> > > > > > > > > 2, Introduce EFI_RT_VIRTUAL_OFFSET instead of changing flat_va_mapping.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Thanks for the update.
> > > > > > > >
> > > > > > > > I am going to queue this up in the efi/next tree. However, due to the
> > > > > > > > many changes to arch/loongarch in this patch, conflicts are not
> > > > > > > > unlikely, so I created a signed stable tag for the patch that you can
> > > > > > > > merge into the loongarch arch tree if you want.
> > > > > > > >
> > > > > > > > *However*, you must *not* rebase your tree after merging this tag.
> > > > > > > > Therefore, it is probably best that the merge of this tag appears as
> > > > > > > > the very first change on your PR to Linus for v6.1. Everything after
> > > > > > > > can be rebased at will (assuming there are no other impediments to
> > > > > > > > doing so)
> > > > > > > >
> > > > > > > > You can fetch it and merge it like so:
> > > > > > > >
> > > > > > > > git fetch -t git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> > > > > > > > git verify-tag efi-loongarch-for-v6.1 # if you like
> > > > > > > > git merge efi-loongarch-for-v6.1
> > > > > > > >
> > > > > > > > and all your other v6.1 changes can go on top.
> > > > > > > >
> > > > > > > > This way, you can resolve conflicts locally without affecting the EFI
> > > > > > > > changes going via the other tree. The EFI stub for LoongArch change
> > > > > > > > will arrive into Linus's tree via whichever tree he pulls first: the
> > > > > > > > LoongArch one or the EFI one.
> > > > > > > >
> > > > > > > > I will rebase my zboot decompressor changes on top of this - I will cc
> > > > > > > > you again, as the LoongArch builds ok but still does not boot.
> > > > > > > >
> > > > > > >
> > > > > > > I have pushed a branch here that includes EFI decompressor support for LoongArch
> > > > > > >
> > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
> > > > > > >
> > > > > > > You will need to enable CONFIG_EFI_ZBOOT and build the zImage.efi
> > > > > > > target. The resulting image should be bootable jus tlike the
> > > > > > > vmlinux.efi but for some reason, it produces the crash I reported
> > > > > > > earlier.
> > > > > > >
> > > > > > > Please give it a try, and if you manage to figure out what's wrong
> > > > > > > with my code, please let me know :-)
> > > > > > I will try zboot on my real machine. For the code, I prefer
> > > > > > vmlinuz.efi rather than zImage.efi for LoongArch since it keeps the
> > > > > > naming consistency.
> > > >
> > > > OK, I will rename it to vmlinuz.efi for all architectures then.
> > > I found RISC-V and ARM64 use Image for the normal kernel, so zImage
> > > may be a suitable name for them, can we use a variable for the
> > > compressed kernel? For example, use KBUILD_IMAGE directly, then we
> > > don't need to use the same name for all architectures.
> > >
> >
> > No. This is supposed to be a generic EFI boot method, and using
> > different names for the same thing on different architectures
> > interferes with that.
> We have already used a EFI_ZBOOT_PAYLOAD which is different across
> architectures, so using another KBUILD_IMAGE may have no problem? This
> can also let "make" rather than "make zImage.efi" to build the zboot
> kernel.
>

This is not about how we define KBUILD_IMAGE internally. If you prefer
to build the zboot kernel by default, we can set KBUILD_IMAGE to
vmlinuz.efi for LoongArch if CONFIG_EFI_ZBOOT=y.

What I was referring to is external consumers: other projects such as
GRUB, systemd-boot or the distros. Using zImage.efi on some
architectures and vmlinuz.efi on other ones only creates confusion, so
we should stick to a single one. I don't mind switching the other
architectures to vmlinuz.efi as well.

> >
> > > And I have tried your zboot series on a real machine, it has the same
> > > problem as you tried in QEMU. I will try to find the root cause with
> > > my colleagues.
> > >
> >
> > Yes please.
> We made the following changes and it works on LoongArch. But we don't
> know why the existing method didn't work. Our guess is that the linker
> calculates the address incorrectly.
>

Could it be due to the misalignment? Could this be a toolchain issue
with __aligned(1)?


> diff --git a/drivers/firmware/efi/libstub/zboot.c
> b/drivers/firmware/efi/libstub/zboot.c
> index f8eb4c7b41d1..ce3bd1affa1a 100644
> --- a/drivers/firmware/efi/libstub/zboot.c
> +++ b/drivers/firmware/efi/libstub/zboot.c
> @@ -29,7 +29,6 @@ static unsigned long free_mem_ptr, free_mem_end_ptr;
>  #endif
>
>  extern char _gzdata_start[], _gzdata_end[];
> -extern u32 uncompressed_size __aligned(1);
>
>  static void log(efi_char16_t str[])
>  {
> @@ -53,6 +52,7 @@ efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
>         unsigned long image_buffer;
>         efi_handle_t child_handle;
>         efi_char16_t *exit_data;
> +       u32 uncompressed_size;
>         efi_status_t status;
>         int ret;
>
> @@ -68,6 +68,9 @@ efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
>                 return status;
>         }
>
> +       // The uncompressed size is appended at the end of the image
> +       uncompressed_size = *(u32 *)(_gzdata_end - 4);
> +
>         status = efi_allocate_pages(uncompressed_size, &image_buffer,
> ULONG_MAX);
>         if (status != EFI_SUCCESS) {
>                 log(L"Failed to allocate memory\n");
> diff --git a/drivers/firmware/efi/libstub/zboot.lds
> b/drivers/firmware/efi/libstub/zboot.lds
> index d6ba89a0c294..7a1059a8a47b 100644
> --- a/drivers/firmware/efi/libstub/zboot.lds
> +++ b/drivers/firmware/efi/libstub/zboot.lds
> @@ -33,7 +33,5 @@ SECTIONS
>
>  PROVIDE(__efistub__gzdata_size = ABSOLUTE(. - __efistub__gzdata_start));
>
> -PROVIDE(__efistub_uncompressed_size = __efistub__gzdata_end - 4);
> -
>  PROVIDE(__data_rawsize = ABSOLUTE(_edata - _etext));
>  PROVIDE(__data_size = ABSOLUTE(_end - _etext));
>
> Huacai

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-08-25  9:51                 ` Ard Biesheuvel
@ 2022-08-25 10:13                   ` Feiyang Chen
  2022-08-25 10:22                     ` Ard Biesheuvel
  2022-08-25 11:54                   ` Huacai Chen
  1 sibling, 1 reply; 26+ messages in thread
From: Feiyang Chen @ 2022-08-25 10:13 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Huacai Chen, Huacai Chen, Arnd Bergmann, Xuefeng Li, Xuerui Wang,
	Xi Ruoyao, Jiaxun Yang, loongarch

On Thu, 25 Aug 2022 at 17:51, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 25 Aug 2022 at 11:47, Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > Hi, Ard,
> >
> > On Wed, Aug 24, 2022 at 9:28 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Wed, 24 Aug 2022 at 14:43, Huacai Chen <chenhuacai@kernel.org> wrote:
> > > >
> > > > Hi, Ard,
> > > >
> > > > On Wed, Aug 24, 2022 at 6:26 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >
> > > > > On Tue, 23 Aug 2022 at 05:15, Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > > >
> > > > > > On Tue, Aug 23, 2022 at 11:11 AM Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > > > >
> > > > > > > Hi, Ard,
> > > > > > >
> > > > > > > On Tue, Aug 23, 2022 at 2:03 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > > >
> > > > > > > > On Mon, 22 Aug 2022 at 12:44, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > > > >
> > > > > > > > > On Fri, 19 Aug 2022 at 12:20, Huacai Chen <chenhuacai@loongson.cn> wrote:
> > > > > > > > > >
> > > > > > > > > > This patch adds efistub booting support, which is the standard UEFI boot
> > > > > > > > > > protocol for us to use.
> > > > > > > > > >
> > > > > > > > > > We use generic efistub, which means we can pass boot information (i.e.,
> > > > > > > > > > system table, memory map, kernel command line, initrd) via a light FDT
> > > > > > > > > > and drop a lot of non-standard code.
> > > > > > > > > >
> > > > > > > > > > We use a flat mapping to map the efi runtime in the kernel's address
> > > > > > > > > > space. In efi, VA = PA; in kernel, VA = PA + PAGE_OFFSET. As a result,
> > > > > > > > > > flat mapping is not identity mapping, SetVirtualAddressMap() is still
> > > > > > > > > > needed for the efi runtime.
> > > > > > > > > >
> > > > > > > > > > Tested-by: Xi Ruoyao <xry111@xry111.site>
> > > > > > > > > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > > > > > > > > > ---
> > > > > > > > > > V1 --> V2:
> > > > > > > > > > 1, Call SetVirtualAddressMap() in stub;
> > > > > > > > > > 2, Use core kernel data directly in alloc_screen_info();
> > > > > > > > > > 3, Remove the magic number in MS-DOS header;
> > > > > > > > > > 4, Disable EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER;
> > > > > > > > > > 5, Some other small changes suggested by Ard Biesheuvel.
> > > > > > > > > >
> > > > > > > > > > V2 --> V3:
> > > > > > > > > > 1, Adjust Makefile to adapt zboot;
> > > > > > > > > > 2, Introduce EFI_RT_VIRTUAL_OFFSET instead of changing flat_va_mapping.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Thanks for the update.
> > > > > > > > >
> > > > > > > > > I am going to queue this up in the efi/next tree. However, due to the
> > > > > > > > > many changes to arch/loongarch in this patch, conflicts are not
> > > > > > > > > unlikely, so I created a signed stable tag for the patch that you can
> > > > > > > > > merge into the loongarch arch tree if you want.
> > > > > > > > >
> > > > > > > > > *However*, you must *not* rebase your tree after merging this tag.
> > > > > > > > > Therefore, it is probably best that the merge of this tag appears as
> > > > > > > > > the very first change on your PR to Linus for v6.1. Everything after
> > > > > > > > > can be rebased at will (assuming there are no other impediments to
> > > > > > > > > doing so)
> > > > > > > > >
> > > > > > > > > You can fetch it and merge it like so:
> > > > > > > > >
> > > > > > > > > git fetch -t git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> > > > > > > > > git verify-tag efi-loongarch-for-v6.1 # if you like
> > > > > > > > > git merge efi-loongarch-for-v6.1
> > > > > > > > >
> > > > > > > > > and all your other v6.1 changes can go on top.
> > > > > > > > >
> > > > > > > > > This way, you can resolve conflicts locally without affecting the EFI
> > > > > > > > > changes going via the other tree. The EFI stub for LoongArch change
> > > > > > > > > will arrive into Linus's tree via whichever tree he pulls first: the
> > > > > > > > > LoongArch one or the EFI one.
> > > > > > > > >
> > > > > > > > > I will rebase my zboot decompressor changes on top of this - I will cc
> > > > > > > > > you again, as the LoongArch builds ok but still does not boot.
> > > > > > > > >
> > > > > > > >
> > > > > > > > I have pushed a branch here that includes EFI decompressor support for LoongArch
> > > > > > > >
> > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
> > > > > > > >
> > > > > > > > You will need to enable CONFIG_EFI_ZBOOT and build the zImage.efi
> > > > > > > > target. The resulting image should be bootable jus tlike the
> > > > > > > > vmlinux.efi but for some reason, it produces the crash I reported
> > > > > > > > earlier.
> > > > > > > >
> > > > > > > > Please give it a try, and if you manage to figure out what's wrong
> > > > > > > > with my code, please let me know :-)
> > > > > > > I will try zboot on my real machine. For the code, I prefer
> > > > > > > vmlinuz.efi rather than zImage.efi for LoongArch since it keeps the
> > > > > > > naming consistency.
> > > > >
> > > > > OK, I will rename it to vmlinuz.efi for all architectures then.
> > > > I found RISC-V and ARM64 use Image for the normal kernel, so zImage
> > > > may be a suitable name for them, can we use a variable for the
> > > > compressed kernel? For example, use KBUILD_IMAGE directly, then we
> > > > don't need to use the same name for all architectures.
> > > >
> > >
> > > No. This is supposed to be a generic EFI boot method, and using
> > > different names for the same thing on different architectures
> > > interferes with that.
> > We have already used a EFI_ZBOOT_PAYLOAD which is different across
> > architectures, so using another KBUILD_IMAGE may have no problem? This
> > can also let "make" rather than "make zImage.efi" to build the zboot
> > kernel.
> >
>
> This is not about how we define KBUILD_IMAGE internally. If you prefer
> to build the zboot kernel by default, we can set KBUILD_IMAGE to
> vmlinuz.efi for LoongArch if CONFIG_EFI_ZBOOT=y.
>
> What I was referring to is external consumers: other projects such as
> GRUB, systemd-boot or the distros. Using zImage.efi on some
> architectures and vmlinuz.efi on other ones only creates confusion, so
> we should stick to a single one. I don't mind switching the other
> architectures to vmlinuz.efi as well.
>
> > >
> > > > And I have tried your zboot series on a real machine, it has the same
> > > > problem as you tried in QEMU. I will try to find the root cause with
> > > > my colleagues.
> > > >
> > >
> > > Yes please.
> > We made the following changes and it works on LoongArch. But we don't
> > know why the existing method didn't work. Our guess is that the linker
> > calculates the address incorrectly.
> >
>
> Could it be due to the misalignment? Could this be a toolchain issue
> with __aligned(1)?
>

Hi, Ard,

We tried to remove __aligned(1), but it doesn't work. We disassembled
zImage.efi.elf and found that uncompressed_size and _gzdata_end are
very far apart.

// s4: uncompressed_size
2f94:   1c012afb        pcaddu12i       $s4, 2391(0x957)
2f98:   02ff837b        addi.d          $s4, $s4, -32(0xfe0)

// s1: _gzdata_end
2fa4:   1c012a98        pcaddu12i       $s1, 2388(0x954)
2fa8:   02c0f318        addi.d          $s1, $s1, 60(0x3c)

Thanks,
Feiyang

>
> > diff --git a/drivers/firmware/efi/libstub/zboot.c
> > b/drivers/firmware/efi/libstub/zboot.c
> > index f8eb4c7b41d1..ce3bd1affa1a 100644
> > --- a/drivers/firmware/efi/libstub/zboot.c
> > +++ b/drivers/firmware/efi/libstub/zboot.c
> > @@ -29,7 +29,6 @@ static unsigned long free_mem_ptr, free_mem_end_ptr;
> >  #endif
> >
> >  extern char _gzdata_start[], _gzdata_end[];
> > -extern u32 uncompressed_size __aligned(1);
> >
> >  static void log(efi_char16_t str[])
> >  {
> > @@ -53,6 +52,7 @@ efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
> >         unsigned long image_buffer;
> >         efi_handle_t child_handle;
> >         efi_char16_t *exit_data;
> > +       u32 uncompressed_size;
> >         efi_status_t status;
> >         int ret;
> >
> > @@ -68,6 +68,9 @@ efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
> >                 return status;
> >         }
> >
> > +       // The uncompressed size is appended at the end of the image
> > +       uncompressed_size = *(u32 *)(_gzdata_end - 4);
> > +
> >         status = efi_allocate_pages(uncompressed_size, &image_buffer,
> > ULONG_MAX);
> >         if (status != EFI_SUCCESS) {
> >                 log(L"Failed to allocate memory\n");
> > diff --git a/drivers/firmware/efi/libstub/zboot.lds
> > b/drivers/firmware/efi/libstub/zboot.lds
> > index d6ba89a0c294..7a1059a8a47b 100644
> > --- a/drivers/firmware/efi/libstub/zboot.lds
> > +++ b/drivers/firmware/efi/libstub/zboot.lds
> > @@ -33,7 +33,5 @@ SECTIONS
> >
> >  PROVIDE(__efistub__gzdata_size = ABSOLUTE(. - __efistub__gzdata_start));
> >
> > -PROVIDE(__efistub_uncompressed_size = __efistub__gzdata_end - 4);
> > -
> >  PROVIDE(__data_rawsize = ABSOLUTE(_edata - _etext));
> >  PROVIDE(__data_size = ABSOLUTE(_end - _etext));
> >
> > Huacai
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-08-25 10:13                   ` Feiyang Chen
@ 2022-08-25 10:22                     ` Ard Biesheuvel
  0 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2022-08-25 10:22 UTC (permalink / raw)
  To: Feiyang Chen
  Cc: Huacai Chen, Huacai Chen, Arnd Bergmann, Xuefeng Li, Xuerui Wang,
	Xi Ruoyao, Jiaxun Yang, loongarch

On Thu, 25 Aug 2022 at 12:14, Feiyang Chen <chris.chenfeiyang@gmail.com> wrote:
>
> On Thu, 25 Aug 2022 at 17:51, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Thu, 25 Aug 2022 at 11:47, Huacai Chen <chenhuacai@kernel.org> wrote:
> > >
> > > Hi, Ard,
> > >
...
> > > > > And I have tried your zboot series on a real machine, it has the same
> > > > > problem as you tried in QEMU. I will try to find the root cause with
> > > > > my colleagues.
> > > > >
> > > >
> > > > Yes please.
> > > We made the following changes and it works on LoongArch. But we don't
> > > know why the existing method didn't work. Our guess is that the linker
> > > calculates the address incorrectly.
> > >
> >
> > Could it be due to the misalignment? Could this be a toolchain issue
> > with __aligned(1)?
> >
>
> Hi, Ard,
>
> We tried to remove __aligned(1), but it doesn't work. We disassembled
> zImage.efi.elf and found that uncompressed_size and _gzdata_end are
> very far apart.
>
> // s4: uncompressed_size
> 2f94:   1c012afb        pcaddu12i       $s4, 2391(0x957)
> 2f98:   02ff837b        addi.d          $s4, $s4, -32(0xfe0)
>
> // s1: _gzdata_end
> 2fa4:   1c012a98        pcaddu12i       $s1, 2388(0x954)
> 2fa8:   02c0f318        addi.d          $s1, $s1, 60(0x3c)
>

This looks like a linker bug to me.

The ELF object has (in my case)

$ nm -n arch/loongarch/boot/vmlinuz.efi.elf|grep -A2 uncompressed

00000000007dd08e A __efistub_uncompressed_size
00000000007dd092 R _binary_arch_loongarch_boot_vmlinuz_end
00000000007dd092 R __efistub__gzdata_end

This means that, in the ELF image, uncompressed_size and _gzdata_end
are exactly 4 bytes apart.

In the object code I see

    b500:       1c00001a        pcaddu12i       $s3, 0
                        b500: R_LARCH_SOP_PUSH_PCREL    uncompressed_size+0x800
                        b500: R_LARCH_SOP_PUSH_ABSOLUTE *ABS*+0xc
                        b500: R_LARCH_SOP_SR    *ABS*
                        b500: R_LARCH_SOP_POP_32_S_5_20 *ABS*
    b504:       02c0035a        addi.d          $s3, $s3, 0
                        b504: R_LARCH_SOP_PUSH_PCREL    uncompressed_size+0x4
                        b504: R_LARCH_SOP_PUSH_PCREL    uncompressed_size+0x804
                        b504: R_LARCH_SOP_PUSH_ABSOLUTE *ABS*+0xc
                        b504: R_LARCH_SOP_SR    *ABS*
                        b504: R_LARCH_SOP_PUSH_ABSOLUTE *ABS*+0xc
                        b504: R_LARCH_SOP_SL    *ABS*
                        b504: R_LARCH_SOP_SUB   *ABS*
                        b504: R_LARCH_SOP_POP_32_S_10_12        *ABS*

and

    b52c:       1c000004        pcaddu12i       $a0, 0
                        b52c: R_LARCH_SOP_PUSH_PCREL    _gzdata_end+0x800
                        b52c: R_LARCH_SOP_PUSH_ABSOLUTE *ABS*+0xc
                        b52c: R_LARCH_SOP_SR    *ABS*
                        b52c: R_LARCH_SOP_POP_32_S_5_20 *ABS*
    b530:       02c00084        addi.d          $a0, $a0, 0
                        b530: R_LARCH_SOP_PUSH_PCREL    _gzdata_end+0x4
                        b530: R_LARCH_SOP_PUSH_PCREL    _gzdata_end+0x804
                        b530: R_LARCH_SOP_PUSH_ABSOLUTE *ABS*+0xc
                        b530: R_LARCH_SOP_SR    *ABS*
                        b530: R_LARCH_SOP_PUSH_ABSOLUTE *ABS*+0xc
                        b530: R_LARCH_SOP_SL    *ABS*
                        b530: R_LARCH_SOP_SUB   *ABS*
                        b530: R_LARCH_SOP_POP_32_S_10_12        *ABS*


I don't speak LoongArch so please figure out what is going wrong here
- this could be a rather severe bug and I don't want to work around it
before we know what the actual problem is.

Thanks,
Ard.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-08-25  9:51                 ` Ard Biesheuvel
  2022-08-25 10:13                   ` Feiyang Chen
@ 2022-08-25 11:54                   ` Huacai Chen
  1 sibling, 0 replies; 26+ messages in thread
From: Huacai Chen @ 2022-08-25 11:54 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Huacai Chen, Arnd Bergmann, Xuefeng Li, Xuerui Wang, Xi Ruoyao,
	Jiaxun Yang, loongarch

Hi, Ard,

On Thu, Aug 25, 2022 at 5:51 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Thu, 25 Aug 2022 at 11:47, Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > Hi, Ard,
> >
> > On Wed, Aug 24, 2022 at 9:28 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Wed, 24 Aug 2022 at 14:43, Huacai Chen <chenhuacai@kernel.org> wrote:
> > > >
> > > > Hi, Ard,
> > > >
> > > > On Wed, Aug 24, 2022 at 6:26 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >
> > > > > On Tue, 23 Aug 2022 at 05:15, Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > > >
> > > > > > On Tue, Aug 23, 2022 at 11:11 AM Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > > > >
> > > > > > > Hi, Ard,
> > > > > > >
> > > > > > > On Tue, Aug 23, 2022 at 2:03 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > > >
> > > > > > > > On Mon, 22 Aug 2022 at 12:44, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > > > >
> > > > > > > > > On Fri, 19 Aug 2022 at 12:20, Huacai Chen <chenhuacai@loongson.cn> wrote:
> > > > > > > > > >
> > > > > > > > > > This patch adds efistub booting support, which is the standard UEFI boot
> > > > > > > > > > protocol for us to use.
> > > > > > > > > >
> > > > > > > > > > We use generic efistub, which means we can pass boot information (i.e.,
> > > > > > > > > > system table, memory map, kernel command line, initrd) via a light FDT
> > > > > > > > > > and drop a lot of non-standard code.
> > > > > > > > > >
> > > > > > > > > > We use a flat mapping to map the efi runtime in the kernel's address
> > > > > > > > > > space. In efi, VA = PA; in kernel, VA = PA + PAGE_OFFSET. As a result,
> > > > > > > > > > flat mapping is not identity mapping, SetVirtualAddressMap() is still
> > > > > > > > > > needed for the efi runtime.
> > > > > > > > > >
> > > > > > > > > > Tested-by: Xi Ruoyao <xry111@xry111.site>
> > > > > > > > > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > > > > > > > > > ---
> > > > > > > > > > V1 --> V2:
> > > > > > > > > > 1, Call SetVirtualAddressMap() in stub;
> > > > > > > > > > 2, Use core kernel data directly in alloc_screen_info();
> > > > > > > > > > 3, Remove the magic number in MS-DOS header;
> > > > > > > > > > 4, Disable EFI_GENERIC_STUB_INITRD_CMDLINE_LOADER;
> > > > > > > > > > 5, Some other small changes suggested by Ard Biesheuvel.
> > > > > > > > > >
> > > > > > > > > > V2 --> V3:
> > > > > > > > > > 1, Adjust Makefile to adapt zboot;
> > > > > > > > > > 2, Introduce EFI_RT_VIRTUAL_OFFSET instead of changing flat_va_mapping.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Thanks for the update.
> > > > > > > > >
> > > > > > > > > I am going to queue this up in the efi/next tree. However, due to the
> > > > > > > > > many changes to arch/loongarch in this patch, conflicts are not
> > > > > > > > > unlikely, so I created a signed stable tag for the patch that you can
> > > > > > > > > merge into the loongarch arch tree if you want.
> > > > > > > > >
> > > > > > > > > *However*, you must *not* rebase your tree after merging this tag.
> > > > > > > > > Therefore, it is probably best that the merge of this tag appears as
> > > > > > > > > the very first change on your PR to Linus for v6.1. Everything after
> > > > > > > > > can be rebased at will (assuming there are no other impediments to
> > > > > > > > > doing so)
> > > > > > > > >
> > > > > > > > > You can fetch it and merge it like so:
> > > > > > > > >
> > > > > > > > > git fetch -t git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> > > > > > > > > git verify-tag efi-loongarch-for-v6.1 # if you like
> > > > > > > > > git merge efi-loongarch-for-v6.1
> > > > > > > > >
> > > > > > > > > and all your other v6.1 changes can go on top.
> > > > > > > > >
> > > > > > > > > This way, you can resolve conflicts locally without affecting the EFI
> > > > > > > > > changes going via the other tree. The EFI stub for LoongArch change
> > > > > > > > > will arrive into Linus's tree via whichever tree he pulls first: the
> > > > > > > > > LoongArch one or the EFI one.
> > > > > > > > >
> > > > > > > > > I will rebase my zboot decompressor changes on top of this - I will cc
> > > > > > > > > you again, as the LoongArch builds ok but still does not boot.
> > > > > > > > >
> > > > > > > >
> > > > > > > > I have pushed a branch here that includes EFI decompressor support for LoongArch
> > > > > > > >
> > > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
> > > > > > > >
> > > > > > > > You will need to enable CONFIG_EFI_ZBOOT and build the zImage.efi
> > > > > > > > target. The resulting image should be bootable jus tlike the
> > > > > > > > vmlinux.efi but for some reason, it produces the crash I reported
> > > > > > > > earlier.
> > > > > > > >
> > > > > > > > Please give it a try, and if you manage to figure out what's wrong
> > > > > > > > with my code, please let me know :-)
> > > > > > > I will try zboot on my real machine. For the code, I prefer
> > > > > > > vmlinuz.efi rather than zImage.efi for LoongArch since it keeps the
> > > > > > > naming consistency.
> > > > >
> > > > > OK, I will rename it to vmlinuz.efi for all architectures then.
> > > > I found RISC-V and ARM64 use Image for the normal kernel, so zImage
> > > > may be a suitable name for them, can we use a variable for the
> > > > compressed kernel? For example, use KBUILD_IMAGE directly, then we
> > > > don't need to use the same name for all architectures.
> > > >
> > >
> > > No. This is supposed to be a generic EFI boot method, and using
> > > different names for the same thing on different architectures
> > > interferes with that.
> > We have already used a EFI_ZBOOT_PAYLOAD which is different across
> > architectures, so using another KBUILD_IMAGE may have no problem? This
> > can also let "make" rather than "make zImage.efi" to build the zboot
> > kernel.
> >
>
> This is not about how we define KBUILD_IMAGE internally. If you prefer
> to build the zboot kernel by default, we can set KBUILD_IMAGE to
> vmlinuz.efi for LoongArch if CONFIG_EFI_ZBOOT=y.
>
> What I was referring to is external consumers: other projects such as
> GRUB, systemd-boot or the distros. Using zImage.efi on some
> architectures and vmlinuz.efi on other ones only creates confusion, so
> we should stick to a single one. I don't mind switching the other
> architectures to vmlinuz.efi as well.
OK, I know. vmlinuz.efi is good to me, but if it makes RISC-V or ARM64
uncomfortable, we can name it zboot.efi for all architectures. :)

Huacai

>
> > >
> > > > And I have tried your zboot series on a real machine, it has the same
> > > > problem as you tried in QEMU. I will try to find the root cause with
> > > > my colleagues.
> > > >
> > >
> > > Yes please.
> > We made the following changes and it works on LoongArch. But we don't
> > know why the existing method didn't work. Our guess is that the linker
> > calculates the address incorrectly.
> >
>
> Could it be due to the misalignment? Could this be a toolchain issue
> with __aligned(1)?
>
>
> > diff --git a/drivers/firmware/efi/libstub/zboot.c
> > b/drivers/firmware/efi/libstub/zboot.c
> > index f8eb4c7b41d1..ce3bd1affa1a 100644
> > --- a/drivers/firmware/efi/libstub/zboot.c
> > +++ b/drivers/firmware/efi/libstub/zboot.c
> > @@ -29,7 +29,6 @@ static unsigned long free_mem_ptr, free_mem_end_ptr;
> >  #endif
> >
> >  extern char _gzdata_start[], _gzdata_end[];
> > -extern u32 uncompressed_size __aligned(1);
> >
> >  static void log(efi_char16_t str[])
> >  {
> > @@ -53,6 +52,7 @@ efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
> >         unsigned long image_buffer;
> >         efi_handle_t child_handle;
> >         efi_char16_t *exit_data;
> > +       u32 uncompressed_size;
> >         efi_status_t status;
> >         int ret;
> >
> > @@ -68,6 +68,9 @@ efi_status_t __efiapi efi_zboot_entry(efi_handle_t handle,
> >                 return status;
> >         }
> >
> > +       // The uncompressed size is appended at the end of the image
> > +       uncompressed_size = *(u32 *)(_gzdata_end - 4);
> > +
> >         status = efi_allocate_pages(uncompressed_size, &image_buffer,
> > ULONG_MAX);
> >         if (status != EFI_SUCCESS) {
> >                 log(L"Failed to allocate memory\n");
> > diff --git a/drivers/firmware/efi/libstub/zboot.lds
> > b/drivers/firmware/efi/libstub/zboot.lds
> > index d6ba89a0c294..7a1059a8a47b 100644
> > --- a/drivers/firmware/efi/libstub/zboot.lds
> > +++ b/drivers/firmware/efi/libstub/zboot.lds
> > @@ -33,7 +33,5 @@ SECTIONS
> >
> >  PROVIDE(__efistub__gzdata_size = ABSOLUTE(. - __efistub__gzdata_start));
> >
> > -PROVIDE(__efistub_uncompressed_size = __efistub__gzdata_end - 4);
> > -
> >  PROVIDE(__data_rawsize = ABSOLUTE(_edata - _etext));
> >  PROVIDE(__data_size = ABSOLUTE(_end - _etext));
> >
> > Huacai

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-08-19 10:20 [PATCH V3] LoongArch: Add efistub booting support Huacai Chen
  2022-08-22 10:44 ` Ard Biesheuvel
@ 2022-08-27  4:41 ` Xi Ruoyao
  2022-08-27  7:13   ` Ard Biesheuvel
  1 sibling, 1 reply; 26+ messages in thread
From: Xi Ruoyao @ 2022-08-27  4:41 UTC (permalink / raw)
  To: Huacai Chen, Arnd Bergmann, Huacai Chen
  Cc: loongarch, linux-arch, Xuefeng Li, Guo Ren, Xuerui Wang,
	Jiaxun Yang, Ard Biesheuvel, linux-efi, linux-kernel

Tested V3 with the magic number check manually removed in my GRUB build.
The system boots successfully.  I've not tested Arnd's zBoot patch yet.
-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-08-27  4:41 ` Xi Ruoyao
@ 2022-08-27  7:13   ` Ard Biesheuvel
  2022-09-01 10:40     ` Huacai Chen
  0 siblings, 1 reply; 26+ messages in thread
From: Ard Biesheuvel @ 2022-08-27  7:13 UTC (permalink / raw)
  To: Xi Ruoyao
  Cc: Huacai Chen, Arnd Bergmann, Huacai Chen, loongarch, linux-arch,
	Xuefeng Li, Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi,
	linux-kernel

On Sat, 27 Aug 2022 at 06:41, Xi Ruoyao <xry111@xry111.site> wrote:
>
> Tested V3 with the magic number check manually removed in my GRUB build.
> The system boots successfully.  I've not tested Arnd's zBoot patch yet.

I am Ard not Arnd :-)

Please use this branch when testing the EFI decompressor:
https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-08-27  7:13   ` Ard Biesheuvel
@ 2022-09-01 10:40     ` Huacai Chen
  2022-09-04 13:24       ` Huacai Chen
  0 siblings, 1 reply; 26+ messages in thread
From: Huacai Chen @ 2022-09-01 10:40 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Xi Ruoyao, Huacai Chen, Arnd Bergmann, loongarch, linux-arch,
	Xuefeng Li, Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi, LKML

Hi, Ard,

On Sat, Aug 27, 2022 at 3:14 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Sat, 27 Aug 2022 at 06:41, Xi Ruoyao <xry111@xry111.site> wrote:
> >
> > Tested V3 with the magic number check manually removed in my GRUB build.
> > The system boots successfully.  I've not tested Arnd's zBoot patch yet.
>
> I am Ard not Arnd :-)
>
> Please use this branch when testing the EFI decompressor:
> https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
The root cause of LoongArch zboot boot failure has been found, it is a
binutils bug, latest toolchain with the below patch can solve the
problem.

diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index 5b44901b9e0..fafdc7c7458 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -2341,9 +2341,10 @@ loongarch_elf_relocate_section (bfd
*output_bfd, struct bfd_link_info *info,
     case R_LARCH_SOP_PUSH_PLT_PCREL:
       unresolved_reloc = false;

-      if (resolved_to_const)
+      if (!is_undefweak && resolved_to_const)
         {
           relocation += rel->r_addend;
+          relocation -= pc;
           break;
         }
       else if (is_undefweak)


Huacai

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-09-01 10:40     ` Huacai Chen
@ 2022-09-04 13:24       ` Huacai Chen
  2022-09-04 21:59         ` Ard Biesheuvel
  0 siblings, 1 reply; 26+ messages in thread
From: Huacai Chen @ 2022-09-04 13:24 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Xi Ruoyao, Huacai Chen, Arnd Bergmann, loongarch, linux-arch,
	Xuefeng Li, Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi, LKML

Hi, Ard,

On Thu, Sep 1, 2022 at 6:40 PM Huacai Chen <chenhuacai@kernel.org> wrote:
>
> Hi, Ard,
>
> On Sat, Aug 27, 2022 at 3:14 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Sat, 27 Aug 2022 at 06:41, Xi Ruoyao <xry111@xry111.site> wrote:
> > >
> > > Tested V3 with the magic number check manually removed in my GRUB build.
> > > The system boots successfully.  I've not tested Arnd's zBoot patch yet.
> >
> > I am Ard not Arnd :-)
> >
> > Please use this branch when testing the EFI decompressor:
> > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
> The root cause of LoongArch zboot boot failure has been found, it is a
> binutils bug, latest toolchain with the below patch can solve the
> problem.
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index 5b44901b9e0..fafdc7c7458 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -2341,9 +2341,10 @@ loongarch_elf_relocate_section (bfd
> *output_bfd, struct bfd_link_info *info,
>      case R_LARCH_SOP_PUSH_PLT_PCREL:
>        unresolved_reloc = false;
>
> -      if (resolved_to_const)
> +      if (!is_undefweak && resolved_to_const)
>          {
>            relocation += rel->r_addend;
> +          relocation -= pc;
>            break;
>          }
>        else if (is_undefweak)
>
>
> Huacai
Now the patch is submitted here:
https://sourceware.org/pipermail/binutils/2022-September/122713.html

And I have some other questions about kexec: kexec should jump to the
elf entry or the pe entry? I think is the elf entry, because if we
jump to the pe entry, then SVAM will be executed twice (but it should
be executed only once). However, how can we jump to the elf entry if
we use zboot? Maybe it is kexec-tool's responsibility to decompress
the zboot kernel image?

Huacai

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-09-04 13:24       ` Huacai Chen
@ 2022-09-04 21:59         ` Ard Biesheuvel
  2022-09-05  3:50           ` Huacai Chen
  0 siblings, 1 reply; 26+ messages in thread
From: Ard Biesheuvel @ 2022-09-04 21:59 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Xi Ruoyao, Huacai Chen, Arnd Bergmann, loongarch, linux-arch,
	Xuefeng Li, Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi, LKML

On Sun, 4 Sept 2022 at 15:24, Huacai Chen <chenhuacai@kernel.org> wrote:
>
> Hi, Ard,
>
> On Thu, Sep 1, 2022 at 6:40 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > Hi, Ard,
> >
> > On Sat, Aug 27, 2022 at 3:14 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Sat, 27 Aug 2022 at 06:41, Xi Ruoyao <xry111@xry111.site> wrote:
> > > >
> > > > Tested V3 with the magic number check manually removed in my GRUB build.
> > > > The system boots successfully.  I've not tested Arnd's zBoot patch yet.
> > >
> > > I am Ard not Arnd :-)
> > >
> > > Please use this branch when testing the EFI decompressor:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
> > The root cause of LoongArch zboot boot failure has been found, it is a
> > binutils bug, latest toolchain with the below patch can solve the
> > problem.
> >
> > diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> > index 5b44901b9e0..fafdc7c7458 100644
> > --- a/bfd/elfnn-loongarch.c
> > +++ b/bfd/elfnn-loongarch.c
> > @@ -2341,9 +2341,10 @@ loongarch_elf_relocate_section (bfd
> > *output_bfd, struct bfd_link_info *info,
> >      case R_LARCH_SOP_PUSH_PLT_PCREL:
> >        unresolved_reloc = false;
> >
> > -      if (resolved_to_const)
> > +      if (!is_undefweak && resolved_to_const)
> >          {
> >            relocation += rel->r_addend;
> > +          relocation -= pc;
> >            break;
> >          }
> >        else if (is_undefweak)
> >
> >
> > Huacai
> Now the patch is submitted here:
> https://sourceware.org/pipermail/binutils/2022-September/122713.html
>

Great. Given the severity of this bug, I imagine that building the
LoongArch kernel will require a version of binutils that carries this
fix.

Therefore, i will revert back to the original approach for accessing
uncompressed_size, using an extern declaration with an __aligned(1)
attribute.

> And I have some other questions about kexec: kexec should jump to the
> elf entry or the pe entry? I think is the elf entry, because if we
> jump to the pe entry, then SVAM will be executed twice (but it should
> be executed only once). However, how can we jump to the elf entry if
> we use zboot? Maybe it is kexec-tool's responsibility to decompress
> the zboot kernel image?
>

Yes, very good point. Kexec kernels cannot boot via the EFI entry
point, as the boot services will already be shutdown. So the kexec
kernel needs to boot via the same entrypoint in the core kernel that
the EFI stub calls when it hands over.

For the EFI zboot image in particular, we will need to teach kexec how
to decompress them. The zboot image has a header that
a) describes it as a EFI linux zimg
b) describes the start and end offset of the compressed payload
c) describes which compression algorithm was used.

This means that any non-EFI loader (including kexec) should be able to
extract the inner PE/COFF image and decompress it. For arm64 and
RISC-V, this is sufficient as the EFI and raw images are the same. For
LoongArch, I suppose it means we need a way to enter the core kernel
directly via the entrypoint that the EFI stub uses when handing over
(and pass the original DT argument so the kexec kernel has access to
the EFI and ACPI firmware tables)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-09-04 21:59         ` Ard Biesheuvel
@ 2022-09-05  3:50           ` Huacai Chen
  2022-09-05  4:34             ` Youling Tang
  2022-09-05  7:02             ` Ard Biesheuvel
  0 siblings, 2 replies; 26+ messages in thread
From: Huacai Chen @ 2022-09-05  3:50 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Xi Ruoyao, Huacai Chen, Arnd Bergmann, loongarch, linux-arch,
	Xuefeng Li, Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi, LKML

Hi, Ard,

On Mon, Sep 5, 2022 at 5:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Sun, 4 Sept 2022 at 15:24, Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > Hi, Ard,
> >
> > On Thu, Sep 1, 2022 at 6:40 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> > >
> > > Hi, Ard,
> > >
> > > On Sat, Aug 27, 2022 at 3:14 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > On Sat, 27 Aug 2022 at 06:41, Xi Ruoyao <xry111@xry111.site> wrote:
> > > > >
> > > > > Tested V3 with the magic number check manually removed in my GRUB build.
> > > > > The system boots successfully.  I've not tested Arnd's zBoot patch yet.
> > > >
> > > > I am Ard not Arnd :-)
> > > >
> > > > Please use this branch when testing the EFI decompressor:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
> > > The root cause of LoongArch zboot boot failure has been found, it is a
> > > binutils bug, latest toolchain with the below patch can solve the
> > > problem.
> > >
> > > diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> > > index 5b44901b9e0..fafdc7c7458 100644
> > > --- a/bfd/elfnn-loongarch.c
> > > +++ b/bfd/elfnn-loongarch.c
> > > @@ -2341,9 +2341,10 @@ loongarch_elf_relocate_section (bfd
> > > *output_bfd, struct bfd_link_info *info,
> > >      case R_LARCH_SOP_PUSH_PLT_PCREL:
> > >        unresolved_reloc = false;
> > >
> > > -      if (resolved_to_const)
> > > +      if (!is_undefweak && resolved_to_const)
> > >          {
> > >            relocation += rel->r_addend;
> > > +          relocation -= pc;
> > >            break;
> > >          }
> > >        else if (is_undefweak)
> > >
> > >
> > > Huacai
> > Now the patch is submitted here:
> > https://sourceware.org/pipermail/binutils/2022-September/122713.html
> >
>
> Great. Given the severity of this bug, I imagine that building the
> LoongArch kernel will require a version of binutils that carries this
> fix.
>
> Therefore, i will revert back to the original approach for accessing
> uncompressed_size, using an extern declaration with an __aligned(1)
> attribute.
>
> > And I have some other questions about kexec: kexec should jump to the
> > elf entry or the pe entry? I think is the elf entry, because if we
> > jump to the pe entry, then SVAM will be executed twice (but it should
> > be executed only once). However, how can we jump to the elf entry if
> > we use zboot? Maybe it is kexec-tool's responsibility to decompress
> > the zboot kernel image?
> >
>
> Yes, very good point. Kexec kernels cannot boot via the EFI entry
> point, as the boot services will already be shutdown. So the kexec
> kernel needs to boot via the same entrypoint in the core kernel that
> the EFI stub calls when it hands over.
>
> For the EFI zboot image in particular, we will need to teach kexec how
> to decompress them. The zboot image has a header that
> a) describes it as a EFI linux zimg
> b) describes the start and end offset of the compressed payload
> c) describes which compression algorithm was used.
>
> This means that any non-EFI loader (including kexec) should be able to
> extract the inner PE/COFF image and decompress it. For arm64 and
> RISC-V, this is sufficient as the EFI and raw images are the same. For
> LoongArch, I suppose it means we need a way to enter the core kernel
> directly via the entrypoint that the EFI stub uses when handing over
> (and pass the original DT argument so the kexec kernel has access to
> the EFI and ACPI firmware tables)
OK, then is this implementation [1] acceptable? I remember that you
said the MS-DOS header shouldn't contain other information, so I guess
this is unacceptable?

[1] https://lore.kernel.org/loongarch/c4dbb14a-5580-1e47-3d15-5d2079e88404@loongson.cn/T/#mb8c1dc44f7fa2d3ef638877f0cd3f958f0be96ad

Huacai

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-09-05  3:50           ` Huacai Chen
@ 2022-09-05  4:34             ` Youling Tang
  2022-09-05  6:28               ` Youling Tang
  2022-09-05  7:03               ` Ard Biesheuvel
  2022-09-05  7:02             ` Ard Biesheuvel
  1 sibling, 2 replies; 26+ messages in thread
From: Youling Tang @ 2022-09-05  4:34 UTC (permalink / raw)
  To: Huacai Chen, Ard Biesheuvel
  Cc: Xi Ruoyao, Huacai Chen, Arnd Bergmann, loongarch, linux-arch,
	Xuefeng Li, Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi, LKML

Hi, Ard & Huacai

On 09/05/2022 11:50 AM, Huacai Chen wrote:
> Hi, Ard,
>
> On Mon, Sep 5, 2022 at 5:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> On Sun, 4 Sept 2022 at 15:24, Huacai Chen <chenhuacai@kernel.org> wrote:
>>>
>>> Hi, Ard,
>>>
>>> On Thu, Sep 1, 2022 at 6:40 PM Huacai Chen <chenhuacai@kernel.org> wrote:
>>>>
>>>> Hi, Ard,
>>>>
>>>> On Sat, Aug 27, 2022 at 3:14 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>>
>>>>> On Sat, 27 Aug 2022 at 06:41, Xi Ruoyao <xry111@xry111.site> wrote:
>>>>>>
>>>>>> Tested V3 with the magic number check manually removed in my GRUB build.
>>>>>> The system boots successfully.  I've not tested Arnd's zBoot patch yet.
>>>>>
>>>>> I am Ard not Arnd :-)
>>>>>
>>>>> Please use this branch when testing the EFI decompressor:
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
>>>> The root cause of LoongArch zboot boot failure has been found, it is a
>>>> binutils bug, latest toolchain with the below patch can solve the
>>>> problem.
>>>>
>>>> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
>>>> index 5b44901b9e0..fafdc7c7458 100644
>>>> --- a/bfd/elfnn-loongarch.c
>>>> +++ b/bfd/elfnn-loongarch.c
>>>> @@ -2341,9 +2341,10 @@ loongarch_elf_relocate_section (bfd
>>>> *output_bfd, struct bfd_link_info *info,
>>>>      case R_LARCH_SOP_PUSH_PLT_PCREL:
>>>>        unresolved_reloc = false;
>>>>
>>>> -      if (resolved_to_const)
>>>> +      if (!is_undefweak && resolved_to_const)
>>>>          {
>>>>            relocation += rel->r_addend;
>>>> +          relocation -= pc;
>>>>            break;
>>>>          }
>>>>        else if (is_undefweak)
>>>>
>>>>
>>>> Huacai
>>> Now the patch is submitted here:
>>> https://sourceware.org/pipermail/binutils/2022-September/122713.html
>>>
>>
>> Great. Given the severity of this bug, I imagine that building the
>> LoongArch kernel will require a version of binutils that carries this
>> fix.
>>
>> Therefore, i will revert back to the original approach for accessing
>> uncompressed_size, using an extern declaration with an __aligned(1)
>> attribute.
>>
>>> And I have some other questions about kexec: kexec should jump to the
>>> elf entry or the pe entry? I think is the elf entry, because if we
>>> jump to the pe entry, then SVAM will be executed twice (but it should
>>> be executed only once). However, how can we jump to the elf entry if
>>> we use zboot? Maybe it is kexec-tool's responsibility to decompress
>>> the zboot kernel image?
>>>
>>
>> Yes, very good point. Kexec kernels cannot boot via the EFI entry
>> point, as the boot services will already be shutdown. So the kexec
>> kernel needs to boot via the same entrypoint in the core kernel that
>> the EFI stub calls when it hands over.
>>
>> For the EFI zboot image in particular, we will need to teach kexec how
>> to decompress them. The zboot image has a header that
>> a) describes it as a EFI linux zimg
>> b) describes the start and end offset of the compressed payload
>> c) describes which compression algorithm was used.
>>
>> This means that any non-EFI loader (including kexec) should be able to
>> extract the inner PE/COFF image and decompress it. For arm64 and
>> RISC-V, this is sufficient as the EFI and raw images are the same. For
>> LoongArch, I suppose it means we need a way to enter the core kernel
>> directly via the entrypoint that the EFI stub uses when handing over
>> (and pass the original DT argument so the kexec kernel has access to
>> the EFI and ACPI firmware tables)
> OK, then is this implementation [1] acceptable? I remember that you
> said the MS-DOS header shouldn't contain other information, so I guess
> this is unacceptable?
>
> [1] https://lore.kernel.org/loongarch/c4dbb14a-5580-1e47-3d15-5d2079e88404@loongson.cn/T/#mb8c1dc44f7fa2d3ef638877f0cd3f958f0be96ad

Modifications to the MS-DOS header refer to the arm64 and riscv
implementations [1], and to provide the necessary information to
kexec-tools[2] when loading uncompressed efi images.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0f327f2aaad6a87356cbccfa390d4d3b64d0d3b6
[2] 
https://github.com/horms/kexec-tools/blob/main/kexec/arch/arm64/image-header.h

Thanks,
Youling

>
> Huacai
>


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-09-05  4:34             ` Youling Tang
@ 2022-09-05  6:28               ` Youling Tang
  2022-09-05  7:04                 ` Ard Biesheuvel
  2022-09-05  7:03               ` Ard Biesheuvel
  1 sibling, 1 reply; 26+ messages in thread
From: Youling Tang @ 2022-09-05  6:28 UTC (permalink / raw)
  To: Huacai Chen, Ard Biesheuvel
  Cc: Xi Ruoyao, Huacai Chen, Arnd Bergmann, loongarch, linux-arch,
	Xuefeng Li, Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi, LKML

Hi, Ard & Huacai

While applying this patch [1] we need to add vmlinuz* (vmlinuz and
vmlinuz.efi) to arch/loongarch/boot/.gitignore to ignore the generated
binary (also required for arm64 and riscv).

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/commit/?h=efi-decompressor-v4&id=efcc03a013f2ddbed0ee782e5049b39432dc9db2

Thanks,
Youling.

On 09/05/2022 12:34 PM, Youling Tang wrote:
> Hi, Ard & Huacai
>
> On 09/05/2022 11:50 AM, Huacai Chen wrote:
>> Hi, Ard,
>>
>> On Mon, Sep 5, 2022 at 5:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>>>
>>> On Sun, 4 Sept 2022 at 15:24, Huacai Chen <chenhuacai@kernel.org> wrote:
>>>>
>>>> Hi, Ard,
>>>>
>>>> On Thu, Sep 1, 2022 at 6:40 PM Huacai Chen <chenhuacai@kernel.org>
>>>> wrote:
>>>>>
>>>>> Hi, Ard,
>>>>>
>>>>> On Sat, Aug 27, 2022 at 3:14 PM Ard Biesheuvel <ardb@kernel.org>
>>>>> wrote:
>>>>>>
>>>>>> On Sat, 27 Aug 2022 at 06:41, Xi Ruoyao <xry111@xry111.site> wrote:
>>>>>>>
>>>>>>> Tested V3 with the magic number check manually removed in my GRUB
>>>>>>> build.
>>>>>>> The system boots successfully.  I've not tested Arnd's zBoot
>>>>>>> patch yet.
>>>>>>
>>>>>> I am Ard not Arnd :-)
>>>>>>
>>>>>> Please use this branch when testing the EFI decompressor:
>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
>>>>>>
>>>>> The root cause of LoongArch zboot boot failure has been found, it is a
>>>>> binutils bug, latest toolchain with the below patch can solve the
>>>>> problem.
>>>>>
>>>>> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
>>>>> index 5b44901b9e0..fafdc7c7458 100644
>>>>> --- a/bfd/elfnn-loongarch.c
>>>>> +++ b/bfd/elfnn-loongarch.c
>>>>> @@ -2341,9 +2341,10 @@ loongarch_elf_relocate_section (bfd
>>>>> *output_bfd, struct bfd_link_info *info,
>>>>>      case R_LARCH_SOP_PUSH_PLT_PCREL:
>>>>>        unresolved_reloc = false;
>>>>>
>>>>> -      if (resolved_to_const)
>>>>> +      if (!is_undefweak && resolved_to_const)
>>>>>          {
>>>>>            relocation += rel->r_addend;
>>>>> +          relocation -= pc;
>>>>>            break;
>>>>>          }
>>>>>        else if (is_undefweak)
>>>>>
>>>>>
>>>>> Huacai
>>>> Now the patch is submitted here:
>>>> https://sourceware.org/pipermail/binutils/2022-September/122713.html
>>>>
>>>
>>> Great. Given the severity of this bug, I imagine that building the
>>> LoongArch kernel will require a version of binutils that carries this
>>> fix.
>>>
>>> Therefore, i will revert back to the original approach for accessing
>>> uncompressed_size, using an extern declaration with an __aligned(1)
>>> attribute.
>>>
>>>> And I have some other questions about kexec: kexec should jump to the
>>>> elf entry or the pe entry? I think is the elf entry, because if we
>>>> jump to the pe entry, then SVAM will be executed twice (but it should
>>>> be executed only once). However, how can we jump to the elf entry if
>>>> we use zboot? Maybe it is kexec-tool's responsibility to decompress
>>>> the zboot kernel image?
>>>>
>>>
>>> Yes, very good point. Kexec kernels cannot boot via the EFI entry
>>> point, as the boot services will already be shutdown. So the kexec
>>> kernel needs to boot via the same entrypoint in the core kernel that
>>> the EFI stub calls when it hands over.
>>>
>>> For the EFI zboot image in particular, we will need to teach kexec how
>>> to decompress them. The zboot image has a header that
>>> a) describes it as a EFI linux zimg
>>> b) describes the start and end offset of the compressed payload
>>> c) describes which compression algorithm was used.
>>>
>>> This means that any non-EFI loader (including kexec) should be able to
>>> extract the inner PE/COFF image and decompress it. For arm64 and
>>> RISC-V, this is sufficient as the EFI and raw images are the same. For
>>> LoongArch, I suppose it means we need a way to enter the core kernel
>>> directly via the entrypoint that the EFI stub uses when handing over
>>> (and pass the original DT argument so the kexec kernel has access to
>>> the EFI and ACPI firmware tables)
>> OK, then is this implementation [1] acceptable? I remember that you
>> said the MS-DOS header shouldn't contain other information, so I guess
>> this is unacceptable?
>>
>> [1]
>> https://lore.kernel.org/loongarch/c4dbb14a-5580-1e47-3d15-5d2079e88404@loongson.cn/T/#mb8c1dc44f7fa2d3ef638877f0cd3f958f0be96ad
>>
>
> Modifications to the MS-DOS header refer to the arm64 and riscv
> implementations [1], and to provide the necessary information to
> kexec-tools[2] when loading uncompressed efi images.
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0f327f2aaad6a87356cbccfa390d4d3b64d0d3b6
>
> [2]
> https://github.com/horms/kexec-tools/blob/main/kexec/arch/arm64/image-header.h
>
>
> Thanks,
> Youling
>
>>
>> Huacai
>>
>


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-09-05  3:50           ` Huacai Chen
  2022-09-05  4:34             ` Youling Tang
@ 2022-09-05  7:02             ` Ard Biesheuvel
  2022-09-05  7:24               ` Huacai Chen
  1 sibling, 1 reply; 26+ messages in thread
From: Ard Biesheuvel @ 2022-09-05  7:02 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Xi Ruoyao, Huacai Chen, Arnd Bergmann, loongarch, linux-arch,
	Xuefeng Li, Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi, LKML

On Mon, 5 Sept 2022 at 05:51, Huacai Chen <chenhuacai@kernel.org> wrote:
>
> Hi, Ard,
>
> On Mon, Sep 5, 2022 at 5:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Sun, 4 Sept 2022 at 15:24, Huacai Chen <chenhuacai@kernel.org> wrote:
> > >
> > > Hi, Ard,
> > >
> > > On Thu, Sep 1, 2022 at 6:40 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> > > >
> > > > Hi, Ard,
> > > >
> > > > On Sat, Aug 27, 2022 at 3:14 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >
> > > > > On Sat, 27 Aug 2022 at 06:41, Xi Ruoyao <xry111@xry111.site> wrote:
> > > > > >
> > > > > > Tested V3 with the magic number check manually removed in my GRUB build.
> > > > > > The system boots successfully.  I've not tested Arnd's zBoot patch yet.
> > > > >
> > > > > I am Ard not Arnd :-)
> > > > >
> > > > > Please use this branch when testing the EFI decompressor:
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
> > > > The root cause of LoongArch zboot boot failure has been found, it is a
> > > > binutils bug, latest toolchain with the below patch can solve the
> > > > problem.
> > > >
> > > > diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> > > > index 5b44901b9e0..fafdc7c7458 100644
> > > > --- a/bfd/elfnn-loongarch.c
> > > > +++ b/bfd/elfnn-loongarch.c
> > > > @@ -2341,9 +2341,10 @@ loongarch_elf_relocate_section (bfd
> > > > *output_bfd, struct bfd_link_info *info,
> > > >      case R_LARCH_SOP_PUSH_PLT_PCREL:
> > > >        unresolved_reloc = false;
> > > >
> > > > -      if (resolved_to_const)
> > > > +      if (!is_undefweak && resolved_to_const)
> > > >          {
> > > >            relocation += rel->r_addend;
> > > > +          relocation -= pc;
> > > >            break;
> > > >          }
> > > >        else if (is_undefweak)
> > > >
> > > >
> > > > Huacai
> > > Now the patch is submitted here:
> > > https://sourceware.org/pipermail/binutils/2022-September/122713.html
> > >
> >
> > Great. Given the severity of this bug, I imagine that building the
> > LoongArch kernel will require a version of binutils that carries this
> > fix.
> >
> > Therefore, i will revert back to the original approach for accessing
> > uncompressed_size, using an extern declaration with an __aligned(1)
> > attribute.
> >
> > > And I have some other questions about kexec: kexec should jump to the
> > > elf entry or the pe entry? I think is the elf entry, because if we
> > > jump to the pe entry, then SVAM will be executed twice (but it should
> > > be executed only once). However, how can we jump to the elf entry if
> > > we use zboot? Maybe it is kexec-tool's responsibility to decompress
> > > the zboot kernel image?
> > >
> >
> > Yes, very good point. Kexec kernels cannot boot via the EFI entry
> > point, as the boot services will already be shutdown. So the kexec
> > kernel needs to boot via the same entrypoint in the core kernel that
> > the EFI stub calls when it hands over.
> >
> > For the EFI zboot image in particular, we will need to teach kexec how
> > to decompress them. The zboot image has a header that
> > a) describes it as a EFI linux zimg
> > b) describes the start and end offset of the compressed payload
> > c) describes which compression algorithm was used.
> >
> > This means that any non-EFI loader (including kexec) should be able to
> > extract the inner PE/COFF image and decompress it. For arm64 and
> > RISC-V, this is sufficient as the EFI and raw images are the same. For
> > LoongArch, I suppose it means we need a way to enter the core kernel
> > directly via the entrypoint that the EFI stub uses when handing over
> > (and pass the original DT argument so the kexec kernel has access to
> > the EFI and ACPI firmware tables)
> OK, then is this implementation [1] acceptable? I remember that you
> said the MS-DOS header shouldn't contain other information, so I guess
> this is unacceptable?
>

No, this looks reasonable to me. I objected to using magic numbers in
the 'pure PE' view of the image, as it does not make sense for a pure
PE loader such as GRUB to rely on such metadata.

In this case (like on arm64), we are dealing with something else: we
need to identify the image to the kernel itself, and here, using the
unused space in the MS-DOS header is fine.

> [1] https://lore.kernel.org/loongarch/c4dbb14a-5580-1e47-3d15-5d2079e88404@loongson.cn/T/#mb8c1dc44f7fa2d3ef638877f0cd3f958f0be96ad
>
> Huacai

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-09-05  4:34             ` Youling Tang
  2022-09-05  6:28               ` Youling Tang
@ 2022-09-05  7:03               ` Ard Biesheuvel
  1 sibling, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2022-09-05  7:03 UTC (permalink / raw)
  To: Youling Tang
  Cc: Huacai Chen, Xi Ruoyao, Huacai Chen, Arnd Bergmann, loongarch,
	linux-arch, Xuefeng Li, Guo Ren, Xuerui Wang, Jiaxun Yang,
	linux-efi, LKML

On Mon, 5 Sept 2022 at 06:34, Youling Tang <tangyouling@loongson.cn> wrote:
>
> Hi, Ard & Huacai
>
> On 09/05/2022 11:50 AM, Huacai Chen wrote:
> > Hi, Ard,
> >
> > On Mon, Sep 5, 2022 at 5:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >>
> >> On Sun, 4 Sept 2022 at 15:24, Huacai Chen <chenhuacai@kernel.org> wrote:
> >>>
> >>> Hi, Ard,
> >>>
> >>> On Thu, Sep 1, 2022 at 6:40 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> >>>>
> >>>> Hi, Ard,
> >>>>
> >>>> On Sat, Aug 27, 2022 at 3:14 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >>>>>
> >>>>> On Sat, 27 Aug 2022 at 06:41, Xi Ruoyao <xry111@xry111.site> wrote:
> >>>>>>
> >>>>>> Tested V3 with the magic number check manually removed in my GRUB build.
> >>>>>> The system boots successfully.  I've not tested Arnd's zBoot patch yet.
> >>>>>
> >>>>> I am Ard not Arnd :-)
> >>>>>
> >>>>> Please use this branch when testing the EFI decompressor:
> >>>>> https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
> >>>> The root cause of LoongArch zboot boot failure has been found, it is a
> >>>> binutils bug, latest toolchain with the below patch can solve the
> >>>> problem.
> >>>>
> >>>> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> >>>> index 5b44901b9e0..fafdc7c7458 100644
> >>>> --- a/bfd/elfnn-loongarch.c
> >>>> +++ b/bfd/elfnn-loongarch.c
> >>>> @@ -2341,9 +2341,10 @@ loongarch_elf_relocate_section (bfd
> >>>> *output_bfd, struct bfd_link_info *info,
> >>>>      case R_LARCH_SOP_PUSH_PLT_PCREL:
> >>>>        unresolved_reloc = false;
> >>>>
> >>>> -      if (resolved_to_const)
> >>>> +      if (!is_undefweak && resolved_to_const)
> >>>>          {
> >>>>            relocation += rel->r_addend;
> >>>> +          relocation -= pc;
> >>>>            break;
> >>>>          }
> >>>>        else if (is_undefweak)
> >>>>
> >>>>
> >>>> Huacai
> >>> Now the patch is submitted here:
> >>> https://sourceware.org/pipermail/binutils/2022-September/122713.html
> >>>
> >>
> >> Great. Given the severity of this bug, I imagine that building the
> >> LoongArch kernel will require a version of binutils that carries this
> >> fix.
> >>
> >> Therefore, i will revert back to the original approach for accessing
> >> uncompressed_size, using an extern declaration with an __aligned(1)
> >> attribute.
> >>
> >>> And I have some other questions about kexec: kexec should jump to the
> >>> elf entry or the pe entry? I think is the elf entry, because if we
> >>> jump to the pe entry, then SVAM will be executed twice (but it should
> >>> be executed only once). However, how can we jump to the elf entry if
> >>> we use zboot? Maybe it is kexec-tool's responsibility to decompress
> >>> the zboot kernel image?
> >>>
> >>
> >> Yes, very good point. Kexec kernels cannot boot via the EFI entry
> >> point, as the boot services will already be shutdown. So the kexec
> >> kernel needs to boot via the same entrypoint in the core kernel that
> >> the EFI stub calls when it hands over.
> >>
> >> For the EFI zboot image in particular, we will need to teach kexec how
> >> to decompress them. The zboot image has a header that
> >> a) describes it as a EFI linux zimg
> >> b) describes the start and end offset of the compressed payload
> >> c) describes which compression algorithm was used.
> >>
> >> This means that any non-EFI loader (including kexec) should be able to
> >> extract the inner PE/COFF image and decompress it. For arm64 and
> >> RISC-V, this is sufficient as the EFI and raw images are the same. For
> >> LoongArch, I suppose it means we need a way to enter the core kernel
> >> directly via the entrypoint that the EFI stub uses when handing over
> >> (and pass the original DT argument so the kexec kernel has access to
> >> the EFI and ACPI firmware tables)
> > OK, then is this implementation [1] acceptable? I remember that you
> > said the MS-DOS header shouldn't contain other information, so I guess
> > this is unacceptable?
> >
> > [1] https://lore.kernel.org/loongarch/c4dbb14a-5580-1e47-3d15-5d2079e88404@loongson.cn/T/#mb8c1dc44f7fa2d3ef638877f0cd3f958f0be96ad
>
> Modifications to the MS-DOS header refer to the arm64 and riscv
> implementations [1], and to provide the necessary information to
> kexec-tools[2] when loading uncompressed efi images.
>

Indeed. On top of the EFI zboot series, we need some [generic!] kexec
plumbing to identify such compressed images, and decompress the
payload before passing the contents to the arch specific probe/load
image hooks.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-09-05  6:28               ` Youling Tang
@ 2022-09-05  7:04                 ` Ard Biesheuvel
  0 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2022-09-05  7:04 UTC (permalink / raw)
  To: Youling Tang
  Cc: Huacai Chen, Xi Ruoyao, Huacai Chen, Arnd Bergmann, loongarch,
	linux-arch, Xuefeng Li, Guo Ren, Xuerui Wang, Jiaxun Yang,
	linux-efi, LKML

On Mon, 5 Sept 2022 at 08:29, Youling Tang <tangyouling@loongson.cn> wrote:
>
> Hi, Ard & Huacai
>
> While applying this patch [1] we need to add vmlinuz* (vmlinuz and
> vmlinuz.efi) to arch/loongarch/boot/.gitignore to ignore the generated
> binary (also required for arm64 and riscv).
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/commit/?h=efi-decompressor-v4&id=efcc03a013f2ddbed0ee782e5049b39432dc9db2
>

Good point. I'll add that.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-09-05  7:02             ` Ard Biesheuvel
@ 2022-09-05  7:24               ` Huacai Chen
  2022-09-05  7:28                 ` Ard Biesheuvel
  2022-09-05  7:34                 ` Youling Tang
  0 siblings, 2 replies; 26+ messages in thread
From: Huacai Chen @ 2022-09-05  7:24 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Xi Ruoyao, Huacai Chen, Arnd Bergmann, loongarch, linux-arch,
	Xuefeng Li, Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi, LKML

Hi, Ard and Youling,

On Mon, Sep 5, 2022 at 3:02 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 5 Sept 2022 at 05:51, Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > Hi, Ard,
> >
> > On Mon, Sep 5, 2022 at 5:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Sun, 4 Sept 2022 at 15:24, Huacai Chen <chenhuacai@kernel.org> wrote:
> > > >
> > > > Hi, Ard,
> > > >
> > > > On Thu, Sep 1, 2022 at 6:40 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > >
> > > > > Hi, Ard,
> > > > >
> > > > > On Sat, Aug 27, 2022 at 3:14 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > >
> > > > > > On Sat, 27 Aug 2022 at 06:41, Xi Ruoyao <xry111@xry111.site> wrote:
> > > > > > >
> > > > > > > Tested V3 with the magic number check manually removed in my GRUB build.
> > > > > > > The system boots successfully.  I've not tested Arnd's zBoot patch yet.
> > > > > >
> > > > > > I am Ard not Arnd :-)
> > > > > >
> > > > > > Please use this branch when testing the EFI decompressor:
> > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
> > > > > The root cause of LoongArch zboot boot failure has been found, it is a
> > > > > binutils bug, latest toolchain with the below patch can solve the
> > > > > problem.
> > > > >
> > > > > diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> > > > > index 5b44901b9e0..fafdc7c7458 100644
> > > > > --- a/bfd/elfnn-loongarch.c
> > > > > +++ b/bfd/elfnn-loongarch.c
> > > > > @@ -2341,9 +2341,10 @@ loongarch_elf_relocate_section (bfd
> > > > > *output_bfd, struct bfd_link_info *info,
> > > > >      case R_LARCH_SOP_PUSH_PLT_PCREL:
> > > > >        unresolved_reloc = false;
> > > > >
> > > > > -      if (resolved_to_const)
> > > > > +      if (!is_undefweak && resolved_to_const)
> > > > >          {
> > > > >            relocation += rel->r_addend;
> > > > > +          relocation -= pc;
> > > > >            break;
> > > > >          }
> > > > >        else if (is_undefweak)
> > > > >
> > > > >
> > > > > Huacai
> > > > Now the patch is submitted here:
> > > > https://sourceware.org/pipermail/binutils/2022-September/122713.html
> > > >
> > >
> > > Great. Given the severity of this bug, I imagine that building the
> > > LoongArch kernel will require a version of binutils that carries this
> > > fix.
> > >
> > > Therefore, i will revert back to the original approach for accessing
> > > uncompressed_size, using an extern declaration with an __aligned(1)
> > > attribute.
> > >
> > > > And I have some other questions about kexec: kexec should jump to the
> > > > elf entry or the pe entry? I think is the elf entry, because if we
> > > > jump to the pe entry, then SVAM will be executed twice (but it should
> > > > be executed only once). However, how can we jump to the elf entry if
> > > > we use zboot? Maybe it is kexec-tool's responsibility to decompress
> > > > the zboot kernel image?
> > > >
> > >
> > > Yes, very good point. Kexec kernels cannot boot via the EFI entry
> > > point, as the boot services will already be shutdown. So the kexec
> > > kernel needs to boot via the same entrypoint in the core kernel that
> > > the EFI stub calls when it hands over.
> > >
> > > For the EFI zboot image in particular, we will need to teach kexec how
> > > to decompress them. The zboot image has a header that
> > > a) describes it as a EFI linux zimg
> > > b) describes the start and end offset of the compressed payload
> > > c) describes which compression algorithm was used.
> > >
> > > This means that any non-EFI loader (including kexec) should be able to
> > > extract the inner PE/COFF image and decompress it. For arm64 and
> > > RISC-V, this is sufficient as the EFI and raw images are the same. For
> > > LoongArch, I suppose it means we need a way to enter the core kernel
> > > directly via the entrypoint that the EFI stub uses when handing over
> > > (and pass the original DT argument so the kexec kernel has access to
> > > the EFI and ACPI firmware tables)
> > OK, then is this implementation [1] acceptable? I remember that you
> > said the MS-DOS header shouldn't contain other information, so I guess
> > this is unacceptable?
> >
>
> No, this looks reasonable to me. I objected to using magic numbers in
> the 'pure PE' view of the image, as it does not make sense for a pure
> PE loader such as GRUB to rely on such metadata.
>
> In this case (like on arm64), we are dealing with something else: we
> need to identify the image to the kernel itself, and here, using the
> unused space in the MS-DOS header is fine.
>
> > [1] https://lore.kernel.org/loongarch/c4dbb14a-5580-1e47-3d15-5d2079e88404@loongson.cn/T/#mb8c1dc44f7fa2d3ef638877f0cd3f958f0be96ad
OK, then there is no big problem here. And I found that arm64/riscv
don't need the kernel entry point in the header. I don't know why, but
I think it implies that a unified layout across architectures is
unnecessary, and I prefer to put the kernel entry point before
effective kernel size. :)

Huacai

> >
> > Huacai

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-09-05  7:24               ` Huacai Chen
@ 2022-09-05  7:28                 ` Ard Biesheuvel
  2022-09-05 18:07                   ` WANG Xuerui
  2022-09-05  7:34                 ` Youling Tang
  1 sibling, 1 reply; 26+ messages in thread
From: Ard Biesheuvel @ 2022-09-05  7:28 UTC (permalink / raw)
  To: Huacai Chen
  Cc: Xi Ruoyao, Huacai Chen, Arnd Bergmann, loongarch, linux-arch,
	Xuefeng Li, Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi, LKML

On Mon, 5 Sept 2022 at 09:25, Huacai Chen <chenhuacai@kernel.org> wrote:
>
> Hi, Ard and Youling,
>
> On Mon, Sep 5, 2022 at 3:02 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Mon, 5 Sept 2022 at 05:51, Huacai Chen <chenhuacai@kernel.org> wrote:
> > >
> > > Hi, Ard,
> > >
> > > On Mon, Sep 5, 2022 at 5:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > On Sun, 4 Sept 2022 at 15:24, Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > >
> > > > > Hi, Ard,
> > > > >
> > > > > On Thu, Sep 1, 2022 at 6:40 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> > > > > >
> > > > > > Hi, Ard,
> > > > > >
> > > > > > On Sat, Aug 27, 2022 at 3:14 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > > > >
> > > > > > > On Sat, 27 Aug 2022 at 06:41, Xi Ruoyao <xry111@xry111.site> wrote:
> > > > > > > >
> > > > > > > > Tested V3 with the magic number check manually removed in my GRUB build.
> > > > > > > > The system boots successfully.  I've not tested Arnd's zBoot patch yet.
> > > > > > >
> > > > > > > I am Ard not Arnd :-)
> > > > > > >
> > > > > > > Please use this branch when testing the EFI decompressor:
> > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
> > > > > > The root cause of LoongArch zboot boot failure has been found, it is a
> > > > > > binutils bug, latest toolchain with the below patch can solve the
> > > > > > problem.
> > > > > >
> > > > > > diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> > > > > > index 5b44901b9e0..fafdc7c7458 100644
> > > > > > --- a/bfd/elfnn-loongarch.c
> > > > > > +++ b/bfd/elfnn-loongarch.c
> > > > > > @@ -2341,9 +2341,10 @@ loongarch_elf_relocate_section (bfd
> > > > > > *output_bfd, struct bfd_link_info *info,
> > > > > >      case R_LARCH_SOP_PUSH_PLT_PCREL:
> > > > > >        unresolved_reloc = false;
> > > > > >
> > > > > > -      if (resolved_to_const)
> > > > > > +      if (!is_undefweak && resolved_to_const)
> > > > > >          {
> > > > > >            relocation += rel->r_addend;
> > > > > > +          relocation -= pc;
> > > > > >            break;
> > > > > >          }
> > > > > >        else if (is_undefweak)
> > > > > >
> > > > > >
> > > > > > Huacai
> > > > > Now the patch is submitted here:
> > > > > https://sourceware.org/pipermail/binutils/2022-September/122713.html
> > > > >
> > > >
> > > > Great. Given the severity of this bug, I imagine that building the
> > > > LoongArch kernel will require a version of binutils that carries this
> > > > fix.
> > > >
> > > > Therefore, i will revert back to the original approach for accessing
> > > > uncompressed_size, using an extern declaration with an __aligned(1)
> > > > attribute.
> > > >
> > > > > And I have some other questions about kexec: kexec should jump to the
> > > > > elf entry or the pe entry? I think is the elf entry, because if we
> > > > > jump to the pe entry, then SVAM will be executed twice (but it should
> > > > > be executed only once). However, how can we jump to the elf entry if
> > > > > we use zboot? Maybe it is kexec-tool's responsibility to decompress
> > > > > the zboot kernel image?
> > > > >
> > > >
> > > > Yes, very good point. Kexec kernels cannot boot via the EFI entry
> > > > point, as the boot services will already be shutdown. So the kexec
> > > > kernel needs to boot via the same entrypoint in the core kernel that
> > > > the EFI stub calls when it hands over.
> > > >
> > > > For the EFI zboot image in particular, we will need to teach kexec how
> > > > to decompress them. The zboot image has a header that
> > > > a) describes it as a EFI linux zimg
> > > > b) describes the start and end offset of the compressed payload
> > > > c) describes which compression algorithm was used.
> > > >
> > > > This means that any non-EFI loader (including kexec) should be able to
> > > > extract the inner PE/COFF image and decompress it. For arm64 and
> > > > RISC-V, this is sufficient as the EFI and raw images are the same. For
> > > > LoongArch, I suppose it means we need a way to enter the core kernel
> > > > directly via the entrypoint that the EFI stub uses when handing over
> > > > (and pass the original DT argument so the kexec kernel has access to
> > > > the EFI and ACPI firmware tables)
> > > OK, then is this implementation [1] acceptable? I remember that you
> > > said the MS-DOS header shouldn't contain other information, so I guess
> > > this is unacceptable?
> > >
> >
> > No, this looks reasonable to me. I objected to using magic numbers in
> > the 'pure PE' view of the image, as it does not make sense for a pure
> > PE loader such as GRUB to rely on such metadata.
> >
> > In this case (like on arm64), we are dealing with something else: we
> > need to identify the image to the kernel itself, and here, using the
> > unused space in the MS-DOS header is fine.
> >
> > > [1] https://lore.kernel.org/loongarch/c4dbb14a-5580-1e47-3d15-5d2079e88404@loongson.cn/T/#mb8c1dc44f7fa2d3ef638877f0cd3f958f0be96ad
> OK, then there is no big problem here. And I found that arm64/riscv
> don't need the kernel entry point in the header. I don't know why, but
> I think it implies that a unified layout across architectures is
> unnecessary, and I prefer to put the kernel entry point before
> effective kernel size. :)
>

It is fine to put the entry point offset in the header. arm64 and
RISC-V don't need this because the first instructions are a pseudo-NOP
(an instruction that does nothing but its binary encoding looks like
'MZ..') and a jump to the actual entry point.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-09-05  7:24               ` Huacai Chen
  2022-09-05  7:28                 ` Ard Biesheuvel
@ 2022-09-05  7:34                 ` Youling Tang
  1 sibling, 0 replies; 26+ messages in thread
From: Youling Tang @ 2022-09-05  7:34 UTC (permalink / raw)
  To: Huacai Chen, Ard Biesheuvel
  Cc: Xi Ruoyao, Huacai Chen, Arnd Bergmann, loongarch, linux-arch,
	Xuefeng Li, Guo Ren, Xuerui Wang, Jiaxun Yang, linux-efi, LKML

Hi, Huacai

On 09/05/2022 03:24 PM, Huacai Chen wrote:
> Hi, Ard and Youling,
>
> On Mon, Sep 5, 2022 at 3:02 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>>
>> On Mon, 5 Sept 2022 at 05:51, Huacai Chen <chenhuacai@kernel.org> wrote:
>>>
>>> Hi, Ard,
>>>
>>> On Mon, Sep 5, 2022 at 5:59 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>
>>>> On Sun, 4 Sept 2022 at 15:24, Huacai Chen <chenhuacai@kernel.org> wrote:
>>>>>
>>>>> Hi, Ard,
>>>>>
>>>>> On Thu, Sep 1, 2022 at 6:40 PM Huacai Chen <chenhuacai@kernel.org> wrote:
>>>>>>
>>>>>> Hi, Ard,
>>>>>>
>>>>>> On Sat, Aug 27, 2022 at 3:14 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>>>>>>>
>>>>>>> On Sat, 27 Aug 2022 at 06:41, Xi Ruoyao <xry111@xry111.site> wrote:
>>>>>>>>
>>>>>>>> Tested V3 with the magic number check manually removed in my GRUB build.
>>>>>>>> The system boots successfully.  I've not tested Arnd's zBoot patch yet.
>>>>>>>
>>>>>>> I am Ard not Arnd :-)
>>>>>>>
>>>>>>> Please use this branch when testing the EFI decompressor:
>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-decompressor-v4
>>>>>> The root cause of LoongArch zboot boot failure has been found, it is a
>>>>>> binutils bug, latest toolchain with the below patch can solve the
>>>>>> problem.
>>>>>>
>>>>>> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
>>>>>> index 5b44901b9e0..fafdc7c7458 100644
>>>>>> --- a/bfd/elfnn-loongarch.c
>>>>>> +++ b/bfd/elfnn-loongarch.c
>>>>>> @@ -2341,9 +2341,10 @@ loongarch_elf_relocate_section (bfd
>>>>>> *output_bfd, struct bfd_link_info *info,
>>>>>>      case R_LARCH_SOP_PUSH_PLT_PCREL:
>>>>>>        unresolved_reloc = false;
>>>>>>
>>>>>> -      if (resolved_to_const)
>>>>>> +      if (!is_undefweak && resolved_to_const)
>>>>>>          {
>>>>>>            relocation += rel->r_addend;
>>>>>> +          relocation -= pc;
>>>>>>            break;
>>>>>>          }
>>>>>>        else if (is_undefweak)
>>>>>>
>>>>>>
>>>>>> Huacai
>>>>> Now the patch is submitted here:
>>>>> https://sourceware.org/pipermail/binutils/2022-September/122713.html
>>>>>
>>>>
>>>> Great. Given the severity of this bug, I imagine that building the
>>>> LoongArch kernel will require a version of binutils that carries this
>>>> fix.
>>>>
>>>> Therefore, i will revert back to the original approach for accessing
>>>> uncompressed_size, using an extern declaration with an __aligned(1)
>>>> attribute.
>>>>
>>>>> And I have some other questions about kexec: kexec should jump to the
>>>>> elf entry or the pe entry? I think is the elf entry, because if we
>>>>> jump to the pe entry, then SVAM will be executed twice (but it should
>>>>> be executed only once). However, how can we jump to the elf entry if
>>>>> we use zboot? Maybe it is kexec-tool's responsibility to decompress
>>>>> the zboot kernel image?
>>>>>
>>>>
>>>> Yes, very good point. Kexec kernels cannot boot via the EFI entry
>>>> point, as the boot services will already be shutdown. So the kexec
>>>> kernel needs to boot via the same entrypoint in the core kernel that
>>>> the EFI stub calls when it hands over.
>>>>
>>>> For the EFI zboot image in particular, we will need to teach kexec how
>>>> to decompress them. The zboot image has a header that
>>>> a) describes it as a EFI linux zimg
>>>> b) describes the start and end offset of the compressed payload
>>>> c) describes which compression algorithm was used.
>>>>
>>>> This means that any non-EFI loader (including kexec) should be able to
>>>> extract the inner PE/COFF image and decompress it. For arm64 and
>>>> RISC-V, this is sufficient as the EFI and raw images are the same. For
>>>> LoongArch, I suppose it means we need a way to enter the core kernel
>>>> directly via the entrypoint that the EFI stub uses when handing over
>>>> (and pass the original DT argument so the kexec kernel has access to
>>>> the EFI and ACPI firmware tables)
>>> OK, then is this implementation [1] acceptable? I remember that you
>>> said the MS-DOS header shouldn't contain other information, so I guess
>>> this is unacceptable?
>>>
>>
>> No, this looks reasonable to me. I objected to using magic numbers in
>> the 'pure PE' view of the image, as it does not make sense for a pure
>> PE loader such as GRUB to rely on such metadata.
>>
>> In this case (like on arm64), we are dealing with something else: we
>> need to identify the image to the kernel itself, and here, using the
>> unused space in the MS-DOS header is fine.
>>
>>> [1] https://lore.kernel.org/loongarch/c4dbb14a-5580-1e47-3d15-5d2079e88404@loongson.cn/T/#mb8c1dc44f7fa2d3ef638877f0cd3f958f0be96ad
> OK, then there is no big problem here. And I found that arm64/riscv
> don't need the kernel entry point in the header. I don't know why, but
> I think it implies that a unified layout across architectures is
> unnecessary, and I prefer to put the kernel entry point before
> effective kernel size. :)

The kernel entry point is added because LoongArch has not implemented
purgatory in kexec-tools, so I want to get it from the head through a
simpler method, similar to the elf image operation.

Youling.

>
> Huacai
>
>>>
>>> Huacai


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-09-05  7:28                 ` Ard Biesheuvel
@ 2022-09-05 18:07                   ` WANG Xuerui
  2022-09-05 18:35                     ` Ard Biesheuvel
  0 siblings, 1 reply; 26+ messages in thread
From: WANG Xuerui @ 2022-09-05 18:07 UTC (permalink / raw)
  To: Ard Biesheuvel, Huacai Chen
  Cc: Xi Ruoyao, Huacai Chen, Arnd Bergmann, loongarch, linux-arch,
	Xuefeng Li, Guo Ren, Jiaxun Yang, linux-efi, LKML

On 9/5/22 15:28, Ard Biesheuvel wrote:
> [snip]
>>>>>> And I have some other questions about kexec: kexec should jump to the
>>>>>> elf entry or the pe entry? I think is the elf entry, because if we
>>>>>> jump to the pe entry, then SVAM will be executed twice (but it should
>>>>>> be executed only once). However, how can we jump to the elf entry if
>>>>>> we use zboot? Maybe it is kexec-tool's responsibility to decompress
>>>>>> the zboot kernel image?
>>>>>>
>>>>> Yes, very good point. Kexec kernels cannot boot via the EFI entry
>>>>> point, as the boot services will already be shutdown. So the kexec
>>>>> kernel needs to boot via the same entrypoint in the core kernel that
>>>>> the EFI stub calls when it hands over.
>>>>>
>>>>> For the EFI zboot image in particular, we will need to teach kexec how
>>>>> to decompress them. The zboot image has a header that
>>>>> a) describes it as a EFI linux zimg
>>>>> b) describes the start and end offset of the compressed payload
>>>>> c) describes which compression algorithm was used.
>>>>>
>>>>> This means that any non-EFI loader (including kexec) should be able to
>>>>> extract the inner PE/COFF image and decompress it. For arm64 and
>>>>> RISC-V, this is sufficient as the EFI and raw images are the same. For
>>>>> LoongArch, I suppose it means we need a way to enter the core kernel
>>>>> directly via the entrypoint that the EFI stub uses when handing over
>>>>> (and pass the original DT argument so the kexec kernel has access to
>>>>> the EFI and ACPI firmware tables)
>>>> OK, then is this implementation [1] acceptable? I remember that you
>>>> said the MS-DOS header shouldn't contain other information, so I guess
>>>> this is unacceptable?
>>>>
>>> No, this looks reasonable to me. I objected to using magic numbers in
>>> the 'pure PE' view of the image, as it does not make sense for a pure
>>> PE loader such as GRUB to rely on such metadata.
>>>
>>> In this case (like on arm64), we are dealing with something else: we
>>> need to identify the image to the kernel itself, and here, using the
>>> unused space in the MS-DOS header is fine.
>>>
>>>> [1] https://lore.kernel.org/loongarch/c4dbb14a-5580-1e47-3d15-5d2079e88404@loongson.cn/T/#mb8c1dc44f7fa2d3ef638877f0cd3f958f0be96ad
>> OK, then there is no big problem here. And I found that arm64/riscv
>> don't need the kernel entry point in the header. I don't know why, but
>> I think it implies that a unified layout across architectures is
>> unnecessary, and I prefer to put the kernel entry point before
>> effective kernel size. :)
>>
> It is fine to put the entry point offset in the header. arm64 and
> RISC-V don't need this because the first instructions are a pseudo-NOP
> (an instruction that does nothing but its binary encoding looks like
> 'MZ..') and a jump to the actual entry point.

FYI the same trick also works for LoongArch: the code "MZ\x00\x00" i.e. 
00005a4d is in fact "ext.w.h $t1, $t6", which is going to simply trash 
one temporary register without any other effect, so a similar jump to 
the actual entrypoint could follow.

This instruction is available for both LA32 and LA64. The only subset 
without it is the LA32 Primary, which is meant for university courses 
and probably would never run UEFI, so the instruction is safe to use.

P.S. If we'd go the extra mile just for ensuring the instruction works 
on every possible LoongArch core, due to the prefix construction of 
LoongArch encoding, we could just change the bytes toward the MSB (so we 
keep the "MZ" with ease) and still only trash $t1. For example 
"MZ\x10\x00" or 00105a4d is "add.w $t1, $t6, $fp", which is similarly 
harmless, but this time it works on even coursework cores!

-- 
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V3] LoongArch: Add efistub booting support
  2022-09-05 18:07                   ` WANG Xuerui
@ 2022-09-05 18:35                     ` Ard Biesheuvel
  0 siblings, 0 replies; 26+ messages in thread
From: Ard Biesheuvel @ 2022-09-05 18:35 UTC (permalink / raw)
  To: WANG Xuerui
  Cc: Huacai Chen, Xi Ruoyao, Huacai Chen, Arnd Bergmann, loongarch,
	linux-arch, Xuefeng Li, Guo Ren, Jiaxun Yang, linux-efi, LKML

On Mon, 5 Sept 2022 at 20:08, WANG Xuerui <kernel@xen0n.name> wrote:
>
> On 9/5/22 15:28, Ard Biesheuvel wrote:
> > [snip]
> >>>>>> And I have some other questions about kexec: kexec should jump to the
> >>>>>> elf entry or the pe entry? I think is the elf entry, because if we
> >>>>>> jump to the pe entry, then SVAM will be executed twice (but it should
> >>>>>> be executed only once). However, how can we jump to the elf entry if
> >>>>>> we use zboot? Maybe it is kexec-tool's responsibility to decompress
> >>>>>> the zboot kernel image?
> >>>>>>
> >>>>> Yes, very good point. Kexec kernels cannot boot via the EFI entry
> >>>>> point, as the boot services will already be shutdown. So the kexec
> >>>>> kernel needs to boot via the same entrypoint in the core kernel that
> >>>>> the EFI stub calls when it hands over.
> >>>>>
> >>>>> For the EFI zboot image in particular, we will need to teach kexec how
> >>>>> to decompress them. The zboot image has a header that
> >>>>> a) describes it as a EFI linux zimg
> >>>>> b) describes the start and end offset of the compressed payload
> >>>>> c) describes which compression algorithm was used.
> >>>>>
> >>>>> This means that any non-EFI loader (including kexec) should be able to
> >>>>> extract the inner PE/COFF image and decompress it. For arm64 and
> >>>>> RISC-V, this is sufficient as the EFI and raw images are the same. For
> >>>>> LoongArch, I suppose it means we need a way to enter the core kernel
> >>>>> directly via the entrypoint that the EFI stub uses when handing over
> >>>>> (and pass the original DT argument so the kexec kernel has access to
> >>>>> the EFI and ACPI firmware tables)
> >>>> OK, then is this implementation [1] acceptable? I remember that you
> >>>> said the MS-DOS header shouldn't contain other information, so I guess
> >>>> this is unacceptable?
> >>>>
> >>> No, this looks reasonable to me. I objected to using magic numbers in
> >>> the 'pure PE' view of the image, as it does not make sense for a pure
> >>> PE loader such as GRUB to rely on such metadata.
> >>>
> >>> In this case (like on arm64), we are dealing with something else: we
> >>> need to identify the image to the kernel itself, and here, using the
> >>> unused space in the MS-DOS header is fine.
> >>>
> >>>> [1] https://lore.kernel.org/loongarch/c4dbb14a-5580-1e47-3d15-5d2079e88404@loongson.cn/T/#mb8c1dc44f7fa2d3ef638877f0cd3f958f0be96ad
> >> OK, then there is no big problem here. And I found that arm64/riscv
> >> don't need the kernel entry point in the header. I don't know why, but
> >> I think it implies that a unified layout across architectures is
> >> unnecessary, and I prefer to put the kernel entry point before
> >> effective kernel size. :)
> >>
> > It is fine to put the entry point offset in the header. arm64 and
> > RISC-V don't need this because the first instructions are a pseudo-NOP
> > (an instruction that does nothing but its binary encoding looks like
> > 'MZ..') and a jump to the actual entry point.
>
> FYI the same trick also works for LoongArch: the code "MZ\x00\x00" i.e.
> 00005a4d is in fact "ext.w.h $t1, $t6", which is going to simply trash
> one temporary register without any other effect, so a similar jump to
> the actual entrypoint could follow.
>
> This instruction is available for both LA32 and LA64. The only subset
> without it is the LA32 Primary, which is meant for university courses
> and probably would never run UEFI, so the instruction is safe to use.
>
> P.S. If we'd go the extra mile just for ensuring the instruction works
> on every possible LoongArch core, due to the prefix construction of
> LoongArch encoding, we could just change the bytes toward the MSB (so we
> keep the "MZ" with ease) and still only trash $t1. For example
> "MZ\x10\x00" or 00105a4d is "add.w $t1, $t6, $fp", which is similarly
> harmless, but this time it works on even coursework cores!
>

I don't think this is necessary. On arm64, the boot protocol was
already defined when the EFI stub requirements became apparent, and
RISC-V just copied arm64 for some reason.

There is no reason for the 'bare metal' image to be executable from
offset 0x0: in fact, it is better to restrict executable permissions
to code regions, and treat the header as a data region, which is what
it is fundamentally.

In fact, if there is a need to duplicate this information (given that
the PE/COFF header also carries the same information), I would
recommend describing the code (R-X) and data (RW-) regions, as well as
the entry point, and potentially permit the image to be booted with
memory protections enabled.

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2022-09-05 18:36 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-19 10:20 [PATCH V3] LoongArch: Add efistub booting support Huacai Chen
2022-08-22 10:44 ` Ard Biesheuvel
2022-08-22 18:03   ` Ard Biesheuvel
2022-08-23  3:11     ` Huacai Chen
     [not found]       ` <CAAhV-H7-JGGjcFBenpwUz1itZvFK9f1pH88b1dcRtPcGKToojA@mail.gmail.com>
     [not found]         ` <CAMj1kXFEaM7KULxygTABRXB2HHnmRbpSq4N3Q4bYaPOtJxEN2g@mail.gmail.com>
     [not found]           ` <CAAhV-H4CuCSTv8G+bBy52P7kWEc9mapatJ-83mrLYmozG+5SXA@mail.gmail.com>
     [not found]             ` <CAMj1kXF8u=M6R_9pCciY+5oWj6VbLcMEFFC=JYQm5aq1-c8eRg@mail.gmail.com>
2022-08-25  9:47               ` Huacai Chen
2022-08-25  9:51                 ` Ard Biesheuvel
2022-08-25 10:13                   ` Feiyang Chen
2022-08-25 10:22                     ` Ard Biesheuvel
2022-08-25 11:54                   ` Huacai Chen
2022-08-23  2:04   ` Huacai Chen
2022-08-27  4:41 ` Xi Ruoyao
2022-08-27  7:13   ` Ard Biesheuvel
2022-09-01 10:40     ` Huacai Chen
2022-09-04 13:24       ` Huacai Chen
2022-09-04 21:59         ` Ard Biesheuvel
2022-09-05  3:50           ` Huacai Chen
2022-09-05  4:34             ` Youling Tang
2022-09-05  6:28               ` Youling Tang
2022-09-05  7:04                 ` Ard Biesheuvel
2022-09-05  7:03               ` Ard Biesheuvel
2022-09-05  7:02             ` Ard Biesheuvel
2022-09-05  7:24               ` Huacai Chen
2022-09-05  7:28                 ` Ard Biesheuvel
2022-09-05 18:07                   ` WANG Xuerui
2022-09-05 18:35                     ` Ard Biesheuvel
2022-09-05  7:34                 ` Youling Tang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.