Re: [PATCH v3 3/6] powerpc/module: Optimise nearby branches in ELF V2 ABI stub

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

On Wed, 2022-10-05 at 16:32 +1100, Benjamin Gray wrote:
> Inserts a direct branch to the stub target when possible, replacing
> the
> mtctr/btctr sequence.
> 
> The load into r12 could potentially be skipped too, but that change
> would need to refactor the arguments to indicate that the address
> does not have a separate local entry point.
> 
> This helps the static call implementation, where modules calling
> their
> own trampolines are called through this stub and the trampoline is
> easily within range of a direct branch.
> 
> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>

I'm not well versed in this code but nothing stands out as problematic
and it makes sense.

Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>

> ---
>  arch/powerpc/kernel/module_64.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/module_64.c
> b/arch/powerpc/kernel/module_64.c
> index 4d816f7785b4..13ce7a4d8a8d 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -141,6 +141,12 @@ static u32 ppc64_stub_insns[] = {
>         PPC_RAW_BCTR(),
>  };
>  
> +#ifdef CONFIG_PPC64_ELF_ABI_V1
> +#define PPC64_STUB_MTCTR_OFFSET 5
> +#else
> +#define PPC64_STUB_MTCTR_OFFSET 4
> +#endif
> +
>  /* Count how many different 24-bit relocations (different symbol,
>     different addend) */
>  static unsigned int count_relocs(const Elf64_Rela *rela, unsigned
> int num)
> @@ -426,6 +432,7 @@ static inline int create_stub(const Elf64_Shdr
> *sechdrs,
>                               struct module *me,
>                               const char *name)
>  {
> +       int err;
>         long reladdr;
>         func_desc_t desc;
>         int i;
> @@ -439,6 +446,11 @@ static inline int create_stub(const Elf64_Shdr
> *sechdrs,
>                         return 0;
>         }
>  
> +       /* Replace indirect branch sequence with direct branch where
> possible */
> +       err = patch_branch(&entry->jump[PPC64_STUB_MTCTR_OFFSET],
> addr, 0);
> +       if (err && err != -ERANGE)
> +               return 0;
> +
>         /* Stub uses address relative to r2. */
>         reladdr = (unsigned long)entry - my_r2(sechdrs, me);
>         if (reladdr > 0x7FFFFFFF || reladdr < -(0x80000000L)) {

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