On 21.11.18 16:51, Daniel Kiper wrote: > On Wed, Nov 14, 2018 at 06:27:36PM +0100, Alexander Graf wrote: >> This patch adds awareness of RISC-V relocations throughout the grub tools >> as well as dynamic linkage and elf->PE relocation conversion support. >> >> Signed-off-by: Alexander Graf <ag...@suse.de> >> >> --- >> >> v2 -> v3: >> >> - Fix riscv32 target >> --- >> grub-core/kern/dl.c | 6 +- >> grub-core/kern/riscv/dl.c | 335 >> ++++++++++++++++++++++++++++++++++++++++++++ >> include/grub/dl.h | 6 +- >> util/grub-mkimagexx.c | 268 +++++++++++++++++++++++++++++++++++ >> util/grub-module-verifier.c | 56 ++++++++ >> 5 files changed, 666 insertions(+), 5 deletions(-) >> create mode 100644 grub-core/kern/riscv/dl.c >> >> diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c >> index f8d58f029..48eb5e7b6 100644 >> --- a/grub-core/kern/dl.c >> +++ b/grub-core/kern/dl.c >> @@ -225,7 +225,7 @@ grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e) >> unsigned i; >> const Elf_Shdr *s; >> grub_size_t tsize = 0, talign = 1; >> -#if !defined (__i386__) && !defined (__x86_64__) >> +#if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv) > > Could not we use __riscv__?
Unfortunately __riscv__ is not defined in recent versions of gcc: https://github.com/riscv/riscv-toolchain-conventions#cc-preprocessor-definitions > >> grub_size_t tramp; >> grub_size_t got; >> grub_err_t err; >> @@ -241,7 +241,7 @@ grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e) >> talign = s->sh_addralign; >> } >> >> -#if !defined (__i386__) && !defined (__x86_64__) >> +#if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv) >> err = grub_arch_dl_get_tramp_got_size (e, &tramp, &got); >> if (err) >> return err; >> @@ -304,7 +304,7 @@ grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e) >> mod->segment = seg; >> } >> } >> -#if !defined (__i386__) && !defined (__x86_64__) >> +#if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv) >> ptr = (char *) ALIGN_UP ((grub_addr_t) ptr, GRUB_ARCH_DL_TRAMP_ALIGN); >> mod->tramp = ptr; >> mod->trampptr = ptr; >> diff --git a/grub-core/kern/riscv/dl.c b/grub-core/kern/riscv/dl.c >> new file mode 100644 >> index 000000000..503b67df1 >> --- /dev/null >> +++ b/grub-core/kern/riscv/dl.c >> @@ -0,0 +1,335 @@ >> +/* dl.c - arch-dependent part of loadable module support */ >> +/* >> + * GRUB -- GRand Unified Bootloader >> + * Copyright (C) 2013 Free Software Foundation, Inc. > > s/2013/2018/ > >> + * >> + * GRUB is free software: you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation, either version 3 of the License, or >> + * (at your option) any later version. >> + * >> + * GRUB is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with GRUB. If not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#include <grub/dl.h> >> +#include <grub/elf.h> >> +#include <grub/misc.h> >> +#include <grub/err.h> >> +#include <grub/mm.h> >> +#include <grub/i18n.h> >> + >> +#define LDR 0x58000050 >> +#define BR 0xd61f0200 >> + >> + > > Please drop this empty line. > >> +/* >> + * Check if EHDR is a valid ELF header. >> + */ >> +grub_err_t >> +grub_arch_dl_check_header (void *ehdr) >> +{ >> + Elf_Ehdr *e = ehdr; >> + >> + /* Check the magic numbers. */ >> + if (e->e_ident[EI_DATA] != ELFDATA2LSB || e->e_machine != EM_RISCV) >> + return grub_error (GRUB_ERR_BAD_OS, >> + N_("invalid arch-dependent ELF magic")); >> + >> + return GRUB_ERR_NONE; >> +} >> + >> +#pragma GCC diagnostic ignored "-Wcast-align" > > Why? Could you add a comment here? Mostly because all other targets have that as well? I assume it's to silence warnings that the alignment could be mismatched now, because offsets to elf data pointers could potentially be odd. In practice, it never is though. If I remove it, I get the following compile errors: kern/riscv/dl.c: In function ‘grub_arch_dl_relocate_symbols’: kern/riscv/dl.c:59:14: error: cast increases required alignment of target type [-Werror=cast-align] for (rel = (Elf_Rel *) ((char *) ehdr + s->sh_offset), ^ kern/riscv/dl.c:60:9: error: cast increases required alignment of target type [-Werror=cast-align] max = (Elf_Rel *) ((char *) rel + s->sh_size); ^ kern/riscv/dl.c:62:14: error: cast increases required alignment of target type [-Werror=cast-align] rel = (Elf_Rel *) ((char *) rel + s->sh_entsize)) ^ kern/riscv/dl.c:72:13: error: cast increases required alignment of target type [-Werror=cast-align] sym = (Elf_Sym *) ((char *) mod->symtab ^ kern/riscv/dl.c:256:18: error: cast increases required alignment of target type [-Werror=cast-align] for (rel2 = (Elf_Rela *) ((char *) rel - s->sh_entsize); ^ kern/riscv/dl.c:258:14: error: cast increases required alignment of target type [-Werror=cast-align] rel2 = (Elf_Rela *) ((char *) rel2 - s->sh_entsize)) ^ kern/riscv/dl.c:275:14: error: cast increases required alignment of target type [-Werror=cast-align] sym2 = (Elf_Sym *) ((char *) mod->symtab ^ > >> +/* Relocate symbols. */ >> +grub_err_t >> +grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr, >> + Elf_Shdr *s, grub_dl_segment_t seg) >> +{ >> + Elf_Rel *rel, *max; >> + >> + for (rel = (Elf_Rel *) ((char *) ehdr + s->sh_offset), >> + max = (Elf_Rel *) ((char *) rel + s->sh_size); >> + rel < max; >> + rel = (Elf_Rel *) ((char *) rel + s->sh_entsize)) >> + { >> + Elf_Sym *sym; >> + void *place; >> + grub_size_t sym_addr; >> + >> + if (rel->r_offset >= seg->size) >> + return grub_error (GRUB_ERR_BAD_MODULE, >> + "reloc offset is out of the segment"); >> + >> + sym = (Elf_Sym *) ((char *) mod->symtab >> + + mod->symsize * ELF_R_SYM (rel->r_info)); >> + >> + sym_addr = sym->st_value; >> + if (s->sh_type == SHT_RELA) >> + sym_addr += ((Elf_Rela *) rel)->r_addend; >> + >> + place = (void *) ((grub_addr_t) seg->addr + rel->r_offset); >> + >> + switch (ELF_R_TYPE (rel->r_info)) >> + { >> + case R_RISCV_32: >> + { >> + grub_uint32_t *abs_place = place; >> + >> + grub_dprintf ("dl", " reloc_abs32 %p => 0x%016llx\n", >> + place, (unsigned long long) sym_addr); >> + >> + *abs_place = (grub_uint32_t) sym_addr; >> + } >> + break; >> + case R_RISCV_64: >> + { >> + grub_size_t *abs_place = place; >> + >> + grub_dprintf ("dl", " reloc_abs64 %p => 0x%016llx\n", >> + place, (unsigned long long) sym_addr); >> + >> + *abs_place = (grub_size_t) sym_addr; >> + } >> + break; >> + >> + case R_RISCV_ADD8: >> + { >> + grub_uint8_t *abs_place = place; >> + >> + *abs_place += (grub_uint8_t) sym_addr; >> + } >> + break; >> + case R_RISCV_ADD16: >> + { >> + grub_uint16_t *abs_place = place; >> + >> + *abs_place += (grub_uint16_t) sym_addr; >> + } >> + break; >> + case R_RISCV_ADD32: >> + { >> + grub_uint32_t *abs_place = place; >> + >> + *abs_place += (grub_uint32_t) sym_addr; >> + } >> + break; >> + case R_RISCV_ADD64: >> + { >> + grub_size_t *abs_place = place; >> + >> + *abs_place += (grub_size_t) sym_addr; >> + } >> + break; >> + >> + case R_RISCV_SUB8: >> + { >> + grub_uint8_t *abs_place = place; >> + >> + *abs_place -= (grub_uint8_t) sym_addr; >> + } >> + break; >> + case R_RISCV_SUB16: >> + { >> + grub_uint16_t *abs_place = place; >> + >> + *abs_place -= (grub_uint16_t) sym_addr; >> + } >> + break; >> + case R_RISCV_SUB32: >> + { >> + grub_uint32_t *abs_place = place; >> + >> + *abs_place -= (grub_uint32_t) sym_addr; >> + } >> + break; >> + case R_RISCV_SUB64: >> + { >> + grub_size_t *abs_place = place; >> + >> + *abs_place -= (grub_size_t) sym_addr; >> + } >> + break; >> + >> + case R_RISCV_BRANCH: >> + { >> + grub_uint32_t *abs_place = place; >> + grub_ssize_t off = sym_addr - (grub_addr_t) place; >> + grub_uint32_t imm12 = (off & 0x1000) << (31 - 12); >> + grub_uint32_t imm11 = (off & 0x800) >> (11 - 7); >> + grub_uint32_t imm10_5 = (off & 0x7e0) << (30 - 10); >> + grub_uint32_t imm4_1 = (off & 0x1e) << (11 - 4); >> + *abs_place = (*abs_place & 0x1fff07f) >> + | imm12 | imm11 | imm10_5 | imm4_1; > > Could not we use some constants instead of numbers here? > If this does not make sense than please name the source of this numbers. As Andreas already mentioned, these are just taken from the spec. I agree that it looks quite unreadable, but putting the numbers into constants would only make things worse I guess. I'll add the spec reference into the head of this file. > >> + } >> + break; >> + >> + case R_RISCV_JAL: >> + { >> + grub_uint32_t *abs_place = place; >> + grub_ssize_t off = sym_addr - (grub_addr_t) place; >> + grub_uint32_t imm20 = (off & 0x100000) << (31 - 20); >> + grub_uint32_t imm19_12 = (off & 0xff000); >> + grub_uint32_t imm11 = (off & 0x800) << (20 - 11); >> + grub_uint32_t imm10_1 = (off & 0x7fe) << (30 - 10); >> + *abs_place = (*abs_place & 0xfff) >> + | imm20 | imm19_12 | imm11 | imm10_1; > > Ditto. > >> + } >> + break; >> + >> + case R_RISCV_CALL: >> + { >> + grub_uint32_t *abs_place = place; >> + grub_ssize_t off = sym_addr - (grub_addr_t) place; >> + grub_uint32_t hi20, lo12; >> + >> + if (off != (grub_int32_t) off) >> + return grub_error (GRUB_ERR_BAD_MODULE, "relocation overflow"); >> + >> + hi20 = (off + 0x800) & 0xfffff000; >> + lo12 = (off - hi20) & 0xfff; >> + abs_place[0] = (abs_place[0] & 0xfff) | hi20; >> + abs_place[1] = (abs_place[1] & 0xfffff) | (lo12 << 20); > > Ditto and below... > >> + } >> + break; >> + >> + case R_RISCV_RVC_BRANCH: >> + { >> + grub_uint16_t *abs_place = place; >> + grub_ssize_t off = sym_addr - (grub_addr_t) place; >> + grub_uint16_t imm8 = (off & 0x100) << (12 - 8); >> + grub_uint16_t imm7_6 = (off & 0xc0) >> (6 - 5); >> + grub_uint16_t imm5 = (off & 0x20) >> (5 - 2); >> + grub_uint16_t imm4_3 = (off & 0x18) << (12 - 5); >> + grub_uint16_t imm2_1 = (off & 0x6) << (12 - 10); >> + *abs_place = (*abs_place & 0xe383) >> + | imm8 | imm7_6 | imm5 | imm4_3 | imm2_1; >> + } >> + break; >> + >> + case R_RISCV_RVC_JUMP: >> + { >> + grub_uint16_t *abs_place = place; >> + grub_ssize_t off = sym_addr - (grub_addr_t) place; >> + grub_uint16_t imm11 = (off & 0x800) << (12 - 11); >> + grub_uint16_t imm10 = (off & 0x400) >> (10 - 8); >> + grub_uint16_t imm9_8 = (off & 0x300) << (12 - 11); >> + grub_uint16_t imm7 = (off & 0x80) >> (7 - 6); >> + grub_uint16_t imm6 = (off & 0x40) << (12 - 11); >> + grub_uint16_t imm5 = (off & 0x20) >> (5 - 2); >> + grub_uint16_t imm4 = (off & 0x10) << (12 - 5); >> + grub_uint16_t imm3_1 = (off & 0xe) << (12 - 10); >> + *abs_place = ((*abs_place & 0xe003) >> + | imm11 | imm10 | imm9_8 | imm7 | imm6 >> + | imm5 | imm4 | imm3_1); >> + } >> + break; >> + >> + case R_RISCV_PCREL_HI20: >> + { >> + grub_uint32_t *abs_place = place; >> + grub_ssize_t off = sym_addr - (grub_addr_t) place; >> + grub_int32_t hi20; >> + >> + if (off != (grub_int32_t)off) >> + return grub_error (GRUB_ERR_BAD_MODULE, "relocation overflow"); >> + >> + hi20 = (off + 0x800) & 0xfffff000; >> + *abs_place = (*abs_place & 0xfff) | hi20; >> + } >> + break; >> + >> + case R_RISCV_PCREL_LO12_I: >> + case R_RISCV_PCREL_LO12_S: >> + { >> + grub_uint32_t *t32 = place; >> + Elf_Rela *rel2; >> + /* Search backwards for matching HI20 reloc. */ > > Why "backwards"? Because then the chance is higher that we hit a matching relocation quickly :). Alex _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
