2022-03-18 18:22:57

by Sathvika Vasireddy

[permalink] [raw]
Subject: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

This patch adds powerpc specific functions required for
'objtool mcount' to work, and enables mcount for ppc.

Signed-off-by: Sathvika Vasireddy <[email protected]>
---
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

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++;
}
--
2.31.1


2022-03-21 21:05:39

by Christophe Leroy

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions



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 <[email protected]> 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

2022-03-21 21:17:39

by Christophe Leroy

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions



Le 21/03/2022 à 08:56, Christophe Leroy a écrit :
>
>
> Le 21/03/2022 à 03:27, Michael Ellerman a écrit :
>> Christophe Leroy <[email protected]> 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

2022-03-28 22:31:01

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

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

--
Josh

2022-05-22 03:10:44

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

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.


2022-05-23 08:05:17

by Naveen N. Rao

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] objtool/mcount: Add powerpc specific functions

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