diff mbox series

[RFC,3/3] objtool/mcount: Add powerpc specific functions

Message ID 20220318105140.43914-4-sv@linux.ibm.com (mailing list archive)
State RFC
Headers show
Series objtool: Add mcount sub-command | expand

Checks

Context Check Description
snowpatch_ozlabs/github-powerpc_kernel_qemu fail kernel (ppc64_defconfig, korg-5.5.0) failed at step build.
snowpatch_ozlabs/github-powerpc_ppctests success Successfully ran 8 jobs.
snowpatch_ozlabs/github-powerpc_selftests success Successfully ran 8 jobs.
snowpatch_ozlabs/github-powerpc_clang success Successfully ran 7 jobs.
snowpatch_ozlabs/github-powerpc_sparse success Successfully ran 4 jobs.

Commit Message

Sathvika Vasireddy March 18, 2022, 10:51 a.m. UTC
This patch adds powerpc specific functions required for
'objtool mcount' to work, and enables mcount for ppc.

Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
---
 arch/powerpc/Kconfig                          |  1 +
 tools/objtool/Makefile                        |  4 ++
 tools/objtool/arch/powerpc/Build              |  1 +
 tools/objtool/arch/powerpc/decode.c           | 51 +++++++++++++++++++
 .../arch/powerpc/include/arch/cfi_regs.h      | 37 ++++++++++++++
 tools/objtool/arch/powerpc/include/arch/elf.h |  8 +++
 tools/objtool/utils.c                         | 28 ++++++++--
 7 files changed, 125 insertions(+), 5 deletions(-)
 create mode 100644 tools/objtool/arch/powerpc/Build
 create mode 100644 tools/objtool/arch/powerpc/decode.c
 create mode 100644 tools/objtool/arch/powerpc/include/arch/cfi_regs.h
 create mode 100644 tools/objtool/arch/powerpc/include/arch/elf.h

Comments

Michael Ellerman March 21, 2022, 2:27 a.m. UTC | #1
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>>> This patch adds powerpc specific functions required for
>>> 'objtool mcount' to work, and enables mcount for ppc.
>> 
>> I would love to see more objtool enablement for Power :-)
>
> I have not received this series and I can't see it on powerpc patchwork 
> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)
>
> Did you send it to linuxppc-dev list ? If not can you resend it there ?

It is there, might have been delayed?

http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129

There are some CI failures.

cheers
Naveen N. Rao March 21, 2022, 6:25 a.m. UTC | #2
Peter Zijlstra wrote:
> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>> This patch adds powerpc specific functions required for
>> 'objtool mcount' to work, and enables mcount for ppc.
> 
> I would love to see more objtool enablement for Power :-)
> 
> 
>> diff --git a/tools/objtool/arch/powerpc/include/arch/elf.h b/tools/objtool/arch/powerpc/include/arch/elf.h
>> new file mode 100644
>> index 000000000000..3c8ebb7d2a6b
>> --- /dev/null
>> +++ b/tools/objtool/arch/powerpc/include/arch/elf.h
>> @@ -0,0 +1,8 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +
>> +#ifndef _OBJTOOL_ARCH_ELF
>> +#define _OBJTOOL_ARCH_ELF
>> +
>> +#define R_NONE R_PPC_NONE
>> +
>> +#endif /* _OBJTOOL_ARCH_ELF */
>> diff --git a/tools/objtool/utils.c b/tools/objtool/utils.c
>> index d1fc6a123a6e..c9c14fa0dfd7 100644
>> --- a/tools/objtool/utils.c
>> +++ b/tools/objtool/utils.c
>> @@ -179,11 +179,29 @@ int create_mcount_loc_sections(struct objtool_file *file)
>>  		loc = (unsigned long *)sec->data->d_buf + idx;
>>  		memset(loc, 0, sizeof(unsigned long));
>>  
>> -		if (elf_add_reloc_to_insn(file->elf, sec,
>> -					  idx * sizeof(unsigned long),
>> -					  R_X86_64_64,
>> -					  insn->sec, insn->offset))
>> -			return -1;
>> +		if (file->elf->ehdr.e_machine == EM_X86_64) {
>> +			if (elf_add_reloc_to_insn(file->elf, sec,
>> +						  idx * sizeof(unsigned long),
>> +						  R_X86_64_64,
>> +						  insn->sec, insn->offset))
>> +				return -1;
>> +		}
>> +
>> +		if (file->elf->ehdr.e_machine == EM_PPC64) {
>> +			if (elf_add_reloc_to_insn(file->elf, sec,
>> +						  idx * sizeof(unsigned long),
>> +						  R_PPC64_ADDR64,
>> +						  insn->sec, insn->offset))
>> +				return -1;
>> +		}
>> +
>> +		if (file->elf->ehdr.e_machine == EM_PPC) {
>> +			if (elf_add_reloc_to_insn(file->elf, sec,
>> +						  idx * sizeof(unsigned long),
>> +						  R_PPC_ADDR32,
>> +						  insn->sec, insn->offset))
>> +				return -1;
>> +		}
> 
> It appears to me that repeating this code 3 times doesn't really scale
> well, how about we introduce a helper like:
> 
> 
> int elf_reloc_type_long(struct elf *elf)
> {
> 	switch (elf->ehdr.e_machine) {
> 	case EM_X86_64:
> 		return R_X86_64_64;
> 	case EM_PPC64:
> 		return R_PPC64_ADDR64;
> 	case EM_PPC:
> 		return R_PPC_ADDR32;
> 	default:
> 		WARN("unknown machine...")
> 		exit(-1);
> 	}
> }
> 
> 		if (elf_add_reloc_to_insn(file->elf, sec,
> 					  idx * sizeof(unsigned long),
> 					  elf_reloc_type_long(file->elf),
> 					  insn->sec, insn->offset))
> 			return -1;

Good point. I suppose we'll need a similar helper, say, 
elf_reloc_type_none(), to replace R_NONE usage if we want to have 
objtool work for cross-arch builds.


- Naveen
Christophe Leroy March 21, 2022, 6:47 a.m. UTC | #3
Le 21/03/2022 à 03:27, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
>>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>>>> This patch adds powerpc specific functions required for
>>>> 'objtool mcount' to work, and enables mcount for ppc.
>>>
>>> I would love to see more objtool enablement for Power :-)
>>
>> I have not received this series and I can't see it on powerpc patchwork
>> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)
>>
>> Did you send it to linuxppc-dev list ? If not can you resend it there ?
> 
> It is there, might have been delayed?

Yes something must have gone wrong.

Peter's and myself's responses are not at 
http://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220318105140.43914-4-sv@linux.ibm.com/ 
while yours and Naveen's ones are there.

