Re: [PATCH v4 2/6] powerpc/module: Handle caller-saved TOC in module linker

[Andrew Donnellan <ajd@linux.ibm.com>]

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);

-- 
Andrew Donnellan    OzLabs, ADL Canberra
ajd@linux.ibm.com   IBM Australia Limited