On Tue, Jun 06, 2023 at 10:34:06AM +0800, Xiaotian Wu wrote: > Add R_LARCH_B16, R_LARCH_B21 and R_LARCH_RELAX relocation types to support > LoongArch relaxation.
Please explain why this relaxation support is needed. I think you can copy explanation from the cover letter. > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=56576f4a722b7398d35802ecf7d4185c27d6d69b > https://github.com/loongson/la-abi-specs/blob/release/laelf.adoc#relocations > > Signed-off-by: Xiaotian Wu <wuxiaot...@loongson.cn> > --- > grub-core/kern/loongarch64/dl.c | 21 +++++++- > grub-core/kern/loongarch64/dl_helper.c | 72 ++++++++++++++++++++++++-- > include/grub/elf.h | 3 ++ > include/grub/loongarch64/reloc.h | 6 ++- > util/grub-mkimagexx.c | 28 ++++++++-- > util/grub-module-verifier.c | 3 ++ > 6 files changed, 122 insertions(+), 11 deletions(-) > > diff --git a/grub-core/kern/loongarch64/dl.c b/grub-core/kern/loongarch64/dl.c > index 43080e72e..c22d2bd52 100644 > --- a/grub-core/kern/loongarch64/dl.c > +++ b/grub-core/kern/loongarch64/dl.c > @@ -87,11 +87,28 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr, > } > break; > case R_LARCH_MARK_LA: > + case R_LARCH_RELAX: > break; > case R_LARCH_SOP_PUSH_PCREL: > case R_LARCH_SOP_PUSH_PLT_PCREL: > grub_loongarch64_sop_push (&stack, sym_addr - (grub_uint64_t)place); > break; > + case R_LARCH_B16: > + { > + grub_uint32_t *abs_place = place; > + grub_ssize_t off = sym_addr - (grub_addr_t) place; > + > + grub_loongarch64_b16 (abs_place, off); > + } > + break; > + case R_LARCH_B21: > + { > + grub_uint32_t *abs_place = place; > + grub_ssize_t off = sym_addr - (grub_addr_t) place; > + > + grub_loongarch64_b21 (abs_place, off); > + } > + break; > case R_LARCH_B26: > { > grub_uint32_t *abs_place = place; > @@ -109,13 +126,13 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void > *ehdr, > case R_LARCH_ABS64_LO20: > { > grub_uint32_t *abs_place = place; > - grub_loongarch64_xxx64_lo20 (abs_place, sym_addr); > + grub_loongarch64_abs64_lo20 (abs_place, sym_addr); This change... > } > break; > case R_LARCH_ABS64_HI12: > { > grub_uint32_t *abs_place = place; > - grub_loongarch64_xxx64_hi12 (abs_place, sym_addr); > + grub_loongarch64_abs64_hi12 (abs_place, sym_addr); ... and this one does not seem to belong to this patch. Please move these changes to separate patch. > } > break; > case R_LARCH_PCALA_HI20: > diff --git a/grub-core/kern/loongarch64/dl_helper.c > b/grub-core/kern/loongarch64/dl_helper.c > index cda1a53c8..2809eeb7f 100644 > --- a/grub-core/kern/loongarch64/dl_helper.c > +++ b/grub-core/kern/loongarch64/dl_helper.c > @@ -24,6 +24,10 @@ > #include <grub/i18n.h> > #include <grub/loongarch64/reloc.h> > > +/* > + * LoongArch relocations documentation: > + * > https://github.com/loongson/la-abi-specs/blob/release/laelf.adoc#relocations > + */ Probably not a part of this patch too... > static void grub_loongarch64_stack_push (grub_loongarch64_stack_t stack, > grub_uint64_t x); > static grub_uint64_t grub_loongarch64_stack_pop (grub_loongarch64_stack_t > stack); > > @@ -200,14 +204,58 @@ grub_loongarch64_sop_32_s_0_10_10_16_s2 > (grub_loongarch64_stack_t stack, > *place =(*place) | ((a >> 18) & 0x3ff); > } > > +/* > + * B16 relocation for the 18-bit PC-relative jump > + * (*(uint32_t *) PC) [25 ... 10] = (S+A-PC) [17 ... 2] > + */ > +void grub_loongarch64_b16 (grub_uint32_t *place, grub_int64_t offset) > +{ > + grub_uint32_t val; > + const grub_uint32_t insmask = grub_cpu_to_le32_compile_time (0xfc0003ff); > + > + grub_dprintf ("dl", " reloc_b16 %p %c= 0x%"PRIxGRUB_INT64_T"\n", s/"PRIxGRUB_INT64_T"/" PRIxGRUB_INT64_T "/ Simply add spaces before and after PRIxGRUB_INT64_T... > + place, offset > 0 ? '+' : '-', > + offset < 0 ? (grub_int64_t) -(grub_uint64_t) offset : offset); These two casts does not make a lot of sense for me. You remove minus sign from the offset. So, I do not expect an overflow here. Then you can safely drop casts. Right? > + val = ((offset >> 2) & 0xffff) << 10; > + > + *place &= insmask; > + *place |= grub_cpu_to_le32 (val) & ~insmask; > +} > + > +/* > + * B21 relocation for the 23-bit PC-relative jump > + * (*(uint32_t *) PC) [4 ... 0] = (S+A-PC) [22 ... 18] > + * (*(uint32_t *) PC) [25 ... 10] = (S+A-PC) [17 ... 2] > + */ > +void grub_loongarch64_b21 (grub_uint32_t *place, grub_int64_t offset) > +{ > + grub_uint32_t val; > + const grub_uint32_t insmask = grub_cpu_to_le32_compile_time (0xfc0003e0); > + > + grub_dprintf ("dl", " reloc_b21 %p %c= 0x%"PRIxGRUB_INT64_T"\n", > + place, offset > 0 ? '+' : '-', > + offset < 0 ? (grub_int64_t) -(grub_uint64_t) offset : offset); Ditto and below the same things... > + > + val = ((offset >> 18) & 0x1f) | (((offset >> 2) & 0xffff) << 10); > + > + *place &= insmask; > + *place |= grub_cpu_to_le32 (val) & ~insmask; > +} > + > +/* > + * B26 relocation for the 28-bit PC-relative jump > + * (*(uint32_t *) PC) [9 ... 0] = (S+A-PC) [27 ... 18] > + * (*(uint32_t *) PC) [25 ... 10] = (S+A-PC) [17 ... 2] > + */ This... > void grub_loongarch64_b26 (grub_uint32_t *place, grub_int64_t offset) > { > grub_uint32_t val; > const grub_uint32_t insmask = grub_cpu_to_le32_compile_time (0xfc000000); > > - grub_dprintf ("dl", " reloc_xxxx64 %p %c= 0x%llx\n", > + grub_dprintf ("dl", " reloc_b26 %p %c= 0x%"PRIxGRUB_INT64_T"\n", ... and... > place, offset > 0 ? '+' : '-', > - offset < 0 ? (long long) -(unsigned long long) offset : offset); > + offset < 0 ? (grub_int64_t) -(grub_uint64_t) offset : offset); ... and this should go to separate patch... > val = ((offset >> 18) & 0x3ff) | (((offset >> 2) & 0xffff) << 10); > > @@ -215,6 +263,10 @@ void grub_loongarch64_b26 (grub_uint32_t *place, > grub_int64_t offset) > *place |= grub_cpu_to_le32 (val) & ~insmask; > } > > +/* > + * ABS_HI20/PCALA_HI20 relocations for 32/64-bit absolute > address/PC-relative offset > + * (*(uint32_t *) PC) [24 ... 5] = (S+A) [31 ... 12] > + */ Ditto. > void grub_loongarch64_xxx_hi20 (grub_uint32_t *place, grub_int64_t offset) > { > const grub_uint32_t insmask = grub_cpu_to_le32_compile_time (0xfe00001f); > @@ -227,6 +279,10 @@ void grub_loongarch64_xxx_hi20 (grub_uint32_t *place, > grub_int64_t offset) > *place |= grub_cpu_to_le32 (val) & ~insmask; > } > > +/* > + * ABS_LO12/PCALA_LO12 relocations for 32/64-bit absolute address > + * (*(uint32_t *) PC) [21 ... 10] = (S+A) [11 ... 0] > + */ Ditto. > void grub_loongarch64_xxx_lo12 (grub_uint32_t *place, grub_int64_t offset) > { > const grub_uint32_t insmask = grub_cpu_to_le32_compile_time (0xffc003ff); > @@ -235,7 +291,11 @@ void grub_loongarch64_xxx_lo12 (grub_uint32_t *place, > grub_int64_t offset) > *place |= grub_cpu_to_le32 (offset << 10) & ~insmask; > } > > -void grub_loongarch64_xxx64_hi12 (grub_uint32_t *place, grub_int64_t offset) > +/* > + * ABS64_HI12 relocation for the 64-bit absolute address > + * (*(uint32_t *) PC) [21 ... 10] = (S+A) [63 ... 52] > + */ Ditto... In general please split this patch into separate logical parts. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
