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

[Benjamin Gray <bgray@linux.ibm.com>]

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.
> >