[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 07/10] RISC-V: Add awareness for RISC-V reloations
|
From: |
Daniel Kiper |
|
Subject: |
Re: [PATCH v3 07/10] RISC-V: Add awareness for RISC-V reloations |
|
Date: |
Wed, 21 Nov 2018 16:51:57 +0100 |
|
User-agent: |
NeoMutt/20170113 (1.7.2) |
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 <address@hidden>
>
> ---
>
> 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__?
> 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?
> +/* 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.
> + }
> + 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"?
> + for (rel2 = (Elf_Rela *) ((char *) rel - s->sh_entsize);
> + (unsigned long)rel2 >= ((unsigned long)ehdr + s->sh_offset);
> + rel2 = (Elf_Rela *) ((char *) rel2 - s->sh_entsize))
> + {
> + Elf_Addr rel2_info;
> + Elf_Addr rel2_offset;
> + Elf_Addr rel2_sym_addr;
> + Elf_Addr rel2_loc;
> + grub_ssize_t rel2_off;
> + grub_ssize_t off;
> + Elf_Sym *sym2;
> +
> + rel2_offset = rel2->r_offset;
> + rel2_info = rel2->r_info;
> + rel2_loc = (grub_addr_t) seg->addr + rel2_offset;
> +
> + if (ELF_R_TYPE (rel2_info) == R_RISCV_PCREL_HI20
> + && rel2_loc == sym_addr)
> + {
> + sym2 = (Elf_Sym *) ((char *) mod->symtab
> + + mod->symsize * ELF_R_SYM (rel2->r_info));
> + rel2_sym_addr = sym2->st_value;
> + if (s->sh_type == SHT_RELA)
> + rel2_sym_addr += ((Elf_Rela *) rel2)->r_addend;
> +
> + rel2_off = rel2_sym_addr - rel2_loc;
> + off = rel2_off - ((rel2_off + 0x800) & 0xfffff000);
> +
> + if (ELF_R_TYPE (rel->r_info) == R_RISCV_PCREL_LO12_I)
> + *t32 = (*t32 & 0xfffff) | (off & 0xfff) << 20;
> + else
> + {
> + grub_uint32_t imm11_5 = (off & 0xfe0) << (31 - 11);
> + grub_uint32_t imm4_0 = (off & 0x1f) << (11 - 4);
> + *t32 = (*t32 & 0x1fff07f) | imm11_5 | imm4_0;
> + }
> + break;
> + }
> + }
> + if ((unsigned long)rel2 < ((unsigned long)ehdr + s->sh_offset))
> + return grub_error (GRUB_ERR_BAD_MODULE, "cannot find matching
> HI20 relocation");
> + }
> + break;
> +
> + case R_RISCV_HI20:
> + {
> + grub_uint32_t *abs_place = place;
> + *abs_place = (*abs_place & 0xfff) |
> + (((grub_int32_t) sym_addr + 0x800) & 0xfffff000);
> + }
> + break;
> +
> + case R_RISCV_LO12_I:
> + {
> + grub_uint32_t *abs_place = place;
> + grub_int32_t lo12 = (grub_int32_t) sym_addr -
> + (((grub_int32_t) sym_addr + 0x800) &
> 0xfffff000);
> + *abs_place = (*abs_place & 0xfffff) | ((lo12 & 0xfff) << 20);
> + }
> + break;
> +
> + case R_RISCV_LO12_S:
> + {
> + grub_uint32_t *abs_place = place;
> + grub_int32_t lo12 = (grub_int32_t) sym_addr -
> + (((grub_int32_t) sym_addr + 0x800) &
> 0xfffff000);
> + grub_uint32_t imm11_5 = (lo12 & 0xfe0) << (31 - 11);
> + grub_uint32_t imm4_0 = (lo12 & 0x1f) << (11 - 4);
> + *abs_place = (*abs_place & 0x1fff07f) | imm11_5 | imm4_0;
Again, why not constants? And below...
Daniel
[PATCH v3 01/10] efi: Rename armxx to arch, Alexander Graf, 2018/11/14
[PATCH v3 03/10] elf.h: Add RISC-V definitions, Alexander Graf, 2018/11/14
[PATCH v3 07/10] RISC-V: Add awareness for RISC-V reloations, Alexander Graf, 2018/11/14
[PATCH v3 06/10] RISC-V: Add Linux load logic, Alexander Graf, 2018/11/14
[PATCH v3 09/10] RISC-V: Add to build system, Alexander Graf, 2018/11/14