Messages in this thread | | | Subject | Re: [PATCH v1 4/6] objtool/LoongArch: Enable orc to be built | From | Tiezhu Yang <> | Date | Fri, 28 Jul 2023 09:31:26 +0800 |
| |
On 07/25/2023 07:56 PM, Peter Zijlstra wrote: > On Tue, Jul 25, 2023 at 04:15:08PM +0800, Tiezhu Yang wrote: > >> +void arch_write_orc(struct elf *elf, struct orc_entry *orc) >> +{ >> + orc->ra_offset = bswap_if_needed(elf, orc->ra_offset); >> +} > > This hardly 'writes' anything, all it does is a bswap. > >> diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c >> index 1eff7e0a..6975056 100644 >> --- a/tools/objtool/orc_gen.c >> +++ b/tools/objtool/orc_gen.c >> @@ -14,6 +14,8 @@ >> #include <objtool/warn.h> >> #include <objtool/endianness.h> >> >> +void __weak arch_write_orc(struct elf *elf, struct orc_entry *orc) {} > > And for that you need a weak function? What's wrong with an inline if > anything? > >> static int write_orc_entry(struct elf *elf, struct section *orc_sec, >> struct section *ip_sec, unsigned int idx, >> struct section *insn_sec, unsigned long insn_off, >> @@ -26,6 +28,7 @@ static int write_orc_entry(struct elf *elf, struct section *orc_sec, >> memcpy(orc, o, sizeof(*orc)); >> orc->sp_offset = bswap_if_needed(elf, orc->sp_offset); >> orc->bp_offset = bswap_if_needed(elf, orc->bp_offset); >> + arch_write_orc(elf, orc); > > Why can't this simply be: > > orc->ra_offset = bswap_if_needed(elf, orc->ra_offset); > > AFAICT this won't actually do anything for x86 and I don't think Power > got around to doing ORC yet. > >> >> /* populate reloc for ip */ >> if (!elf_init_reloc_text_sym(elf, ip_sec, idx * sizeof(int), idx, > > Again, not much explaination for why you did things >
The initial aim is to keep consistent with the above two lines, but for now, I think there is no need to call bswap_if_needed() for LoongArch, the related changes are not necessary, I will drop them too.
LoongArch is little-endian, the __BYTE_ORDER is defined as __LITTLE_ENDIAN, and elf->ehdr.e_ident[EI_DATA] is 1 which is equal with ELFDATA2LSB, so need_bswap() returns false.
static inline bool need_bswap(struct elf *elf) { return (__BYTE_ORDER == __LITTLE_ENDIAN) ^ (elf->ehdr.e_ident[EI_DATA] == ELFDATA2LSB); }
Thank you very much for your review, sorry for that.
Thanks, Tiezhu
| |