Christophe
Christophe Leroy March 21, 2022, 7:46 a.m. UTC | #4
Le 21/03/2022 à 03:27, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
>>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>>>> This patch adds powerpc specific functions required for
>>>> 'objtool mcount' to work, and enables mcount for ppc.
>>>
>>> I would love to see more objtool enablement for Power :-)
>>
>> I have not received this series and I can't see it on powerpc patchwork
>> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)
>>
>> Did you send it to linuxppc-dev list ? If not can you resend it there ?
> 
> It is there, might have been delayed?
> 
> http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129
> 
> There are some CI failures.
> 

On PPC64 we randomly get:

/bin/sh: 1: ./tools/objtool/objtool: not found
make[2]: *** [arch/powerpc/kernel/vdso/vgettimeofday-64.o] Error 127
/linux/arch/powerpc/kernel/vdso/Makefile:77: recipe for target 
'arch/powerpc/kernel/vdso/vgettimeofday-64.o' failed
make[2]: *** Deleting file 'arch/powerpc/kernel/vdso/vgettimeofday-64.o'
make[1]: *** [vdso_prepare] Error 2
make[1]: *** Waiting for unfinished jobs....
/linux/arch/powerpc/Makefile:423: recipe for target 'vdso_prepare' failed
make: *** [__sub-make] Error 2
Makefile:219: recipe for target '__sub-make' failed

This is linkely because prepare: target depends on vdso_prepare and 
prepare: target depends on objtool, but vdso_prepare: doesn't depend on 
objtool.

I'm not sure it is correct to run objtool mcount on VDSO as it is kind 
of useless. Only VDSO64 seems to call objtool, not VDSO32.

Christophe
Christophe Leroy March 21, 2022, 7:56 a.m. UTC | #5
Le 21/03/2022 à 03:27, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
>>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>>>> This patch adds powerpc specific functions required for
>>>> 'objtool mcount' to work, and enables mcount for ppc.
>>>
>>> I would love to see more objtool enablement for Power :-)
>>
>> I have not received this series and I can't see it on powerpc patchwork
>> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)
>>
>> Did you send it to linuxppc-dev list ? If not can you resend it there ?
> 
> It is there, might have been delayed?
> 
> http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129
> 
> There are some CI failures.
> 

On PPC32 I get :

[    0.000000] ftrace: No functions to be traced?

Without this series I get:

[    0.000000] ftrace: allocating 22508 entries in 17 pages
[    0.000000] ftrace: allocated 17 pages with 2 groups


Christophe
Christophe Leroy March 21, 2022, 8:30 a.m. UTC | #6
Le 21/03/2022 à 08:56, Christophe Leroy a écrit :
> 
> 
> Le 21/03/2022 à 03:27, Michael Ellerman a écrit :
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
>>>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>>>>> This patch adds powerpc specific functions required for
>>>>> 'objtool mcount' to work, and enables mcount for ppc.
>>>>
>>>> I would love to see more objtool enablement for Power :-)
>>>
>>> I have not received this series and I can't see it on powerpc patchwork
>>> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)
>>>
>>> Did you send it to linuxppc-dev list ? If not can you resend it there ?
>>
>> It is there, might have been delayed?
>>
>> http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129
>>
>> There are some CI failures.
>>
> 
> On PPC32 I get :
> 
> [    0.000000] ftrace: No functions to be traced?
> 
> Without this series I get:
> 
> [    0.000000] ftrace: allocating 22508 entries in 17 pages
> [    0.000000] ftrace: allocated 17 pages with 2 groups
> 
> 


ppc64_defconfig on QEMU:

[    0.000000][    T0] ftrace: No functions to be traced?

Without your series:

[    0.000000][    T0] ftrace: allocating 38750 entries in 15 pages
[    0.000000][    T0] ftrace: allocated 15 pages with 4 groups
[    0.000000][    T0] trace event string verifier disabled
Christophe Leroy March 21, 2022, 8:59 a.m. UTC | #7
Le 21/03/2022 à 09:30, Christophe Leroy a écrit :
> 
> 
> Le 21/03/2022 à 08:56, Christophe Leroy a écrit :
>>
>>
>> Le 21/03/2022 à 03:27, Michael Ellerman a écrit :
>>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>>> Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
>>>>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>>>>>> This patch adds powerpc specific functions required for
>>>>>> 'objtool mcount' to work, and enables mcount for ppc.
>>>>>
>>>>> I would love to see more objtool enablement for Power :-)
>>>>
>>>> I have not received this series and I can't see it on powerpc patchwork
>>>> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)
>>>>
>>>> Did you send it to linuxppc-dev list ? If not can you resend it there ?
>>>
>>> It is there, might have been delayed?
>>>
>>> http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129
>>>
>>> There are some CI failures.
>>>
>>
>> On PPC32 I get :
>>
>> [    0.000000] ftrace: No functions to be traced?
>>
>> Without this series I get:
>>
>> [    0.000000] ftrace: allocating 22508 entries in 17 pages
>> [    0.000000] ftrace: allocated 17 pages with 2 groups
>>
>>
> 
> 
> ppc64_defconfig on QEMU:
> 
> [    0.000000][    T0] ftrace: No functions to be traced?
> 
> Without your series:
> 
> [    0.000000][    T0] ftrace: allocating 38750 entries in 15 pages
> [    0.000000][    T0] ftrace: allocated 15 pages with 4 groups
> [    0.000000][    T0] trace event string verifier disabled
> 

ppc64le_defconfig on QEMU:

[    0.000000][    T0] ftrace: allocating 37236 entries in 14 pages
[    0.000000][    T0] ftrace: allocated 14 pages with 3 groups

Without your series:

[    0.000000][    T0] ftrace: allocating 37236 entries in 14 pages
[    0.000000][    T0] ftrace: allocated 14 pages with 3 groups

So it seems it works only on Little Endian.
Works neither on PPC32 nor on PPC64 Big Endian

