| Message ID | 20221010002957.128276-3-bgray@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Out-of-line static calls for powerpc64 ELF V2 | expand |
On Mon, 2022-10-10 at 11:29 +1100, Benjamin Gray wrote: > > A function symbol may set a value in the st_other field to indicate > > the TOC should be treated as caller-saved. The linker should > > ensure> the > > current TOC is saved before calling it and restore the TOC> > > afterwards, > > much like external calls. As I suggested on the last revision, worth mentioning here that it's the '.localentry <NAME>, 1' directive we're talking about here. > > > > This is necessary for supporting V2 ABI static calls that do not > > preserve the TOC. > > > > Signed-off-by: Benjamin Gray <bgray@linux.ibm.com> > > --- > > arch/powerpc/kernel/module_64.c | 14 +++++++++++++- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/arch/powerpc/kernel/module_64.c> > > b/arch/powerpc/kernel/module_64.c > > index 7e45dc98df8a..83a6f6e22e3b 100644 > > --- a/arch/powerpc/kernel/module_64.c > > +++ b/arch/powerpc/kernel/module_64.c > > @@ -55,6 +55,12 @@ static unsigned int local_entry_offset(const> > > Elf64_Sym *sym) > > * of function and try to derive r2 from it). */ > > return PPC64_LOCAL_ENTRY_OFFSET(sym->st_other); > > } > > + > > +static bool need_r2save_stub(unsigned char st_other) > > +{ > > + return (st_other & STO_PPC64_LOCAL_MASK) == (1 <<> > > STO_PPC64_LOCAL_BIT); > > +} > > + > > #else > > > > static func_desc_t func_desc(unsigned long addr) > > @@ -66,6 +72,11 @@ static unsigned int local_entry_offset(const> > > Elf64_Sym *sym) > > return 0; > > } > > > > +static bool need_r2save_stub(unsigned char st_other) > > +{ > > + return false; > > +} > > + > > void *dereference_module_function_descriptor(struct module *mod,> > > void *ptr) > > { > > if (ptr < (void *)mod->arch.start_opd || > > @@ -632,7 +643,8 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, > > case R_PPC_REL24: > > /* FIXME: Handle weak symbols here --RR */ > > if (sym->st_shndx == SHN_UNDEF || > > - sym->st_shndx == SHN_LIVEPATCH) { > > + sym->st_shndx == SHN_LIVEPATCH || > > + need_r2save_stub(sym->st_other)) { > > /* External: go via stub */ Perhaps this comment should be updated to mention that there are non- external but external-like calls? Otherwise Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com> > > value = stub_for_addr(sechdrs, > > value,> me, > > strtab +> sym- > > >st_name);
On Tue, 2022-10-25 at 13:10 +1100, Andrew Donnellan wrote: > On Mon, 2022-10-10 at 11:29 +1100, Benjamin Gray wrote: > > > A function symbol may set a value in the st_other field to > > > indicate > > > the TOC should be treated as caller-saved. The linker should > > > ensure> the > > > current TOC is saved before calling it and restore the TOC> > > > afterwards, > > > much like external calls. > > As I suggested on the last revision, worth mentioning here that it's > the '.localentry <NAME>, 1' directive we're talking about here. Ah right, whoops. Added "For example, GCC and Clang support a '.localentry <NAME>, 1' directive to set this explicitly in assembly." The exact method for doing this seems to be nonstandard (GCC supports arbitrary value in 1--7, Clang special cases only 1), so originally I was avoiding specifying how it is set. > > > @@ -632,7 +643,8 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, > > > case R_PPC_REL24: > > > /* FIXME: Handle weak symbols here --RR > > > */ > > > if (sym->st_shndx == SHN_UNDEF || > > > - sym->st_shndx == SHN_LIVEPATCH) { > > > + sym->st_shndx == SHN_LIVEPATCH || > > > + need_r2save_stub(sym->st_other)) { > > > /* External: go via stub */ > > Perhaps this comment should be updated to mention that there are non- > external but external-like calls? > > Otherwise > > Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com> Updated to "/* May use different / not preserve TOC: go via stub */", will add your reviewed-by in the next version. For now I'll wait for any other feedback before sending it. > >
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c index 7e45dc98df8a..83a6f6e22e3b 100644 --- a/arch/powerpc/kernel/module_64.c +++ b/arch/powerpc/kernel/module_64.c @@ -55,6 +55,12 @@ static unsigned int local_entry_offset(const Elf64_Sym *sym) * of function and try to derive r2 from it). */ return PPC64_LOCAL_ENTRY_OFFSET(sym->st_other); } + +static bool need_r2save_stub(unsigned char st_other) +{ + return (st_other & STO_PPC64_LOCAL_MASK) == (1 << STO_PPC64_LOCAL_BIT); +} + #else static func_desc_t func_desc(unsigned long addr) @@ -66,6 +72,11 @@ static unsigned int local_entry_offset(const Elf64_Sym *sym) return 0; } +static bool need_r2save_stub(unsigned char st_other) +{ + return false; +} + void *dereference_module_function_descriptor(struct module *mod, void *ptr) { if (ptr < (void *)mod->arch.start_opd || @@ -632,7 +643,8 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, case R_PPC_REL24: /* FIXME: Handle weak symbols here --RR */ if (sym->st_shndx == SHN_UNDEF || - sym->st_shndx == SHN_LIVEPATCH) { + sym->st_shndx == SHN_LIVEPATCH || + need_r2save_stub(sym->st_other)) { /* External: go via stub */ value = stub_for_addr(sechdrs, value, me, strtab + sym->st_name);
A function symbol may set a value in the st_other field to indicate the TOC should be treated as caller-saved. The linker should ensure the current TOC is saved before calling it and restore the TOC afterwards, much like external calls. This is necessary for supporting V2 ABI static calls that do not preserve the TOC. Signed-off-by: Benjamin Gray <bgray@linux.ibm.com> --- arch/powerpc/kernel/module_64.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)