Add a post-processing step to compilation of KVM nVHE hyp code which
calls a custom host tool (gen-hyprel) on the partially linked object
file (hyp sections' names prefixed).
The tool lists all R_AARCH64_ABS64 data relocations targeting hyp
sections and generates an assembly file that will form a new section
.hyp.reloc in the kernel binary. The new section contains an array of
32-bit offsets to the positions targeted by these relocations.
Since these addresses of those positions will not be determined until
linking of `vmlinux`, each 32-bit entry carries a R_AARCH64_PREL32
relocation with addend <section_base_sym> + <r_offset>. The linker of
`vmlinux` will therefore fill the slot accordingly.
This relocation data will be used at runtime to convert the kernel VAs
at those positions to hyp VAs.
Signed-off-by: David Brazdil <[email protected]>
---
arch/arm64/kernel/vmlinux.lds.S | 11 +
arch/arm64/kvm/hyp/nvhe/.gitignore | 2 +
arch/arm64/kvm/hyp/nvhe/Makefile | 28 +-
arch/arm64/kvm/hyp/nvhe/gen-hyprel.c | 413 +++++++++++++++++++++++++++
4 files changed, 451 insertions(+), 3 deletions(-)
create mode 100644 arch/arm64/kvm/hyp/nvhe/gen-hyprel.c
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 9672b54bba7c..636ca45aa1d4 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -43,10 +43,19 @@ jiffies = jiffies_64;
HYP_SECTION_NAME(.data..percpu) : { \
*(HYP_SECTION_NAME(.data..percpu)) \
}
+
+#define HYPERVISOR_RELOC_SECTION \
+ .hyp.reloc : ALIGN(4) { \
+ __hyp_reloc_begin = .; \
+ *(.hyp.reloc) \
+ __hyp_reloc_end = .; \
+ }
+
#else /* CONFIG_KVM */
#define HYPERVISOR_EXTABLE
#define HYPERVISOR_DATA_SECTIONS
#define HYPERVISOR_PERCPU_SECTION
+#define HYPERVISOR_RELOC_SECTION
#endif
#define HYPERVISOR_TEXT \
@@ -217,6 +226,8 @@ SECTIONS
PERCPU_SECTION(L1_CACHE_BYTES)
HYPERVISOR_PERCPU_SECTION
+ HYPERVISOR_RELOC_SECTION
+
.rela.dyn : ALIGN(8) {
*(.rela .rela*)
}
diff --git a/arch/arm64/kvm/hyp/nvhe/.gitignore b/arch/arm64/kvm/hyp/nvhe/.gitignore
index 695d73d0249e..5b6c43cc96f8 100644
--- a/arch/arm64/kvm/hyp/nvhe/.gitignore
+++ b/arch/arm64/kvm/hyp/nvhe/.gitignore
@@ -1,2 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
+gen-hyprel
hyp.lds
+hyp-reloc.S
diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
index 1f1e351c5fe2..268be1376f74 100644
--- a/arch/arm64/kvm/hyp/nvhe/Makefile
+++ b/arch/arm64/kvm/hyp/nvhe/Makefile
@@ -6,6 +6,8 @@
asflags-y := -D__KVM_NVHE_HYPERVISOR__
ccflags-y := -D__KVM_NVHE_HYPERVISOR__
+hostprogs := gen-hyprel
+
obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o \
hyp-main.o hyp-smp.o psci-relay.o
obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \
@@ -19,7 +21,7 @@ obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \
hyp-obj := $(patsubst %.o,%.nvhe.o,$(obj-y))
obj-y := kvm_nvhe.o
-extra-y := $(hyp-obj) kvm_nvhe.tmp.o hyp.lds
+extra-y := $(hyp-obj) kvm_nvhe.tmp.o kvm_nvhe.rel.o hyp.lds hyp-reloc.S hyp-reloc.o
# 1) Compile all source files to `.nvhe.o` object files. The file extension
# avoids file name clashes for files shared with VHE.
@@ -42,11 +44,31 @@ LDFLAGS_kvm_nvhe.tmp.o := -r -T
$(obj)/kvm_nvhe.tmp.o: $(obj)/hyp.lds $(addprefix $(obj)/,$(hyp-obj)) FORCE
$(call if_changed,ld)
-# 4) Produce the final 'kvm_nvhe.o', ready to be linked into 'vmlinux'.
+# 4) Generate list of hyp code/data positions that need to be relocated at
+# runtime. Because the hypervisor is part of the kernel binary, relocations
+# produce a kernel VA. We enumerate relocations targeting hyp at build time
+# and convert the kernel VAs at those positions to hyp VAs.
+$(obj)/hyp-reloc.S: $(obj)/kvm_nvhe.tmp.o $(obj)/gen-hyprel
+ $(call if_changed,hyprel)
+
+# 5) Compile hyp-reloc.S and link it into the existing partially linked object.
+# The object file now contains a section with pointers to hyp positions that
+# will contain kernel VAs at runtime. These pointers have relocations on them
+# so that they get updated as the hyp object is linked into `vmlinux`.
+LDFLAGS_kvm_nvhe.rel.o := -r
+$(obj)/kvm_nvhe.rel.o: $(obj)/kvm_nvhe.tmp.o $(obj)/hyp-reloc.o FORCE
+ $(call if_changed,ld)
+
+# 6) Produce the final 'kvm_nvhe.o', ready to be linked into 'vmlinux'.
# Prefixes names of ELF symbols with '__kvm_nvhe_'.
-$(obj)/kvm_nvhe.o: $(obj)/kvm_nvhe.tmp.o FORCE
+$(obj)/kvm_nvhe.o: $(obj)/kvm_nvhe.rel.o FORCE
$(call if_changed,hypcopy)
+# The HYPREL command calls `gen-hyprel` to generate an assembly file with
+# a list of relocations targeting hyp code/data.
+quiet_cmd_hyprel = HYPREL $@
+ cmd_hyprel = $(obj)/gen-hyprel $< > $@
+
# The HYPCOPY command uses `objcopy` to prefix all ELF symbol names
# to avoid clashes with VHE code/data.
quiet_cmd_hypcopy = HYPCOPY $@
diff --git a/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c b/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c
new file mode 100644
index 000000000000..58fe31fdba8e
--- /dev/null
+++ b/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c
@@ -0,0 +1,413 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2020 - Google LLC
+ * Author: David Brazdil <[email protected]>
+ *
+ * Generates relocation information used by the kernel to convert
+ * absolute addresses in hyp data from kernel VAs to hyp VAs.
+ *
+ * This is necessary because hyp code is linked into the same binary
+ * as the kernel but executes under different memory mappings.
+ * If the compiler used absolute addressing, those addresses need to
+ * be converted before they are used by hyp code.
+ *
+ * The input of this program is the relocatable ELF object containing
+ * all hyp code/data, not yet linked into vmlinux. Hyp section names
+ * should have been prefixed with `.hyp` at this point.
+ *
+ * The output (printed to stdout) is an assembly file containing
+ * an array of 32-bit integers and static relocations that instruct
+ * the linker of `vmlinux` to populate the array entries with offsets
+ * to positions in the kernel binary containing VAs used by hyp code.
+ *
+ * Note that dynamic relocations could be used for the same purpose.
+ * However, those are only generated if CONFIG_RELOCATABLE=y.
+ */
+
+#include <elf.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#define HYP_SECTION_PREFIX ".hyp"
+#define HYP_RELOC_SECTION ".hyp.reloc"
+#define HYP_SECTION_SYMBOL_PREFIX "__hyp_section_"
+
+/*
+ * AArch64 relocation type constants.
+ * Included in case these are not defined in the host toolchain.
+ */
+#ifndef R_AARCH64_ABS64
+#define R_AARCH64_ABS64 257
+#endif
+#ifndef R_AARCH64_LD_PREL_LO19
+#define R_AARCH64_LD_PREL_LO19 273
+#endif
+#ifndef R_AARCH64_ADR_PREL_LO21
+#define R_AARCH64_ADR_PREL_LO21 274
+#endif
+#ifndef R_AARCH64_ADR_PREL_PG_HI21
+#define R_AARCH64_ADR_PREL_PG_HI21 275
+#endif
+#ifndef R_AARCH64_ADR_PREL_PG_HI21_NC
+#define R_AARCH64_ADR_PREL_PG_HI21_NC 276
+#endif
+#ifndef R_AARCH64_ADD_ABS_LO12_NC
+#define R_AARCH64_ADD_ABS_LO12_NC 277
+#endif
+#ifndef R_AARCH64_LDST8_ABS_LO12_NC
+#define R_AARCH64_LDST8_ABS_LO12_NC 278
+#endif
+#ifndef R_AARCH64_TSTBR14
+#define R_AARCH64_TSTBR14 279
+#endif
+#ifndef R_AARCH64_CONDBR19
+#define R_AARCH64_CONDBR19 280
+#endif
+#ifndef R_AARCH64_JUMP26
+#define R_AARCH64_JUMP26 282
+#endif
+#ifndef R_AARCH64_CALL26
+#define R_AARCH64_CALL26 283
+#endif
+#ifndef R_AARCH64_LDST16_ABS_LO12_NC
+#define R_AARCH64_LDST16_ABS_LO12_NC 284
+#endif
+#ifndef R_AARCH64_LDST32_ABS_LO12_NC
+#define R_AARCH64_LDST32_ABS_LO12_NC 285
+#endif
+#ifndef R_AARCH64_LDST64_ABS_LO12_NC
+#define R_AARCH64_LDST64_ABS_LO12_NC 286
+#endif
+#ifndef R_AARCH64_MOVW_PREL_G0
+#define R_AARCH64_MOVW_PREL_G0 287
+#endif
+#ifndef R_AARCH64_MOVW_PREL_G0_NC
+#define R_AARCH64_MOVW_PREL_G0_NC 288
+#endif
+#ifndef R_AARCH64_MOVW_PREL_G1
+#define R_AARCH64_MOVW_PREL_G1 289
+#endif
+#ifndef R_AARCH64_MOVW_PREL_G1_NC
+#define R_AARCH64_MOVW_PREL_G1_NC 290
+#endif
+#ifndef R_AARCH64_MOVW_PREL_G2
+#define R_AARCH64_MOVW_PREL_G2 291
+#endif
+#ifndef R_AARCH64_MOVW_PREL_G2_NC
+#define R_AARCH64_MOVW_PREL_G2_NC 292
+#endif
+#ifndef R_AARCH64_MOVW_PREL_G3
+#define R_AARCH64_MOVW_PREL_G3 293
+#endif
+#ifndef R_AARCH64_LDST128_ABS_LO12_NC
+#define R_AARCH64_LDST128_ABS_LO12_NC 299
+#endif
+
+/* Global state of the processed ELF. */
+static struct {
+ const char *path;
+ char *begin;
+ size_t size;
+ Elf64_Ehdr *ehdr;
+ Elf64_Shdr *sh_table;
+ const char *sh_string;
+} elf;
+
+#define fatal_error(fmt, ...) \
+ ({ \
+ fprintf(stderr, "error: %s: " fmt "\n", \
+ elf.path, ## __VA_ARGS__); \
+ exit(EXIT_FAILURE); \
+ __builtin_unreachable(); \
+ })
+
+#define fatal_perror(msg) \
+ ({ \
+ fprintf(stderr, "error: %s: " msg ": %s\n", \
+ elf.path, strerror(errno)); \
+ exit(EXIT_FAILURE); \
+ __builtin_unreachable(); \
+ })
+
+#define assert_op(lhs, rhs, fmt, op) \
+ ({ \
+ typeof(lhs) _lhs = (lhs); \
+ typeof(rhs) _rhs = (rhs); \
+ \
+ if (!(_lhs op _rhs)) { \
+ fatal_error("assertion " #lhs " " #op " " #rhs \
+ " failed (lhs=" fmt ", rhs=" fmt \
+ ", line=%d)", _lhs, _rhs, __LINE__); \
+ } \
+ })
+
+#define assert_eq(lhs, rhs, fmt) assert_op(lhs, rhs, fmt, ==)
+#define assert_ne(lhs, rhs, fmt) assert_op(lhs, rhs, fmt, !=)
+#define assert_lt(lhs, rhs, fmt) assert_op(lhs, rhs, fmt, <)
+#define assert_ge(lhs, rhs, fmt) assert_op(lhs, rhs, fmt, >=)
+
+/*
+ * Return a pointer of a given type at a given offset from
+ * the beginning of the ELF file.
+ */
+#define elf_ptr(type, off) ((type *)(elf.begin + (off)))
+
+/* Iterate over all sections in the ELF. */
+#define for_each_section(var) \
+ for (var = elf.sh_table; var < elf.sh_table + elf.ehdr->e_shnum; ++var)
+
+/* Iterate over all Elf64_Rela relocations in a given section. */
+#define for_each_rela(shdr, var) \
+ for (var = elf_ptr(Elf64_Rela, shdr->sh_offset); \
+ var < elf_ptr(Elf64_Rela, shdr->sh_offset + shdr->sh_size); var++)
+
+/* True if a string starts with a given prefix. */
+static inline bool starts_with(const char *str, const char *prefix)
+{
+ return memcmp(str, prefix, strlen(prefix)) == 0;
+}
+
+/* Returns a string containing the name of a given section. */
+static inline const char *section_name(Elf64_Shdr *shdr)
+{
+ return elf.sh_string + shdr->sh_name;
+}
+
+/* Returns a pointer to the first byte of section data. */
+static inline const char *section_begin(Elf64_Shdr *shdr)
+{
+ return elf_ptr(char, shdr->sh_offset);
+}
+
+/* Find a section by its offset from the beginning of the file. */
+static inline Elf64_Shdr *section_by_off(Elf64_Off off)
+{
+ assert_ne(off, 0UL, "%lu");
+ return elf_ptr(Elf64_Shdr, off);
+}
+
+/* Find a section by its index. */
+static inline Elf64_Shdr *section_by_idx(uint16_t idx)
+{
+ assert_ne(idx, SHN_UNDEF, "%u");
+ return &elf.sh_table[idx];
+}
+
+/*
+ * Memory-map the given ELF file, perform sanity checks, and
+ * populate global state.
+ */
+static void init_elf(const char *path)
+{
+ int fd, ret;
+ struct stat stat;
+
+ /* Store path in the global struct for error printing. */
+ elf.path = path;
+
+ /* Open the ELF file. */
+ fd = open(path, O_RDONLY);
+ if (fd < 0)
+ fatal_perror("Could not open ELF file");
+
+ /* Get status of ELF file to obtain its size. */
+ ret = fstat(fd, &stat);
+ if (ret < 0) {
+ close(fd);
+ fatal_perror("Could not get status of ELF file");
+ }
+
+ /* mmap() the entire ELF file read-only at an arbitrary address. */
+ elf.begin = mmap(0, stat.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+ if (elf.begin == MAP_FAILED) {
+ close(fd);
+ fatal_perror("Could not mmap ELF file");
+ }
+
+ /* mmap() was successful, close the FD. */
+ close(fd);
+
+ /* Get pointer to the ELF header. */
+ assert_ge(stat.st_size, sizeof(*elf.ehdr), "%lu");
+ elf.ehdr = elf_ptr(Elf64_Ehdr, 0);
+
+ /* Check the ELF magic. */
+ assert_eq(elf.ehdr->e_ident[EI_MAG0], ELFMAG0, "0x%x");
+ assert_eq(elf.ehdr->e_ident[EI_MAG1], ELFMAG1, "0x%x");
+ assert_eq(elf.ehdr->e_ident[EI_MAG2], ELFMAG2, "0x%x");
+ assert_eq(elf.ehdr->e_ident[EI_MAG3], ELFMAG3, "0x%x");
+
+ /* Sanity check that this is an ELF64 relocatable object for AArch64. */
+ assert_eq(elf.ehdr->e_ident[EI_CLASS], ELFCLASS64, "%u");
+ assert_eq(elf.ehdr->e_ident[EI_DATA], ELFDATA2LSB, "%u");
+ assert_eq(elf.ehdr->e_type, ET_REL, "%u");
+ assert_eq(elf.ehdr->e_machine, EM_AARCH64, "%u");
+
+ /* Populate fields of the global struct. */
+ elf.sh_table = section_by_off(elf.ehdr->e_shoff);
+ elf.sh_string = section_begin(section_by_idx(elf.ehdr->e_shstrndx));
+}
+
+/* Print the prologue of the output ASM file. */
+static void emit_prologue(void)
+{
+ printf(".data\n"
+ ".pushsection " HYP_RELOC_SECTION ", \"a\"\n");
+}
+
+/* Print ASM statements needed as a prologue to a processed hyp section. */
+static void emit_section_prologue(const char *sh_orig_name)
+{
+ /* Declare the hyp section symbol. */
+ printf(".global %s%s\n", HYP_SECTION_SYMBOL_PREFIX, sh_orig_name);
+}
+
+/*
+ * Print ASM statements to create a hyp relocation entry for a given
+ * R_AARCH64_ABS64 relocation.
+ *
+ * The linker of vmlinux will populate the position given by `rela` with
+ * an absolute 64-bit kernel VA. If the kernel is relocatable, it will
+ * also generate a dynamic relocation entry so that the kernel can shift
+ * the address at runtime for KASLR.
+ *
+ * Emit a 32-bit offset from the current address to the position given
+ * by `rela`. This way the kernel can iterate over all kernel VAs used
+ * by hyp at runtime and convert them to hyp VAs. However, that offset
+ * will not be known until linking of `vmlinux`, so emit a PREL32
+ * relocation referencing a symbol that the hyp linker script put at
+ * the beginning of the relocated section + the offset from `rela`.
+ */
+static void emit_rela_abs64(Elf64_Rela *rela, const char *sh_orig_name)
+{
+ /* Offset of this reloc from the beginning of HYP_RELOC_SECTION. */
+ static size_t reloc_offset;
+
+ /* Create storage for the 32-bit offset. */
+ printf(".word 0\n");
+
+ /*
+ * Create a PREL32 relocation which instructs the linker of `vmlinux`
+ * to insert offset to position <base> + <offset>, where <base> is
+ * a symbol at the beginning of the relocated section, and <offset>
+ * is `rela->r_offset`.
+ */
+ printf(".reloc %lu, R_AARCH64_PREL32, %s%s + 0x%lx\n",
+ reloc_offset, HYP_SECTION_SYMBOL_PREFIX, sh_orig_name,
+ rela->r_offset);
+
+ reloc_offset += 4;
+}
+
+/* Print the epilogue of the output ASM file. */
+static void emit_epilogue(void)
+{
+ printf(".popsection\n");
+}
+
+/*
+ * Iterate over all RELA relocations in a given section and emit
+ * hyp relocation data for all absolute addresses in hyp code/data.
+ *
+ * Static relocations that generate PC-relative-addressing are ignored.
+ * Failure is reported for unexpected relocation types.
+ */
+static void emit_rela_section(Elf64_Shdr *sh_rela)
+{
+ Elf64_Shdr *sh_orig = &elf.sh_table[sh_rela->sh_info];
+ const char *sh_orig_name = section_name(sh_orig);
+ Elf64_Rela *rela;
+
+ /* Skip all non-hyp sections. */
+ if (!starts_with(sh_orig_name, HYP_SECTION_PREFIX))
+ return;
+
+ emit_section_prologue(sh_orig_name);
+
+ for_each_rela(sh_rela, rela) {
+ uint32_t type = (uint32_t)rela->r_info;
+
+ /* Check that rela points inside the relocated section. */
+ assert_lt(rela->r_offset, sh_orig->sh_size, "0x%lx");
+
+ switch (type) {
+ /*
+ * Data relocations to generate absolute addressing.
+ * Emit a hyp relocation.
+ */
+ case R_AARCH64_ABS64:
+ emit_rela_abs64(rela, sh_orig_name);
+ break;
+ /* Allow relocations to generate PC-relative addressing. */
+ case R_AARCH64_LD_PREL_LO19:
+ case R_AARCH64_ADR_PREL_LO21:
+ case R_AARCH64_ADR_PREL_PG_HI21:
+ case R_AARCH64_ADR_PREL_PG_HI21_NC:
+ case R_AARCH64_ADD_ABS_LO12_NC:
+ case R_AARCH64_LDST8_ABS_LO12_NC:
+ case R_AARCH64_LDST16_ABS_LO12_NC:
+ case R_AARCH64_LDST32_ABS_LO12_NC:
+ case R_AARCH64_LDST64_ABS_LO12_NC:
+ case R_AARCH64_LDST128_ABS_LO12_NC:
+ break;
+ /* Allow relative relocations for control-flow instructions. */
+ case R_AARCH64_TSTBR14:
+ case R_AARCH64_CONDBR19:
+ case R_AARCH64_JUMP26:
+ case R_AARCH64_CALL26:
+ break;
+ /* Allow group relocations to create PC-relative offset inline. */
+ case R_AARCH64_MOVW_PREL_G0:
+ case R_AARCH64_MOVW_PREL_G0_NC:
+ case R_AARCH64_MOVW_PREL_G1:
+ case R_AARCH64_MOVW_PREL_G1_NC:
+ case R_AARCH64_MOVW_PREL_G2:
+ case R_AARCH64_MOVW_PREL_G2_NC:
+ case R_AARCH64_MOVW_PREL_G3:
+ break;
+ default:
+ fatal_error("Unexpected RELA type %u", type);
+ }
+ }
+}
+
+/* Iterate over all sections and emit hyp relocation data for RELA sections. */
+static void emit_all_relocs(void)
+{
+ Elf64_Shdr *shdr;
+
+ for_each_section(shdr) {
+ switch (shdr->sh_type) {
+ case SHT_REL:
+ fatal_error("Unexpected SHT_REL section \"%s\"",
+ section_name(shdr));
+ case SHT_RELA:
+ emit_rela_section(shdr);
+ break;
+ }
+ }
+}
+
+int main(int argc, const char **argv)
+{
+ if (argc != 2) {
+ fprintf(stderr, "Usage: %s <elf_input>\n", argv[0]);
+ return EXIT_FAILURE;
+ }
+
+ init_elf(argv[1]);
+
+ emit_prologue();
+ emit_all_relocs();
+ emit_epilogue();
+
+ return EXIT_SUCCESS;
+}
--
2.29.2.729.g45daf8777d-goog
Hi,
On Tue, Jan 05, 2021 at 06:05:37PM +0000, David Brazdil wrote:
> Add a post-processing step to compilation of KVM nVHE hyp code which
> calls a custom host tool (gen-hyprel) on the partially linked object
> file (hyp sections' names prefixed).
>
> The tool lists all R_AARCH64_ABS64 data relocations targeting hyp
> sections and generates an assembly file that will form a new section
> .hyp.reloc in the kernel binary. The new section contains an array of
> 32-bit offsets to the positions targeted by these relocations.
>
> Since these addresses of those positions will not be determined until
> linking of `vmlinux`, each 32-bit entry carries a R_AARCH64_PREL32
> relocation with addend <section_base_sym> + <r_offset>. The linker of
> `vmlinux` will therefore fill the slot accordingly.
>
> This relocation data will be used at runtime to convert the kernel VAs
> at those positions to hyp VAs.
>
> Signed-off-by: David Brazdil <[email protected]>
This patch results in the following error for me.
error: arch/arm64/kvm/hyp/nvhe/kvm_nvhe.tmp.o: assertion elf.ehdr->e_ident[5] == 1 failed (lhs=2, rhs=1, line=250)
The problem is seen when trying to build aarch64 images in big endian
mode.
Te script used to reproduce the problem as well as bisect results are
attached.
Guenter
---
Script used to reproduce the problem:
make-arm64 allmodconfig
echo "CONFIG_CPU_BIG_ENDIAN=y" >> .config
make-arm64 olddefconfig
rm -f arch/arm64/kvm/hyp/nvhe/kvm_nvhe.o
make-arm64 -j arch/arm64/kvm/hyp/nvhe/kvm_nvhe.o
where make-arm64 is:
make -j32 ARCH=arm64 CROSS_COMPILE=aarch64-linux- $*
---
# bad: [b01f250d83f6c3af5c77699dd14e7b48ee0b5383] Add linux-next specific files for 20210129
# good: [6ee1d745b7c9fd573fba142a2efdad76a9f1cb04] Linux 5.11-rc5
git bisect start 'HEAD' 'v5.11-rc5'
# good: [7b7f6e418b7121fd3f05029f843ff1eb76e4cc4d] Merge remote-tracking branch 'crypto/master'
git bisect good 7b7f6e418b7121fd3f05029f843ff1eb76e4cc4d
# good: [4a8b64f4e85b5f2d8e68641ce26af8cf9d9a66af] Merge remote-tracking branch 'keys/keys-next'
git bisect good 4a8b64f4e85b5f2d8e68641ce26af8cf9d9a66af
# bad: [79a914d7f707c1aa5ede7ce38588b32093b2abbe] Merge remote-tracking branch 'thunderbolt/next'
git bisect bad 79a914d7f707c1aa5ede7ce38588b32093b2abbe
# good: [f22712067d6f3d1884186509a942da26d2d52166] Merge remote-tracking branch 'rcu/rcu/next'
git bisect good f22712067d6f3d1884186509a942da26d2d52166
# bad: [4a42e6b0a53bcc4b58a35b8d5df50581e94b1daa] Merge remote-tracking branch 'usb/usb-next'
git bisect bad 4a42e6b0a53bcc4b58a35b8d5df50581e94b1daa
# good: [1a9e38cabd80356ffb98c2c88fec528ea9644fd5] usb: dwc2: Make "trimming xfer length" a debug message
git bisect good 1a9e38cabd80356ffb98c2c88fec528ea9644fd5
# bad: [589e25c6dfdd535e8eba052f7314054f6d75be4a] Merge remote-tracking branch 'drivers-x86/for-next'
git bisect bad 589e25c6dfdd535e8eba052f7314054f6d75be4a
# bad: [08c11d16ca91e770de81e5959e22eb34541f8af9] Merge remote-tracking branch 'percpu/for-next'
git bisect bad 08c11d16ca91e770de81e5959e22eb34541f8af9
# bad: [cc6d8fa3667aa5513dc2bbca896a4c287aa956f3] Merge branch 'kvm-arm64/misc-5.12' into kvmarm-master/next
git bisect bad cc6d8fa3667aa5513dc2bbca896a4c287aa956f3
# bad: [247bc166e6b3b1e4068f120f55582a3aa210cc2d] KVM: arm64: Remove hyp_symbol_addr
git bisect bad 247bc166e6b3b1e4068f120f55582a3aa210cc2d
# bad: [8c49b5d43d4c45ca0bb0d1faa23feef2e76e89fa] KVM: arm64: Generate hyp relocation data
git bisect bad 8c49b5d43d4c45ca0bb0d1faa23feef2e76e89fa
# good: [16174eea2e4fe8247e04c17da682f2034fec0369] KVM: arm64: Set up .hyp.rodata ELF section
git bisect good 16174eea2e4fe8247e04c17da682f2034fec0369
# good: [f7a4825d9569593b9a81f0768313b86175691ef1] KVM: arm64: Add symbol at the beginning of each hyp section
git bisect good f7a4825d9569593b9a81f0768313b86175691ef1
# first bad commit: [8c49b5d43d4c45ca0bb0d1faa23feef2e76e89fa] KVM: arm64: Generate hyp relocation data
Hi Guenter,
Thanks a lot for the heads up.
On 2021-01-29 21:43, Guenter Roeck wrote:
> Hi,
>
> On Tue, Jan 05, 2021 at 06:05:37PM +0000, David Brazdil wrote:
>> Add a post-processing step to compilation of KVM nVHE hyp code which
>> calls a custom host tool (gen-hyprel) on the partially linked object
>> file (hyp sections' names prefixed).
>>
>> The tool lists all R_AARCH64_ABS64 data relocations targeting hyp
>> sections and generates an assembly file that will form a new section
>> .hyp.reloc in the kernel binary. The new section contains an array of
>> 32-bit offsets to the positions targeted by these relocations.
>>
>> Since these addresses of those positions will not be determined until
>> linking of `vmlinux`, each 32-bit entry carries a R_AARCH64_PREL32
>> relocation with addend <section_base_sym> + <r_offset>. The linker of
>> `vmlinux` will therefore fill the slot accordingly.
>>
>> This relocation data will be used at runtime to convert the kernel VAs
>> at those positions to hyp VAs.
>>
>> Signed-off-by: David Brazdil <[email protected]>
>
> This patch results in the following error for me.
>
> error: arch/arm64/kvm/hyp/nvhe/kvm_nvhe.tmp.o: assertion
> elf.ehdr->e_ident[5] == 1 failed (lhs=2, rhs=1, line=250)
>
> The problem is seen when trying to build aarch64 images in big endian
> mode.
Ah, big-endian. of course, the ELF header is in native endianness,
and the sanity checks explode (still much better than generating crap).
I'll have a look shortly. It shouldn't too hard to fix, just a
bit invasive...
Thanks again,
M.
--
Jazz is not dead. It just smells funny...
On Fri, 29 Jan 2021 21:43:25 +0000,
Guenter Roeck <[email protected]> wrote:
>
> Hi,
>
> On Tue, Jan 05, 2021 at 06:05:37PM +0000, David Brazdil wrote:
> > Add a post-processing step to compilation of KVM nVHE hyp code which
> > calls a custom host tool (gen-hyprel) on the partially linked object
> > file (hyp sections' names prefixed).
> >
> > The tool lists all R_AARCH64_ABS64 data relocations targeting hyp
> > sections and generates an assembly file that will form a new section
> > .hyp.reloc in the kernel binary. The new section contains an array of
> > 32-bit offsets to the positions targeted by these relocations.
> >
> > Since these addresses of those positions will not be determined until
> > linking of `vmlinux`, each 32-bit entry carries a R_AARCH64_PREL32
> > relocation with addend <section_base_sym> + <r_offset>. The linker of
> > `vmlinux` will therefore fill the slot accordingly.
> >
> > This relocation data will be used at runtime to convert the kernel VAs
> > at those positions to hyp VAs.
> >
> > Signed-off-by: David Brazdil <[email protected]>
>
> This patch results in the following error for me.
>
> error: arch/arm64/kvm/hyp/nvhe/kvm_nvhe.tmp.o: assertion elf.ehdr->e_ident[5] == 1 failed (lhs=2, rhs=1, line=250)
>
> The problem is seen when trying to build aarch64 images in big endian
> mode.
>
> Te script used to reproduce the problem as well as bisect results are
> attached.
I came up with the following patch, which allows the kernel to link
and boot. I don't have any BE userspace, so I didn't verify that I
could boot a guest (the hypervisor does correctly initialise though).
It's not exactly pretty, but it does the job...
Thanks,
M.
From d80ca05b2ed90fc30d328041692fa80f525c8d12 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <[email protected]>
Date: Sat, 30 Jan 2021 13:07:51 +0000
Subject: [PATCH] KVM: arm64: Make gen-hyprel endianness agnostic
gen-hyprel is, for better or worse, a native-endian program:
it assumes that the ELF data structures are in the host's
endianness, and even assumes that the compiled kernel is
little-endian in one particular case.
None of these assumptions hold true though: people actually build
(use?) BE arm64 kernels, and seem to avoid doing so on BE hosts.
Madness!
In order to solve this, wrap each access to the ELF data structures
with the required byte-swapping magic. This requires to obtain
the kernel data structure, and provide per-endianess wrappers.
This result in a kernel that links and even boots in a model.
Fixes: 8c49b5d43d4c ("KVM: arm64: Generate hyp relocation data")
Reported-by: Guenter Roeck <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
---
arch/arm64/kvm/hyp/nvhe/Makefile | 1 +
arch/arm64/kvm/hyp/nvhe/gen-hyprel.c | 57 ++++++++++++++++++++--------
2 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
index 268be1376f74..09d04dd50eb8 100644
--- a/arch/arm64/kvm/hyp/nvhe/Makefile
+++ b/arch/arm64/kvm/hyp/nvhe/Makefile
@@ -7,6 +7,7 @@ asflags-y := -D__KVM_NVHE_HYPERVISOR__
ccflags-y := -D__KVM_NVHE_HYPERVISOR__
hostprogs := gen-hyprel
+HOST_EXTRACFLAGS += -I$(srctree)/include
obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o \
hyp-main.o hyp-smp.o psci-relay.o
diff --git a/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c b/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c
index 58fe31fdba8e..ead02c6a7628 100644
--- a/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c
+++ b/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c
@@ -25,6 +25,7 @@
*/
#include <elf.h>
+#include <endian.h>
#include <errno.h>
#include <fcntl.h>
#include <stdbool.h>
@@ -36,6 +37,8 @@
#include <sys/stat.h>
#include <unistd.h>
+#include <generated/autoconf.h>
+
#define HYP_SECTION_PREFIX ".hyp"
#define HYP_RELOC_SECTION ".hyp.reloc"
#define HYP_SECTION_SYMBOL_PREFIX "__hyp_section_"
@@ -121,6 +124,28 @@ static struct {
const char *sh_string;
} elf;
+#if defined(CONFIG_CPU_LITTLE_ENDIAN)
+
+#define elf16toh(x) le16toh(x)
+#define elf32toh(x) le32toh(x)
+#define elf64toh(x) le64toh(x)
+
+#define ELFENDIAN ELFDATA2LSB
+
+#elif defined(CONFIG_CPU_BIG_ENDIAN)
+
+#define elf16toh(x) be16toh(x)
+#define elf32toh(x) be32toh(x)
+#define elf64toh(x) be64toh(x)
+
+#define ELFENDIAN ELFDATA2MSB
+
+#else
+
+#error PDP-endian sadly unsupported...
+
+#endif
+
#define fatal_error(fmt, ...) \
({ \
fprintf(stderr, "error: %s: " fmt "\n", \
@@ -162,12 +187,12 @@ static struct {
/* Iterate over all sections in the ELF. */
#define for_each_section(var) \
- for (var = elf.sh_table; var < elf.sh_table + elf.ehdr->e_shnum; ++var)
+ for (var = elf.sh_table; var < elf.sh_table + elf16toh(elf.ehdr->e_shnum); ++var)
/* Iterate over all Elf64_Rela relocations in a given section. */
#define for_each_rela(shdr, var) \
- for (var = elf_ptr(Elf64_Rela, shdr->sh_offset); \
- var < elf_ptr(Elf64_Rela, shdr->sh_offset + shdr->sh_size); var++)
+ for (var = elf_ptr(Elf64_Rela, elf64toh(shdr->sh_offset)); \
+ var < elf_ptr(Elf64_Rela, elf64toh(shdr->sh_offset) + elf64toh(shdr->sh_size)); var++)
/* True if a string starts with a given prefix. */
static inline bool starts_with(const char *str, const char *prefix)
@@ -178,13 +203,13 @@ static inline bool starts_with(const char *str, const char *prefix)
/* Returns a string containing the name of a given section. */
static inline const char *section_name(Elf64_Shdr *shdr)
{
- return elf.sh_string + shdr->sh_name;
+ return elf.sh_string + elf32toh(shdr->sh_name);
}
/* Returns a pointer to the first byte of section data. */
static inline const char *section_begin(Elf64_Shdr *shdr)
{
- return elf_ptr(char, shdr->sh_offset);
+ return elf_ptr(char, elf64toh(shdr->sh_offset));
}
/* Find a section by its offset from the beginning of the file. */
@@ -247,13 +272,13 @@ static void init_elf(const char *path)
/* Sanity check that this is an ELF64 relocatable object for AArch64. */
assert_eq(elf.ehdr->e_ident[EI_CLASS], ELFCLASS64, "%u");
- assert_eq(elf.ehdr->e_ident[EI_DATA], ELFDATA2LSB, "%u");
- assert_eq(elf.ehdr->e_type, ET_REL, "%u");
- assert_eq(elf.ehdr->e_machine, EM_AARCH64, "%u");
+ assert_eq(elf.ehdr->e_ident[EI_DATA], ELFENDIAN, "%u");
+ assert_eq(elf16toh(elf.ehdr->e_type), ET_REL, "%u");
+ assert_eq(elf16toh(elf.ehdr->e_machine), EM_AARCH64, "%u");
/* Populate fields of the global struct. */
- elf.sh_table = section_by_off(elf.ehdr->e_shoff);
- elf.sh_string = section_begin(section_by_idx(elf.ehdr->e_shstrndx));
+ elf.sh_table = section_by_off(elf64toh(elf.ehdr->e_shoff));
+ elf.sh_string = section_begin(section_by_idx(elf16toh(elf.ehdr->e_shstrndx)));
}
/* Print the prologue of the output ASM file. */
@@ -301,8 +326,8 @@ static void emit_rela_abs64(Elf64_Rela *rela, const char *sh_orig_name)
* is `rela->r_offset`.
*/
printf(".reloc %lu, R_AARCH64_PREL32, %s%s + 0x%lx\n",
- reloc_offset, HYP_SECTION_SYMBOL_PREFIX, sh_orig_name,
- rela->r_offset);
+ reloc_offset, HYP_SECTION_SYMBOL_PREFIX, sh_orig_name,
+ elf64toh(rela->r_offset));
reloc_offset += 4;
}
@@ -322,7 +347,7 @@ static void emit_epilogue(void)
*/
static void emit_rela_section(Elf64_Shdr *sh_rela)
{
- Elf64_Shdr *sh_orig = &elf.sh_table[sh_rela->sh_info];
+ Elf64_Shdr *sh_orig = &elf.sh_table[elf32toh(sh_rela->sh_info)];
const char *sh_orig_name = section_name(sh_orig);
Elf64_Rela *rela;
@@ -333,10 +358,10 @@ static void emit_rela_section(Elf64_Shdr *sh_rela)
emit_section_prologue(sh_orig_name);
for_each_rela(sh_rela, rela) {
- uint32_t type = (uint32_t)rela->r_info;
+ uint32_t type = (uint32_t)elf64toh(rela->r_info);
/* Check that rela points inside the relocated section. */
- assert_lt(rela->r_offset, sh_orig->sh_size, "0x%lx");
+ assert_lt(elf64toh(rela->r_offset), elf64toh(sh_orig->sh_size), "0x%lx");
switch (type) {
/*
@@ -385,7 +410,7 @@ static void emit_all_relocs(void)
Elf64_Shdr *shdr;
for_each_section(shdr) {
- switch (shdr->sh_type) {
+ switch (elf32toh(shdr->sh_type)) {
case SHT_REL:
fatal_error("Unexpected SHT_REL section \"%s\"",
section_name(shdr));
--
2.29.2
--
Without deviation from the norm, progress is not possible.
On Sat, Jan 30, 2021 at 01:44:15PM +0000, Marc Zyngier wrote:
> On Fri, 29 Jan 2021 21:43:25 +0000,
> Guenter Roeck <[email protected]> wrote:
> >
> > Hi,
> >
> > On Tue, Jan 05, 2021 at 06:05:37PM +0000, David Brazdil wrote:
> > > Add a post-processing step to compilation of KVM nVHE hyp code which
> > > calls a custom host tool (gen-hyprel) on the partially linked object
> > > file (hyp sections' names prefixed).
> > >
> > > The tool lists all R_AARCH64_ABS64 data relocations targeting hyp
> > > sections and generates an assembly file that will form a new section
> > > .hyp.reloc in the kernel binary. The new section contains an array of
> > > 32-bit offsets to the positions targeted by these relocations.
> > >
> > > Since these addresses of those positions will not be determined until
> > > linking of `vmlinux`, each 32-bit entry carries a R_AARCH64_PREL32
> > > relocation with addend <section_base_sym> + <r_offset>. The linker of
> > > `vmlinux` will therefore fill the slot accordingly.
> > >
> > > This relocation data will be used at runtime to convert the kernel VAs
> > > at those positions to hyp VAs.
> > >
> > > Signed-off-by: David Brazdil <[email protected]>
> >
> > This patch results in the following error for me.
> >
> > error: arch/arm64/kvm/hyp/nvhe/kvm_nvhe.tmp.o: assertion elf.ehdr->e_ident[5] == 1 failed (lhs=2, rhs=1, line=250)
> >
> > The problem is seen when trying to build aarch64 images in big endian
> > mode.
> >
> > Te script used to reproduce the problem as well as bisect results are
> > attached.
>
> I came up with the following patch, which allows the kernel to link
> and boot. I don't have any BE userspace, so I didn't verify that I
> could boot a guest (the hypervisor does correctly initialise though).
>
> It's not exactly pretty, but it does the job...
>
> Thanks,
>
> M.
>
> From d80ca05b2ed90fc30d328041692fa80f525c8d12 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <[email protected]>
> Date: Sat, 30 Jan 2021 13:07:51 +0000
> Subject: [PATCH] KVM: arm64: Make gen-hyprel endianness agnostic
>
> gen-hyprel is, for better or worse, a native-endian program:
> it assumes that the ELF data structures are in the host's
> endianness, and even assumes that the compiled kernel is
> little-endian in one particular case.
>
> None of these assumptions hold true though: people actually build
> (use?) BE arm64 kernels, and seem to avoid doing so on BE hosts.
> Madness!
>
> In order to solve this, wrap each access to the ELF data structures
> with the required byte-swapping magic. This requires to obtain
> the kernel data structure, and provide per-endianess wrappers.
>
> This result in a kernel that links and even boots in a model.
>
> Fixes: 8c49b5d43d4c ("KVM: arm64: Generate hyp relocation data")
> Reported-by: Guenter Roeck <[email protected]>
> Signed-off-by: Marc Zyngier <[email protected]>
Tested-by: Guenter Roeck <[email protected]>
Compiles and boots both big- and little-endian systems in qemu.
Guenter
> ---
> arch/arm64/kvm/hyp/nvhe/Makefile | 1 +
> arch/arm64/kvm/hyp/nvhe/gen-hyprel.c | 57 ++++++++++++++++++++--------
> 2 files changed, 42 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
> index 268be1376f74..09d04dd50eb8 100644
> --- a/arch/arm64/kvm/hyp/nvhe/Makefile
> +++ b/arch/arm64/kvm/hyp/nvhe/Makefile
> @@ -7,6 +7,7 @@ asflags-y := -D__KVM_NVHE_HYPERVISOR__
> ccflags-y := -D__KVM_NVHE_HYPERVISOR__
>
> hostprogs := gen-hyprel
> +HOST_EXTRACFLAGS += -I$(srctree)/include
>
> obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o \
> hyp-main.o hyp-smp.o psci-relay.o
> diff --git a/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c b/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c
> index 58fe31fdba8e..ead02c6a7628 100644
> --- a/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c
> +++ b/arch/arm64/kvm/hyp/nvhe/gen-hyprel.c
> @@ -25,6 +25,7 @@
> */
>
> #include <elf.h>
> +#include <endian.h>
> #include <errno.h>
> #include <fcntl.h>
> #include <stdbool.h>
> @@ -36,6 +37,8 @@
> #include <sys/stat.h>
> #include <unistd.h>
>
> +#include <generated/autoconf.h>
> +
> #define HYP_SECTION_PREFIX ".hyp"
> #define HYP_RELOC_SECTION ".hyp.reloc"
> #define HYP_SECTION_SYMBOL_PREFIX "__hyp_section_"
> @@ -121,6 +124,28 @@ static struct {
> const char *sh_string;
> } elf;
>
> +#if defined(CONFIG_CPU_LITTLE_ENDIAN)
> +
> +#define elf16toh(x) le16toh(x)
> +#define elf32toh(x) le32toh(x)
> +#define elf64toh(x) le64toh(x)
> +
> +#define ELFENDIAN ELFDATA2LSB
> +
> +#elif defined(CONFIG_CPU_BIG_ENDIAN)
> +
> +#define elf16toh(x) be16toh(x)
> +#define elf32toh(x) be32toh(x)
> +#define elf64toh(x) be64toh(x)
> +
> +#define ELFENDIAN ELFDATA2MSB
> +
> +#else
> +
> +#error PDP-endian sadly unsupported...
> +
> +#endif
> +
> #define fatal_error(fmt, ...) \
> ({ \
> fprintf(stderr, "error: %s: " fmt "\n", \
> @@ -162,12 +187,12 @@ static struct {
>
> /* Iterate over all sections in the ELF. */
> #define for_each_section(var) \
> - for (var = elf.sh_table; var < elf.sh_table + elf.ehdr->e_shnum; ++var)
> + for (var = elf.sh_table; var < elf.sh_table + elf16toh(elf.ehdr->e_shnum); ++var)
>
> /* Iterate over all Elf64_Rela relocations in a given section. */
> #define for_each_rela(shdr, var) \
> - for (var = elf_ptr(Elf64_Rela, shdr->sh_offset); \
> - var < elf_ptr(Elf64_Rela, shdr->sh_offset + shdr->sh_size); var++)
> + for (var = elf_ptr(Elf64_Rela, elf64toh(shdr->sh_offset)); \
> + var < elf_ptr(Elf64_Rela, elf64toh(shdr->sh_offset) + elf64toh(shdr->sh_size)); var++)
>
> /* True if a string starts with a given prefix. */
> static inline bool starts_with(const char *str, const char *prefix)
> @@ -178,13 +203,13 @@ static inline bool starts_with(const char *str, const char *prefix)
> /* Returns a string containing the name of a given section. */
> static inline const char *section_name(Elf64_Shdr *shdr)
> {
> - return elf.sh_string + shdr->sh_name;
> + return elf.sh_string + elf32toh(shdr->sh_name);
> }
>
> /* Returns a pointer to the first byte of section data. */
> static inline const char *section_begin(Elf64_Shdr *shdr)
> {
> - return elf_ptr(char, shdr->sh_offset);
> + return elf_ptr(char, elf64toh(shdr->sh_offset));
> }
>
> /* Find a section by its offset from the beginning of the file. */
> @@ -247,13 +272,13 @@ static void init_elf(const char *path)
>
> /* Sanity check that this is an ELF64 relocatable object for AArch64. */
> assert_eq(elf.ehdr->e_ident[EI_CLASS], ELFCLASS64, "%u");
> - assert_eq(elf.ehdr->e_ident[EI_DATA], ELFDATA2LSB, "%u");
> - assert_eq(elf.ehdr->e_type, ET_REL, "%u");
> - assert_eq(elf.ehdr->e_machine, EM_AARCH64, "%u");
> + assert_eq(elf.ehdr->e_ident[EI_DATA], ELFENDIAN, "%u");
> + assert_eq(elf16toh(elf.ehdr->e_type), ET_REL, "%u");
> + assert_eq(elf16toh(elf.ehdr->e_machine), EM_AARCH64, "%u");
>
> /* Populate fields of the global struct. */
> - elf.sh_table = section_by_off(elf.ehdr->e_shoff);
> - elf.sh_string = section_begin(section_by_idx(elf.ehdr->e_shstrndx));
> + elf.sh_table = section_by_off(elf64toh(elf.ehdr->e_shoff));
> + elf.sh_string = section_begin(section_by_idx(elf16toh(elf.ehdr->e_shstrndx)));
> }
>
> /* Print the prologue of the output ASM file. */
> @@ -301,8 +326,8 @@ static void emit_rela_abs64(Elf64_Rela *rela, const char *sh_orig_name)
> * is `rela->r_offset`.
> */
> printf(".reloc %lu, R_AARCH64_PREL32, %s%s + 0x%lx\n",
> - reloc_offset, HYP_SECTION_SYMBOL_PREFIX, sh_orig_name,
> - rela->r_offset);
> + reloc_offset, HYP_SECTION_SYMBOL_PREFIX, sh_orig_name,
> + elf64toh(rela->r_offset));
>
> reloc_offset += 4;
> }
> @@ -322,7 +347,7 @@ static void emit_epilogue(void)
> */
> static void emit_rela_section(Elf64_Shdr *sh_rela)
> {
> - Elf64_Shdr *sh_orig = &elf.sh_table[sh_rela->sh_info];
> + Elf64_Shdr *sh_orig = &elf.sh_table[elf32toh(sh_rela->sh_info)];
> const char *sh_orig_name = section_name(sh_orig);
> Elf64_Rela *rela;
>
> @@ -333,10 +358,10 @@ static void emit_rela_section(Elf64_Shdr *sh_rela)
> emit_section_prologue(sh_orig_name);
>
> for_each_rela(sh_rela, rela) {
> - uint32_t type = (uint32_t)rela->r_info;
> + uint32_t type = (uint32_t)elf64toh(rela->r_info);
>
> /* Check that rela points inside the relocated section. */
> - assert_lt(rela->r_offset, sh_orig->sh_size, "0x%lx");
> + assert_lt(elf64toh(rela->r_offset), elf64toh(sh_orig->sh_size), "0x%lx");
>
> switch (type) {
> /*
> @@ -385,7 +410,7 @@ static void emit_all_relocs(void)
> Elf64_Shdr *shdr;
>
> for_each_section(shdr) {
> - switch (shdr->sh_type) {
> + switch (elf32toh(shdr->sh_type)) {
> case SHT_REL:
> fatal_error("Unexpected SHT_REL section \"%s\"",
> section_name(shdr));
> --
> 2.29.2
>
>
> --
> Without deviation from the norm, progress is not possible.
On Sat, 30 Jan 2021 16:11:04 +0000,
Guenter Roeck <[email protected]> wrote:
>
> On Sat, Jan 30, 2021 at 01:44:15PM +0000, Marc Zyngier wrote:
> > From d80ca05b2ed90fc30d328041692fa80f525c8d12 Mon Sep 17 00:00:00 2001
> > From: Marc Zyngier <[email protected]>
> > Date: Sat, 30 Jan 2021 13:07:51 +0000
> > Subject: [PATCH] KVM: arm64: Make gen-hyprel endianness agnostic
> >
> > gen-hyprel is, for better or worse, a native-endian program:
> > it assumes that the ELF data structures are in the host's
> > endianness, and even assumes that the compiled kernel is
> > little-endian in one particular case.
> >
> > None of these assumptions hold true though: people actually build
> > (use?) BE arm64 kernels, and seem to avoid doing so on BE hosts.
> > Madness!
> >
> > In order to solve this, wrap each access to the ELF data structures
> > with the required byte-swapping magic. This requires to obtain
> > the kernel data structure, and provide per-endianess wrappers.
> >
> > This result in a kernel that links and even boots in a model.
> >
> > Fixes: 8c49b5d43d4c ("KVM: arm64: Generate hyp relocation data")
> > Reported-by: Guenter Roeck <[email protected]>
> > Signed-off-by: Marc Zyngier <[email protected]>
>
> Tested-by: Guenter Roeck <[email protected]>
>
> Compiles and boots both big- and little-endian systems in qemu.
Great, thanks for confirming that it fixed this issue. Now applied to
kvm-arm64/hyp-reloc, and pushed out to kvmarm/next.
M.
--
Without deviation from the norm, progress is not possible.
Thanks for writing the fix, Marc! There are no corner cases in this code so
if it boots, that should be a good indicator that all BE inputs were converted.
Just one little thing I noticed below, otherwise:
Acked-by: David Brazdil <[email protected]>
> arch/arm64/kvm/hyp/nvhe/Makefile | 1 +
> arch/arm64/kvm/hyp/nvhe/gen-hyprel.c | 57 ++++++++++++++++++++--------
> 2 files changed, 42 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
> index 268be1376f74..09d04dd50eb8 100644
> --- a/arch/arm64/kvm/hyp/nvhe/Makefile
> +++ b/arch/arm64/kvm/hyp/nvhe/Makefile
> @@ -7,6 +7,7 @@ asflags-y := -D__KVM_NVHE_HYPERVISOR__
> ccflags-y := -D__KVM_NVHE_HYPERVISOR__
>
> hostprogs := gen-hyprel
> +HOST_EXTRACFLAGS += -I$(srctree)/include
This should be $(objtree), autoconf.h is generated.
David
Hi David,
On 2021-02-01 10:42, David Brazdil wrote:
> Thanks for writing the fix, Marc! There are no corner cases in this
> code so
> if it boots, that should be a good indicator that all BE inputs were
> converted.
>
> Just one little thing I noticed below, otherwise:
> Acked-by: David Brazdil <[email protected]>
>
>> arch/arm64/kvm/hyp/nvhe/Makefile | 1 +
>> arch/arm64/kvm/hyp/nvhe/gen-hyprel.c | 57
>> ++++++++++++++++++++--------
>> 2 files changed, 42 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile
>> b/arch/arm64/kvm/hyp/nvhe/Makefile
>> index 268be1376f74..09d04dd50eb8 100644
>> --- a/arch/arm64/kvm/hyp/nvhe/Makefile
>> +++ b/arch/arm64/kvm/hyp/nvhe/Makefile
>> @@ -7,6 +7,7 @@ asflags-y := -D__KVM_NVHE_HYPERVISOR__
>> ccflags-y := -D__KVM_NVHE_HYPERVISOR__
>>
>> hostprogs := gen-hyprel
>> +HOST_EXTRACFLAGS += -I$(srctree)/include
> This should be $(objtree), autoconf.h is generated.
Ah, well spotted. I build things in place, so it obviously never showed
up.
Now fixed and pushed out with your Ack.
Thanks,
M.
--
Jazz is not dead. It just smells funny...