Christophe
Christophe Leroy March 26, 2022, 7:58 a.m. UTC | #8
Le 21/03/2022 a 08:56, Christophe Leroy a ecrit :
> 
> 
> Le 21/03/2022 a 03:27, Michael Ellerman a ecrit :
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>> Le 18/03/2022 a 13:26, Peter Zijlstra a ecrit :
>>>> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>>>>> This patch adds powerpc specific functions required for
>>>>> 'objtool mcount' to work, and enables mcount for ppc.
>>>>
>>>> I would love to see more objtool enablement for Power :-)
>>>
>>> I have not received this series and I can't see it on powerpc patchwork
>>> either (https://patchwork.ozlabs.org/project/linuxppc-dev/list/)
>>>
>>> Did you send it to linuxppc-dev list ? If not can you resend it there ?
>>
>> It is there, might have been delayed?
>>
>> http://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=291129
>>
>> There are some CI failures.
>>
> 
> On PPC32 I get :
> 
> [    0.000000] ftrace: No functions to be traced?
> 
> Without this series I get:
> 
> [    0.000000] ftrace: allocating 22508 entries in 17 pages
> [    0.000000] ftrace: allocated 17 pages with 2 groups
> 


With the changes below I managed to get a working ftrace on a PPC32 target.

Christophe

---------
From: Christophe Leroy <christophe.leroy@csgroup.eu>
Subject: [PATCH] powerpc/objtool: Set to big endian and 32 bits

Small ack to crossbuild a PPC32 kernel with a x86_64 host.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 tools/objtool/arch/powerpc/decode.c                  |  3 ++-
 tools/objtool/arch/powerpc/include/arch/endianness.h |  9 +++++++++
 tools/objtool/elf.c                                  |  4 ++--
 tools/objtool/utils.c                                | 12 +++++++-----
 4 files changed, 20 insertions(+), 8 deletions(-)
 create mode 100644 tools/objtool/arch/powerpc/include/arch/endianness.h

diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/powerpc/decode.c
index 58988f88b315..330af382e39f 100644
--- a/tools/objtool/arch/powerpc/decode.c
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -8,6 +8,7 @@
 #include <objtool/arch.h>
 #include <objtool/warn.h>
 #include <objtool/builtin.h>
+#include <objtool/endianness.h>
 
 int arch_decode_instruction(struct objtool_file *file, const struct section *sec,
 			    unsigned long offset, unsigned int maxlen,
@@ -20,7 +21,7 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
 	u64 imm;
 
 	*immediate = imm = 0;
-	memcpy(&insn, sec->data->d_buf+offset, 4);
+	insn = bswap_if_needed(*(u32 *)(sec->data->d_buf + offset));
 	*len = 4;
 	*type = INSN_OTHER;
 
diff --git a/tools/objtool/arch/powerpc/include/arch/endianness.h b/tools/objtool/arch/powerpc/include/arch/endianness.h
new file mode 100644
index 000000000000..275087bfcc16
--- /dev/null
+++ b/tools/objtool/arch/powerpc/include/arch/endianness.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _ARCH_ENDIANNESS_H
+#define _ARCH_ENDIANNESS_H
+
+#include <endian.h>
+
+#define __TARGET_BYTE_ORDER __BIG_ENDIAN
+
+#endif /* _ARCH_ENDIANNESS_H */
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 4b384c907027..433f0e327b68 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -867,7 +867,7 @@ static struct section *elf_create_rela_reloc_section(struct elf *elf, struct sec
 	strcpy(relocname, ".rela");
 	strcat(relocname, base->name);
 
-	sec = elf_create_section(elf, relocname, 0, sizeof(GElf_Rela), 0);
+	sec = elf_create_section(elf, relocname, 0, sizeof(Elf32_Rela), 0);
 	free(relocname);
 	if (!sec)
 		return NULL;
@@ -876,7 +876,7 @@ static struct section *elf_create_rela_reloc_section(struct elf *elf, struct sec
 	sec->base = base;
 
 	sec->sh.sh_type = SHT_RELA;
-	sec->sh.sh_addralign = 8;
+	sec->sh.sh_addralign = 4;
 	sec->sh.sh_link = find_section_by_name(elf, ".symtab")->idx;
 	sec->sh.sh_info = base->idx;
 	sec->sh.sh_flags = SHF_INFO_LINK;
diff --git a/tools/objtool/utils.c b/tools/objtool/utils.c
index c9c14fa0dfd7..f77695c81386 100644
--- a/tools/objtool/utils.c
+++ b/tools/objtool/utils.c
@@ -151,7 +151,7 @@ int decode_instructions(struct objtool_file *file)
 int create_mcount_loc_sections(struct objtool_file *file)
 {
 	struct section *sec;
-	unsigned long *loc;
+	unsigned int *loc;
 	struct instruction *insn;
 	int idx;
 
@@ -169,15 +169,17 @@ int create_mcount_loc_sections(struct objtool_file *file)
 	list_for_each_entry(insn, &file->mcount_loc_list, call_node)
 		idx++;
 
-	sec = elf_create_section(file->elf, "__mcount_loc", 0, sizeof(unsigned long), idx);
+	sec = elf_create_section(file->elf, "__mcount_loc", 0, sizeof(unsigned int), idx);
 	if (!sec)
 		return -1;
 
+	sec->sh.sh_addralign = 4;
+
 	idx = 0;
 	list_for_each_entry(insn, &file->mcount_loc_list, call_node) {
 
-		loc = (unsigned long *)sec->data->d_buf + idx;
-		memset(loc, 0, sizeof(unsigned long));
+		loc = (unsigned int *)sec->data->d_buf + idx;
+		memset(loc, 0, sizeof(unsigned int));
 
 		if (file->elf->ehdr.e_machine == EM_X86_64) {
 			if (elf_add_reloc_to_insn(file->elf, sec,
@@ -197,7 +199,7 @@ int create_mcount_loc_sections(struct objtool_file *file)
 
 		if (file->elf->ehdr.e_machine == EM_PPC) {
 			if (elf_add_reloc_to_insn(file->elf, sec,
-						  idx * sizeof(unsigned long),
+						  idx * sizeof(unsigned int),
 						  R_PPC_ADDR32,
 						  insn->sec, insn->offset))
 				return -1;
Christophe Leroy March 27, 2022, 9:09 a.m. UTC | #9
Hi Peter, Hi Josh

Le 18/03/2022 à 13:26, Peter Zijlstra a écrit :
> On Fri, Mar 18, 2022 at 04:21:40PM +0530, Sathvika Vasireddy wrote:
>> This patch adds powerpc specific functions required for
>> 'objtool mcount' to work, and enables mcount for ppc.
> 
> I would love to see more objtool enablement for Power :-)
> 
> 

I'm also very happy someone started to look at it.

I thought it would be more difficult to get it work on powerpc.

Iml looking forward to being able to use it and implement INLINE STATIC 
CALLs on PPC32 to start with.

I'm wondering what are the plans on your side and what we should wait 
for and what we could start with.

I could do the same as done by Sathvika for static calls, in extenso get 
it out of check.c into a standalone feature. On the other hand I 
understood that Josh is also working at making the different features of 
objtool independant, so should I wait for that ? Any idea of when it 
comes out ?

Second point is the endianess and 32/64 selection, especially when 
crossbuilding. There is already some stuff regarding endianess based on 
bswap_if_needed() but that's based on constant selection at build time 
and I couldn't find an easy way to set it conditionaly based on the 
target being built.

Regarding 32/64 selection, there is almost nothing, it's based on using 
type 'long' which means that at the time being the target and the build 
platform must both be 32 bits or 64 bits.

For both cases (endianess and 32/64) I think the solution should 
probably be to start with the fileformat of the object file being 
reworked by objtool.

What are current works in progress on objtool ? Should I wait Josh's 
changes before starting looking at all this ? Should I wait for anything 
else ?

Christophe
Josh Poimboeuf March 28, 2022, 7:59 p.m. UTC | #10
On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
> Second point is the endianess and 32/64 selection, especially when 
> crossbuilding. There is already some stuff regarding endianess based on 
> bswap_if_needed() but that's based on constant selection at build time 
> and I couldn't find an easy way to set it conditionaly based on the 
> target being built.
>
> Regarding 32/64 selection, there is almost nothing, it's based on using 
> type 'long' which means that at the time being the target and the build 
> platform must both be 32 bits or 64 bits.
> 
> For both cases (endianess and 32/64) I think the solution should 
> probably be to start with the fileformat of the object file being 
> reworked by objtool.

Do we really need to detect the endianness/bitness at runtime?  Objtool
is built with the kernel, why not just build-in the same target
assumptions as the kernel itself?

> What are current works in progress on objtool ? Should I wait Josh's 
> changes before starting looking at all this ? Should I wait for anything 
> else ?

I'm not making any major changes to the code, just shuffling things
around to make the interface more modular.  I hope to have something
soon (this week).  Peter recently added a big feature (Intel IBT) which
is already in -next.

Contributions are welcome, with the understanding that you'll help
maintain it ;-)

Some years ago Kamalesh Babulal had a prototype of objtool for ppc64le
which did the full stack validation.  I'm not sure what ever became of
that.

FWIW, there have been some objtool patches for arm64 stack validation,
but the arm64 maintainers have been hesitant to get on board with
objtool, as it brings a certain maintenance burden.  Especially for the
full stack validation and ORC unwinder.  But if you only want inline
static calls and/or mcount then it'd probably be much easier to
maintain.
Peter Zijlstra March 28, 2022, 8:14 p.m. UTC | #11
On Mon, Mar 28, 2022 at 12:59:20PM -0700, Josh Poimboeuf wrote:

> I'm not making any major changes to the code, just shuffling things
> around to make the interface more modular.  I hope to have something
> soon (this week).  Peter recently added a big feature (Intel IBT) which
> is already in -next.

Hit Linus' tree yesterday :-)

> Some years ago Kamalesh Babulal had a prototype of objtool for ppc64le
> which did the full stack validation.  I'm not sure what ever became of
> that.

I've also heard chatter about s390.

> FWIW, there have been some objtool patches for arm64 stack validation,
> but the arm64 maintainers have been hesitant to get on board with
> objtool, as it brings a certain maintenance burden.  Especially for the
> full stack validation and ORC unwinder.  But if you only want inline
> static calls and/or mcount then it'd probably be much easier to
> maintain.

IIRC the major stumbling block for arm64 is the whole jump-table thing.
Either they need to rely on compiler plugins to provide objtool that
data (yuck, since we support at least 2 different compilers), disable
jump-tables (yuck, for that limits code-gen just to please a tool) or
use DWARF (yuck, because build times).

There was a little talk about an impromptu 'abi' to communicate
jump-table details to objtool without going full on DWARF, but that
seems to have hit a dead end again.
Peter Zijlstra March 28, 2022, 8:15 p.m. UTC | #12
+arm64 people...

On Mon, Mar 28, 2022 at 10:14:38PM +0200, Peter Zijlstra wrote:
> On Mon, Mar 28, 2022 at 12:59:20PM -0700, Josh Poimboeuf wrote:
> 
> > I'm not making any major changes to the code, just shuffling things
> > around to make the interface more modular.  I hope to have something
> > soon (this week).  Peter recently added a big feature (Intel IBT) which
> > is already in -next.
> 
> Hit Linus' tree yesterday :-)
> 
> > Some years ago Kamalesh Babulal had a prototype of objtool for ppc64le
> > which did the full stack validation.  I'm not sure what ever became of
> > that.
> 
> I've also heard chatter about s390.
> 
> > FWIW, there have been some objtool patches for arm64 stack validation,
> > but the arm64 maintainers have been hesitant to get on board with
> > objtool, as it brings a certain maintenance burden.  Especially for the
> > full stack validation and ORC unwinder.  But if you only want inline
> > static calls and/or mcount then it'd probably be much easier to
> > maintain.
> 
> IIRC the major stumbling block for arm64 is the whole jump-table thing.
> Either they need to rely on compiler plugins to provide objtool that
> data (yuck, since we support at least 2 different compilers), disable
> jump-tables (yuck, for that limits code-gen just to please a tool) or
> use DWARF (yuck, because build times).
> 
> There was a little talk about an impromptu 'abi' to communicate
> jump-table details to objtool without going full on DWARF, but that
> seems to have hit a dead end again.
Josh Poimboeuf March 28, 2022, 8:21 p.m. UTC | #13
On Mon, Mar 28, 2022 at 10:14:38PM +0200, Peter Zijlstra wrote:
> > FWIW, there have been some objtool patches for arm64 stack validation,
> > but the arm64 maintainers have been hesitant to get on board with
> > objtool, as it brings a certain maintenance burden.  Especially for the
> > full stack validation and ORC unwinder.  But if you only want inline
> > static calls and/or mcount then it'd probably be much easier to
> > maintain.
> 
> IIRC the major stumbling block for arm64 is the whole jump-table thing.
> Either they need to rely on compiler plugins to provide objtool that
> data (yuck, since we support at least 2 different compilers), disable
> jump-tables (yuck, for that limits code-gen just to please a tool) or
> use DWARF (yuck, because build times).

Well yeah, that was indeed the main technical issue but I seem to
remember some arm64 maintainers not really being sold on the value of
objtool regardless.

> There was a little talk about an impromptu 'abi' to communicate
> jump-table details to objtool without going full on DWARF, but that
> seems to have hit a dead end again.

Probably my fault, not enough hours in the day...
Michael Ellerman March 29, 2022, 12:01 p.m. UTC | #14
Josh Poimboeuf <jpoimboe@redhat.com> writes:
> On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
>> Second point is the endianess and 32/64 selection, especially when 
>> crossbuilding. There is already some stuff regarding endianess based on 
>> bswap_if_needed() but that's based on constant selection at build time 
>> and I couldn't find an easy way to set it conditionaly based on the 
>> target being built.
>>
>> Regarding 32/64 selection, there is almost nothing, it's based on using 
>> type 'long' which means that at the time being the target and the build 
>> platform must both be 32 bits or 64 bits.
>> 
>> For both cases (endianess and 32/64) I think the solution should 
>> probably be to start with the fileformat of the object file being 
>> reworked by objtool.
>
> Do we really need to detect the endianness/bitness at runtime?  Objtool
> is built with the kernel, why not just build-in the same target
> assumptions as the kernel itself?

I don't think we need runtime detection. But it will need to support
basically most combinations of objtool running as 32-bit/64-bit LE/BE
while the kernel it's analysing is 32-bit/64-bit LE/BE.

>> What are current works in progress on objtool ? Should I wait Josh's 
>> changes before starting looking at all this ? Should I wait for anything 
>> else ?
>
> I'm not making any major changes to the code, just shuffling things
> around to make the interface more modular.  I hope to have something
> soon (this week).  Peter recently added a big feature (Intel IBT) which
> is already in -next.
>
> Contributions are welcome, with the understanding that you'll help
> maintain it ;-)
>
> Some years ago Kamalesh Babulal had a prototype of objtool for ppc64le
> which did the full stack validation.  I'm not sure what ever became of
> that.

From memory he was starting to clean the patches up in late 2019, but I
guess that probably got derailed by COVID. AFAIK he never posted
anything. Maybe someone at IBM has a copy internally (Naveen?).

> FWIW, there have been some objtool patches for arm64 stack validation,
> but the arm64 maintainers have been hesitant to get on board with
> objtool, as it brings a certain maintenance burden.  Especially for the
> full stack validation and ORC unwinder.  But if you only want inline
> static calls and/or mcount then it'd probably be much easier to
> maintain.

I would like to have the stack validation, but I am also worried about
the maintenance burden.

I guess we start with mcount, which looks pretty minimal judging by this
series, and see how we go from there.

cheers
Christophe Leroy March 29, 2022, 5:32 p.m. UTC | #15
Le 29/03/2022 à 14:01, Michael Ellerman a écrit :
> Josh Poimboeuf <jpoimboe@redhat.com> writes:
>> On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
>>> Second point is the endianess and 32/64 selection, especially when
>>> crossbuilding. There is already some stuff regarding endianess based on
>>> bswap_if_needed() but that's based on constant selection at build time
>>> and I couldn't find an easy way to set it conditionaly based on the
>>> target being built.
>>>
>>> Regarding 32/64 selection, there is almost nothing, it's based on using
>>> type 'long' which means that at the time being the target and the build
>>> platform must both be 32 bits or 64 bits.
>>>
>>> For both cases (endianess and 32/64) I think the solution should
>>> probably be to start with the fileformat of the object file being
>>> reworked by objtool.
>>
>> Do we really need to detect the endianness/bitness at runtime?  Objtool
>> is built with the kernel, why not just build-in the same target
>> assumptions as the kernel itself?
> 
> I don't think we need runtime detection. But it will need to support
> basically most combinations of objtool running as 32-bit/64-bit LE/BE
> while the kernel it's analysing is 32-bit/64-bit LE/BE.

Exactly, the way it is done today with a constant in 
objtool/endianness.h is too simple, we need to be able to select it 
based on kernel's config. Is there a way to get the CONFIG_ macros from 
the kernel ? If yes then we could use CONFIG_64BIT and 
CONFIG_CPU_LITTLE_ENDIAN to select the correct options in objtool.


> 
>>> What are current works in progress on objtool ? Should I wait Josh's
>>> changes before starting looking at all this ? Should I wait for anything
>>> else ?
>>
>> I'm not making any major changes to the code, just shuffling things
>> around to make the interface more modular.  I hope to have something
>> soon (this week).  Peter recently added a big feature (Intel IBT) which
>> is already in -next.
>>
>> Contributions are welcome, with the understanding that you'll help
>> maintain it ;-)
>>
>> Some years ago Kamalesh Babulal had a prototype of objtool for ppc64le
>> which did the full stack validation.  I'm not sure what ever became of
>> that.
> 
>  From memory he was starting to clean the patches up in late 2019, but I
> guess that probably got derailed by COVID. AFAIK he never posted
> anything. Maybe someone at IBM has a copy internally (Naveen?).
> 
>> FWIW, there have been some objtool patches for arm64 stack validation,
>> but the arm64 maintainers have been hesitant to get on board with
>> objtool, as it brings a certain maintenance burden.  Especially for the
>> full stack validation and ORC unwinder.  But if you only want inline
>> static calls and/or mcount then it'd probably be much easier to
>> maintain.
> 
> I would like to have the stack validation, but I am also worried about
> the maintenance burden.
> 
> I guess we start with mcount, which looks pretty minimal judging by this
> series, and see how we go from there.
> 

I'm not sure mcount is really needed as we have recordmcount, but at 
least it is an easy one to start with and as we have recordmount we can 
easily compare the results and check it works as expected.

Then it should be straight forward to provide static calls.

Then I'd like to go with uaccess blocks checks as suggested by Christoph 
at 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/a94be61f008ab29c231b805e1a97e9dab35cb0cc.1629732940.git.christophe.leroy@csgroup.eu/, 
thought it might be less easy.


Christophe
Josh Poimboeuf March 30, 2022, 4:26 a.m. UTC | #16
On Tue, Mar 29, 2022 at 05:32:18PM +0000, Christophe Leroy wrote:
> 
> 
> Le 29/03/2022 à 14:01, Michael Ellerman a écrit :
> > Josh Poimboeuf <jpoimboe@redhat.com> writes:
> >> On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
> >>> Second point is the endianess and 32/64 selection, especially when
> >>> crossbuilding. There is already some stuff regarding endianess based on
> >>> bswap_if_needed() but that's based on constant selection at build time
> >>> and I couldn't find an easy way to set it conditionaly based on the
> >>> target being built.
> >>>
> >>> Regarding 32/64 selection, there is almost nothing, it's based on using
> >>> type 'long' which means that at the time being the target and the build
> >>> platform must both be 32 bits or 64 bits.
> >>>
> >>> For both cases (endianess and 32/64) I think the solution should
> >>> probably be to start with the fileformat of the object file being
> >>> reworked by objtool.
> >>
> >> Do we really need to detect the endianness/bitness at runtime?  Objtool
> >> is built with the kernel, why not just build-in the same target
> >> assumptions as the kernel itself?
> > 
> > I don't think we need runtime detection. But it will need to support
> > basically most combinations of objtool running as 32-bit/64-bit LE/BE
> > while the kernel it's analysing is 32-bit/64-bit LE/BE.
> 
> Exactly, the way it is done today with a constant in 
> objtool/endianness.h is too simple, we need to be able to select it 
> based on kernel's config. Is there a way to get the CONFIG_ macros from 
> the kernel ? If yes then we could use CONFIG_64BIT and 
> CONFIG_CPU_LITTLE_ENDIAN to select the correct options in objtool.

As of now, there's no good way to get CONFIG options from the kernel.
That's pretty much by design, since objtool is meant to be a standalone
tool.  In fact there are people who've used objtool for other projects.

The objtool Makefile does at least have access to HOSTARCH/SRCARCH, but
I guess that doesn't help here.  We could maybe export the endian/bit
details in env variables to the objtool build somehow.

But, I managed to forget that objtool can already be cross-compiled for
a x86-64 target, from a 32-bit x86 LE host or a 64-bit powerpc BE host.
There are some people out there doing x86 kernel builds on such systems
who reported bugs, which were since fixed.  And the fixes were pretty
trivial, IIRC.

Libelf actually does a decent job of abstracting those details from
objtool.  So, forget what I said, it might be ok to just detect
endian/bit (and possibly even arch) at runtime like you originally
suggested.

For example bswap_if_needed() could be reworked to be a runtime check.
Naveen N. Rao March 30, 2022, 6:40 p.m. UTC | #17
Christophe Leroy wrote:
> 
> 
> Le 29/03/2022 à 14:01, Michael Ellerman a écrit :
>> Josh Poimboeuf <jpoimboe@redhat.com> writes:
>>> On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
>>>> What are current works in progress on objtool ? Should I wait Josh's
>>>> changes before starting looking at all this ? Should I wait for anything
>>>> else ?
>>>
>>> I'm not making any major changes to the code, just shuffling things
>>> around to make the interface more modular.  I hope to have something
>>> soon (this week).  Peter recently added a big feature (Intel IBT) which
>>> is already in -next.
>>>
>>> Contributions are welcome, with the understanding that you'll help
>>> maintain it ;-)
>>>
>>> Some years ago Kamalesh Babulal had a prototype of objtool for ppc64le
>>> which did the full stack validation.  I'm not sure what ever became of
>>> that.
>> 
>>  From memory he was starting to clean the patches up in late 2019, but I
>> guess that probably got derailed by COVID. AFAIK he never posted
>> anything. Maybe someone at IBM has a copy internally (Naveen?).

Kamalesh had a WIP series to enable stack validation on powerpc. From 
what I recall, he was waiting on and/or working with the arm64 folks 
around some of the common changes needed in objtool.

>> 
>>> FWIW, there have been some objtool patches for arm64 stack validation,
>>> but the arm64 maintainers have been hesitant to get on board with
>>> objtool, as it brings a certain maintenance burden.  Especially for the
>>> full stack validation and ORC unwinder.  But if you only want inline
>>> static calls and/or mcount then it'd probably be much easier to
>>> maintain.
>> 
>> I would like to have the stack validation, but I am also worried about
>> the maintenance burden.
>> 
>> I guess we start with mcount, which looks pretty minimal judging by this
>> series, and see how we go from there.
>> 
> 
> I'm not sure mcount is really needed as we have recordmcount, but at 
> least it is an easy one to start with and as we have recordmount we can 
> easily compare the results and check it works as expected.

On the contrary, I think support for mcount in objtool is something we 
want to get going soon (hopefully, in time for v5.19) given the issues 
we are seeing with recordmcount:
- https://github.com/linuxppc/issues/issues/388
- https://lore.kernel.org/all/20220211014313.1790140-1-aik@ozlabs.ru/


- Naveen
Christophe Leroy May 12, 2022, 2:52 p.m. UTC | #18
Hi Josh,

Le 28/03/2022 à 21:59, Josh Poimboeuf a écrit :
> On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
>> What are current works in progress on objtool ? Should I wait Josh's
>> changes before starting looking at all this ? Should I wait for anything
>> else ?
> 
> I'm not making any major changes to the code, just shuffling things
> around to make the interface more modular.  I hope to have something
> soon (this week).  Peter recently added a big feature (Intel IBT) which
> is already in -next.
> 

Were you able to send out something ?

Thanks
Christophe
Josh Poimboeuf May 12, 2022, 3:12 p.m. UTC | #19
On Thu, May 12, 2022 at 02:52:40PM +0000, Christophe Leroy wrote:
> Hi Josh,
> 
> Le 28/03/2022 à 21:59, Josh Poimboeuf a écrit :
> > On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
> >> What are current works in progress on objtool ? Should I wait Josh's
> >> changes before starting looking at all this ? Should I wait for anything
> >> else ?
> > 
> > I'm not making any major changes to the code, just shuffling things
> > around to make the interface more modular.  I hope to have something
> > soon (this week).  Peter recently added a big feature (Intel IBT) which
> > is already in -next.
> > 
> 
> Were you able to send out something ?

Yes, the objtool rewrite is now in tip/objtool/core and linux-next.
Christophe Leroy May 21, 2022, 9:38 a.m. UTC | #20
Le 12/05/2022 à 17:12, Josh Poimboeuf a écrit :
> On Thu, May 12, 2022 at 02:52:40PM +0000, Christophe Leroy wrote:
>> Hi Josh,
>>
>> Le 28/03/2022 à 21:59, Josh Poimboeuf a écrit :
>>> On Sun, Mar 27, 2022 at 09:09:20AM +0000, Christophe Leroy wrote:
>>>> What are current works in progress on objtool ? Should I wait Josh's
>>>> changes before starting looking at all this ? Should I wait for anything
>>>> else ?
>>>
>>> I'm not making any major changes to the code, just shuffling things
>>> around to make the interface more modular.  I hope to have something
>>> soon (this week).  Peter recently added a big feature (Intel IBT) which
>>> is already in -next.
>>>
>>
>> Were you able to send out something ?
> 
> Yes, the objtool rewrite is now in tip/objtool/core and linux-next.

Nice.

I gave it a try this morning, I selected HAVE_OBJTOOL and 
HAVE_OBJTOOL_MCOUNT from arch/powerpc/Kconfig


Seems like there are still some x86 arch specific stuff in common common 
code and I get the following errors.

Also, is it normal to get those functions built allthough I have not 
selected HAVE_STACK_VALIDATION ?

   CC      /home/chleroy/linux-powerpc/tools/objtool/check.o
check.c: In function 'has_valid_stack_frame':
check.c:2369:30: error: 'CFI_BP' undeclared (first use in this 
function); did you mean 'CFI_SP'?
  2369 |         if (cfi->cfa.base == CFI_BP &&
       |                              ^~~~~~
       |                              CFI_SP
check.c:2369:30: note: each undeclared identifier is reported only once 
for each function it appears in
check.c:2371:44: error: 'CFI_RA' undeclared (first use in this 
function); did you mean 'CFI_R3'?
  2371 |             check_reg_frame_pos(&cfi->regs[CFI_RA], 
-cfi->cfa.offset + 8))
       |                                            ^~~~~~
       |                                            CFI_R3
check.c: In function 'update_cfi_state':
check.c:2499:70: error: 'CFI_BP' undeclared (first use in this 
function); did you mean 'CFI_SP'?
  2499 |                         if (op->src.reg == CFI_SP && 
op->dest.reg == CFI_BP &&
       | 
       ^~~~~~
       | 
       CFI_SP
make[3]: *** [/home/chleroy/linux-powerpc/tools/build/Makefile.build:97: 
/home/chleroy/linux-powerpc/tools/objtool/check.o] Error 1
make[2]: *** [Makefile:54: 
/home/chleroy/linux-powerpc/tools/objtool/objtool-in.o] Error 2
make[1]: *** [Makefile:69: objtool] Error 2
make: *** [Makefile:1337: tools/objtool] Error 2


What would be the best approach to fix that ?

Thanks
Christophe
Peter Zijlstra May 21, 2022, 10:57 a.m. UTC | #21
On Sat, May 21, 2022 at 09:38:35AM +0000, Christophe Leroy wrote:

> I gave it a try this morning, I selected HAVE_OBJTOOL and 
> HAVE_OBJTOOL_MCOUNT from arch/powerpc/Kconfig
> 
> 
> Seems like there are still some x86 arch specific stuff in common common 
> code and I get the following errors.

I'm assuming there's a metric ton of x86 specific stuff in there.
That'll take a while to clean out.

Mostly Josh's rewrite was centered around splitting out the runtime
options, but objtool is still always build with all options included,
even the ones you're not using for your arch, which is what's triggering
the problems you see here, I suppose...

> Also, is it normal to get those functions built allthough I have not 
> selected HAVE_STACK_VALIDATION ?
> 
>    CC      /home/chleroy/linux-powerpc/tools/objtool/check.o
> check.c: In function 'has_valid_stack_frame':
> check.c:2369:30: error: 'CFI_BP' undeclared (first use in this 
> function); did you mean 'CFI_SP'?
>   2369 |         if (cfi->cfa.base == CFI_BP &&
>        |                              ^~~~~~
>        |                              CFI_SP
> check.c:2369:30: note: each undeclared identifier is reported only once 
> for each function it appears in
> check.c:2371:44: error: 'CFI_RA' undeclared (first use in this 
> function); did you mean 'CFI_R3'?
>   2371 |             check_reg_frame_pos(&cfi->regs[CFI_RA], 
> -cfi->cfa.offset + 8))
>        |                                            ^~~~~~
>        |                                            CFI_R3
> check.c: In function 'update_cfi_state':
> check.c:2499:70: error: 'CFI_BP' undeclared (first use in this 
> function); did you mean 'CFI_SP'?
>   2499 |                         if (op->src.reg == CFI_SP && 
> op->dest.reg == CFI_BP &&
>        | 
>        ^~~~~~
>        | 
>        CFI_SP
> make[3]: *** [/home/chleroy/linux-powerpc/tools/build/Makefile.build:97: 
> /home/chleroy/linux-powerpc/tools/objtool/check.o] Error 1
> make[2]: *** [Makefile:54: 
> /home/chleroy/linux-powerpc/tools/objtool/objtool-in.o] Error 2
> make[1]: *** [Makefile:69: objtool] Error 2
> make: *** [Makefile:1337: tools/objtool] Error 2
> 
> 
> What would be the best approach to fix that ?

Define CFI_BP to your frame register (r2, afaict) I suppose. If you're
only using OBJTOOL_MCOUNT this code will never run, so all you have to
ensure is that it compiles, not that it makes sense (-:

The very long and complicated way would be to propagate the various
CONFIG_HAVE_* build time things to the objtool build and librally
sprinkle a lot of #ifdef around.
Naveen N. Rao May 23, 2022, 5:39 a.m. UTC | #22
Peter Zijlstra wrote:
> On Sat, May 21, 2022 at 09:38:35AM +0000, Christophe Leroy wrote:
> 
>> I gave it a try this morning, I selected HAVE_OBJTOOL and 
>> HAVE_OBJTOOL_MCOUNT from arch/powerpc/Kconfig
>> 
>> 
>> Seems like there are still some x86 arch specific stuff in common common 
>> code and I get the following errors.
> 
> I'm assuming there's a metric ton of x86 specific stuff in there.
> That'll take a while to clean out.
> 
> Mostly Josh's rewrite was centered around splitting out the runtime
> options, but objtool is still always build with all options included,
> even the ones you're not using for your arch, which is what's triggering
> the problems you see here, I suppose...
> 
>> Also, is it normal to get those functions built allthough I have not 
>> selected HAVE_STACK_VALIDATION ?
>> 
>>    CC      /home/chleroy/linux-powerpc/tools/objtool/check.o
>> check.c: In function 'has_valid_stack_frame':
>> check.c:2369:30: error: 'CFI_BP' undeclared (first use in this 
>> function); did you mean 'CFI_SP'?
>>   2369 |         if (cfi->cfa.base == CFI_BP &&
>>        |                              ^~~~~~
>>        |                              CFI_SP
>> check.c:2369:30: note: each undeclared identifier is reported only once 
>> for each function it appears in
>> check.c:2371:44: error: 'CFI_RA' undeclared (first use in this 
>> function); did you mean 'CFI_R3'?
>>   2371 |             check_reg_frame_pos(&cfi->regs[CFI_RA], 
>> -cfi->cfa.offset + 8))
>>        |                                            ^~~~~~
>>        |                                            CFI_R3
>> check.c: In function 'update_cfi_state':
>> check.c:2499:70: error: 'CFI_BP' undeclared (first use in this 
>> function); did you mean 'CFI_SP'?
>>   2499 |                         if (op->src.reg == CFI_SP && 
>> op->dest.reg == CFI_BP &&
>>        | 
>>        ^~~~~~
>>        | 
>>        CFI_SP
>> make[3]: *** [/home/chleroy/linux-powerpc/tools/build/Makefile.build:97: 
>> /home/chleroy/linux-powerpc/tools/objtool/check.o] Error 1
>> make[2]: *** [Makefile:54: 
>> /home/chleroy/linux-powerpc/tools/objtool/objtool-in.o] Error 2
>> make[1]: *** [Makefile:69: objtool] Error 2
>> make: *** [Makefile:1337: tools/objtool] Error 2
>> 
>> 
>> What would be the best approach to fix that ?
> 
> Define CFI_BP to your frame register (r2, afaict) I suppose. If you're
> only using OBJTOOL_MCOUNT this code will never run, so all you have to
> ensure is that it compiles, not that it makes sense (-:

Sathvika has been looking into this.

> 
> The very long and complicated way would be to propagate the various
> CONFIG_HAVE_* build time things to the objtool build and librally
> sprinkle a lot of #ifdef around.

I think there were just a couple of unrelated checks/warnings that were 
causing problems on powerpc. So, we likely won't need too many #ifdefs, 
at least for mcount purposes.

Sathvika,
Can you post what you have?


- Naveen
diff mbox series

Patch

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index b779603978e1..5ddafd3ce210 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -225,6 +225,7 @@  config PPC
 	select HAVE_MOD_ARCH_SPECIFIC
 	select HAVE_NMI				if PERF_EVENTS || (PPC64 && PPC_BOOK3S)
 	select HAVE_OPTPROBES
+	select HAVE_OBJTOOL_MCOUNT
 	select HAVE_PERF_EVENTS
 	select HAVE_PERF_EVENTS_NMI		if PPC64
 	select HAVE_PERF_REGS
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index 8404cf696954..06a9fcb4a0a3 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -49,6 +49,10 @@  ifeq ($(SRCARCH),x86)
 	SUBCMD_MCOUNT := y
 endif
 
+ifeq ($(SRCARCH),powerpc)
+        SUBCMD_MCOUNT := y
+endif
+
 export SUBCMD_CHECK SUBCMD_ORC SUBCMD_MCOUNT
 export srctree OUTPUT CFLAGS SRCARCH AWK
 include $(srctree)/tools/build/Makefile.include
diff --git a/tools/objtool/arch/powerpc/Build b/tools/objtool/arch/powerpc/Build
new file mode 100644
index 000000000000..3ff1f00c6a47
--- /dev/null
+++ b/tools/objtool/arch/powerpc/Build
@@ -0,0 +1 @@ 
+objtool-y += decode.o
diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/powerpc/decode.c
new file mode 100644
index 000000000000..58988f88b315
--- /dev/null
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -0,0 +1,51 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include <objtool/check.h>
+#include <objtool/elf.h>
+#include <objtool/arch.h>
+#include <objtool/warn.h>
+#include <objtool/builtin.h>
+
+int arch_decode_instruction(struct objtool_file *file, const struct section *sec,
+			    unsigned long offset, unsigned int maxlen,
+			    unsigned int *len, enum insn_type *type,
+			    unsigned long *immediate,
+			    struct list_head *ops_list)
+{
+	u32 insn;
+	unsigned int opcode;
+	u64 imm;
+
+	*immediate = imm = 0;
+	memcpy(&insn, sec->data->d_buf+offset, 4);
+	*len = 4;
+	*type = INSN_OTHER;
+
+	opcode = (insn >> 26);
+
+	switch (opcode) {
+	case 18: /* bl */
+		*type = INSN_CALL;
+		break;
+	}
+	*immediate = imm;
+	return 0;
+}
+
+unsigned long arch_dest_reloc_offset(int addend)
+{
+	return addend;
+}
+
+unsigned long arch_jump_destination(struct instruction *insn)
+{
+	return insn->offset +  insn->immediate;
+}
+
+const char *arch_nop_insn(int len)
+{
+	return NULL;
+}
diff --git a/tools/objtool/arch/powerpc/include/arch/cfi_regs.h b/tools/objtool/arch/powerpc/include/arch/cfi_regs.h
new file mode 100644
index 000000000000..1369176c8a94
--- /dev/null
+++ b/tools/objtool/arch/powerpc/include/arch/cfi_regs.h
@@ -0,0 +1,37 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef _OBJTOOL_CFI_REGS_H
+#define _OBJTOOL_CFI_REGS_H
+
+#define CFI_SP                  1
+#define CFI_TOC                 2
+#define CFI_R3                  3
+#define CFI_R4                  4
+#define CFI_R5                  5
+#define CFI_R6                  6
+#define CFI_R7                  7
+#define CFI_R8                  8
+#define CFI_R9                  9
+#define CFI_R10                 10
+#define CFI_R14                 14
+#define CFI_R15                 15
+#define CFI_R16                 16
+#define CFI_R17                 17
+#define CFI_R18                 18
+#define CFI_R19                 19
+#define CFI_R20                 20
+#define CFI_R21                 21
+#define CFI_R22                 22
+#define CFI_R23                 23
+#define CFI_R24                 24
+#define CFI_R25                 25
+#define CFI_R26                 26
+#define CFI_R27                 27
+#define CFI_R28                 28
+#define CFI_R29                 29
+#define CFI_R30                 30
+#define CFI_R31                 31
+#define CFI_LR                  32
+#define CFI_NUM_REGS            33
+
+#endif
diff --git a/tools/objtool/arch/powerpc/include/arch/elf.h b/tools/objtool/arch/powerpc/include/arch/elf.h
new file mode 100644
index 000000000000..3c8ebb7d2a6b
--- /dev/null
+++ b/tools/objtool/arch/powerpc/include/arch/elf.h
@@ -0,0 +1,8 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef _OBJTOOL_ARCH_ELF
+#define _OBJTOOL_ARCH_ELF
+
+#define R_NONE R_PPC_NONE
+
+#endif /* _OBJTOOL_ARCH_ELF */
diff --git a/tools/objtool/utils.c b/tools/objtool/utils.c
index d1fc6a123a6e..c9c14fa0dfd7 100644
--- a/tools/objtool/utils.c
+++ b/tools/objtool/utils.c
@@ -179,11 +179,29 @@  int create_mcount_loc_sections(struct objtool_file *file)
 		loc = (unsigned long *)sec->data->d_buf + idx;
 		memset(loc, 0, sizeof(unsigned long));
 
-		if (elf_add_reloc_to_insn(file->elf, sec,
-					  idx * sizeof(unsigned long),
-					  R_X86_64_64,
-					  insn->sec, insn->offset))
-			return -1;
+		if (file->elf->ehdr.e_machine == EM_X86_64) {
+			if (elf_add_reloc_to_insn(file->elf, sec,
+						  idx * sizeof(unsigned long),
+						  R_X86_64_64,
+						  insn->sec, insn->offset))
+				return -1;
+		}
+
+		if (file->elf->ehdr.e_machine == EM_PPC64) {
+			if (elf_add_reloc_to_insn(file->elf, sec,
+						  idx * sizeof(unsigned long),
+						  R_PPC64_ADDR64,
+						  insn->sec, insn->offset))
+				return -1;
+		}
+
+		if (file->elf->ehdr.e_machine == EM_PPC) {
+			if (elf_add_reloc_to_insn(file->elf, sec,
+						  idx * sizeof(unsigned long),
+						  R_PPC_ADDR32,
+						  insn->sec, insn->offset))
+				return -1;
+		}
 
 		idx++;
 	}