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;
Hi,
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 ?
Is your series a replacement of scripts/recordmcount.c ?
If so, is there anything common with the following RFC:
https://lore.kernel.org/lkml/[email protected]/
Thanks
Christophe
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.
cheers
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 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
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?
Yes something must have gone wrong.
Peter's and myself's responses are not at
http://patchwork.ozlabs.org/project/linuxppc-dev/patch/[email protected]/
while yours and Naveen's ones are there.
Christophe
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
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
Christophe
Le 21/03/2022 a 08:56, Christophe Leroy a ecrit :
>
>
> Le 21/03/2022 a 03:27, Michael Ellerman a ecrit :
>> Christophe Leroy <[email protected]> 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 <[email protected]>
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 <[email protected]>
---
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;
--
2.35.1
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
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.
--
Josh
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.
+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 <[email protected]> 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
Le 29/03/2022 à 14:01, Michael Ellerman a écrit :
> Josh Poimboeuf <[email protected]> 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
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 <[email protected]> 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.
--
Josh
Christophe Leroy wrote:
>
>
> Le 29/03/2022 à 14:01, Michael Ellerman a écrit :
>> Josh Poimboeuf <[email protected]> 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/[email protected]/
- Naveen
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
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.
--
Josh
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