diff mbox series

[v3,2/6] powerpc/module: Handle caller-saved TOC in module linker

Message ID 20221005053234.29312-3-bgray@linux.ibm.com (mailing list archive)
State Superseded
Headers show
Series Out-of-line static calls for powerpc64 ELF V2 | expand

Commit Message

Benjamin Gray Oct. 5, 2022, 5:32 a.m. UTC
The callee may set a field in st_other to 1 to indicate r2 should be
treated as caller-saved. This means a trampoline must be used to save
the current TOC before calling it and restore it 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(-)

Comments

Christophe Leroy Oct. 5, 2022, 7:18 p.m. UTC | #1
Le 05/10/2022 à 07:32, Benjamin Gray a écrit :
> The callee may set a field in st_other to 1 to indicate r2 should be
> treated as caller-saved. This means a trampoline must be used to save
> the current TOC before calling it and restore it afterwards, much like
> external calls.

The 'callee', what is that here in that context ?

Don't you mean the 'linker' instead ?

> 
> 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..4d816f7785b4 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) >> STO_PPC64_LOCAL_BIT) == 1;

I would have writen :

	return st_other & STO_PPC64_LOCAL_MASK == 1 << STO_PPC64_LOCAL_BIT;

But it is just a matter of preference, so up to you.

> +}
> +
>   #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);
Andrew Donnellan Oct. 6, 2022, 3:51 a.m. UTC | #2
On Wed, 2022-10-05 at 19:18 +0000, Christophe Leroy wrote:
> 
> 
> Le 05/10/2022 à 07:32, Benjamin Gray a écrit :
> > The callee may set a field in st_other to 1 to indicate r2 should
> > be
> > treated as caller-saved. This means a trampoline must be used to
> > save
> > the current TOC before calling it and restore it afterwards, much
> > like
> > external calls.
> 
> The 'callee', what is that here in that context ?
> 
> Don't you mean the 'linker' instead ?

Should probably explain explicitly that this is the .localentry NAME, 1
directive.
Benjamin Gray Oct. 6, 2022, 4:39 a.m. UTC | #3
On Wed, 2022-10-05 at 19:18 +0000, Christophe Leroy wrote:
> 
> 
> Le 05/10/2022 à 07:32, Benjamin Gray a écrit :
> > The callee may set a field in st_other to 1 to indicate r2 should
> > be
> > treated as caller-saved. This means a trampoline must be used to
> > save
> > the current TOC before calling it and restore it afterwards, much
> > like
> > external calls.
> 
> The 'callee', what is that here in that context ?
> 
> Don't you mean the 'linker' instead ?

Callee in the sense of the function body, as opposed to the call-site
like the R_PPC64_REL24, etc., relocations are (which I was thinking of
as the caller). I can see how the comment needs more context though.
diff mbox series

Patch

diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 7e45dc98df8a..4d816f7785b4 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) >> STO_PPC64_LOCAL_BIT) == 1;
+}
+
 #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);