2020-04-28 19:25:36

by Peter Zijlstra

[permalink] [raw]
Subject: [PATCH v2 00/14] objtool vs retpoline

Hi,

Based on Alexandre's patches, here's a few that go on top of tip/objtool/core.

With these patches on objtool can completely understand retpolines and RSB
stuffing, which means it can emit valid ORC unwind information for them, which
in turn means we can now unwind through a retpoline.

New since last time:

- 1-3, alternatives vs ORC unwind
- 7-9: implement some suggestions from Julien
- addressed feedback


2020-04-28 20:20:59

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH v2 00/14] objtool vs retpoline

On Tue, Apr 28, 2020 at 09:11:01PM +0200, Peter Zijlstra wrote:
> Hi,
>
> Based on Alexandre's patches, here's a few that go on top of tip/objtool/core.
>
> With these patches on objtool can completely understand retpolines and RSB
> stuffing, which means it can emit valid ORC unwind information for them, which
> in turn means we can now unwind through a retpoline.
>
> New since last time:
>
> - 1-3, alternatives vs ORC unwind
> - 7-9: implement some suggestions from Julien
> - addressed feedback

For patches 2-14:

Acked-by: Josh Poimboeuf <[email protected]>

Instead of patch 1, maybe you could grab Julien's patch to drop the
useless warning; and also grab the alt_group numbering and the
"orig->alt_group = true" bits from Alexandre's patch.

--
Josh

2020-04-29 10:23:30

by Peter Zijlstra

[permalink] [raw]
Subject: [PATCH v2.1 01-B/14] objtool: Uniquely identify alternative instruction groups

Subject: objtool: Uniquely identify alternative instruction groups
From: Alexandre Chartre <[email protected]>
Date: Tue, 14 Apr 2020 12:36:11 +0200

From: Alexandre Chartre <[email protected]>

Assign a unique identifier to every alternative instruction group in
order to be able to tell which instructions belong to what
alternative.

[peterz: extracted from a larger patch]
Signed-off-by: Alexandre Chartre <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
tools/objtool/check.c | 26 ++++++++++++++++++++------
tools/objtool/check.h | 3 ++-
2 files changed, 22 insertions(+), 7 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -770,7 +770,9 @@ static int handle_group_alt(struct objto
struct instruction *orig_insn,
struct instruction **new_insn)
{
+ static unsigned int alt_group_next_index = 1;
struct instruction *last_orig_insn, *last_new_insn, *insn, *fake_jump = NULL;
+ unsigned int alt_group = alt_group_next_index++;
unsigned long dest_off;

last_orig_insn = NULL;
@@ -779,7 +781,7 @@ static int handle_group_alt(struct objto
if (insn->offset >= special_alt->orig_off + special_alt->orig_len)
break;

- insn->alt_group = true;
+ insn->alt_group = alt_group;
last_orig_insn = insn;
}

@@ -813,6 +815,7 @@ static int handle_group_alt(struct objto
}

last_new_insn = NULL;
+ alt_group = alt_group_next_index++;
insn = *new_insn;
sec_for_each_insn_from(file, insn) {
if (insn->offset >= special_alt->new_off + special_alt->new_len)
@@ -822,6 +825,7 @@ static int handle_group_alt(struct objto

insn->ignore = orig_insn->ignore_alts;
insn->func = orig_insn->func;
+ insn->alt_group = alt_group;

/*
* Since alternative replacement code is copy/pasted by the
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -30,12 +30,13 @@ struct instruction {
unsigned int len;
enum insn_type type;
unsigned long immediate;
- bool alt_group, dead_end, ignore, ignore_alts;
+ bool dead_end, ignore, ignore_alts;
bool hint;
bool retpoline_safe;
s8 instr;
u8 visited;
u8 ret_offset;
+ int alt_group;
struct symbol *call_dest;
struct instruction *jump_dest;
struct instruction *first_jump_src;

2020-04-29 10:24:19

by Peter Zijlstra

[permalink] [raw]
Subject: [PATCH v2.1 01-A/14] objtool: Remove check preventing branches within alternative

Subject: objtool: Remove check preventing branches within alternative
From: Julien Thierry <[email protected]>
Date: Fri, 27 Mar 2020 15:28:42 +0000

From: Julien Thierry <[email protected]>

While jumping from outside an alternative region to the middle of an
alternative region is very likely wrong, jumping from an alternative
region into the same region is valid. It is a common pattern on arm64.

The first pattern is unlikely to happen in practice and checking only
for this adds a lot of complexity.

Just remove the current check.

Suggested-by: Josh Poimboeuf <[email protected]>
Signed-off-by: Julien Thierry <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
tools/objtool/check.c | 6 ------
1 file changed, 6 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2180,12 +2180,6 @@ static int validate_branch(struct objtoo

sec = insn->sec;

- if (insn->alt_group && list_empty(&insn->alts)) {
- WARN_FUNC("don't know how to handle branch to middle of alternative instruction group",
- sec, insn->offset);
- return 1;
- }
-
while (1) {
next_insn = next_insn_same_sec(file, insn);

2020-04-29 16:47:55

by Miroslav Benes

[permalink] [raw]
Subject: Re: [PATCH v2 00/14] objtool vs retpoline

On Tue, 28 Apr 2020, Peter Zijlstra wrote:

> Hi,
>
> Based on Alexandre's patches, here's a few that go on top of tip/objtool/core.
>
> With these patches on objtool can completely understand retpolines and RSB
> stuffing, which means it can emit valid ORC unwind information for them, which
> in turn means we can now unwind through a retpoline.
>
> New since last time:
>
> - 1-3, alternatives vs ORC unwind
> - 7-9: implement some suggestions from Julien
> - addressed feedback

You can add my

Reviewed-by: Miroslav Benes <[email protected]>

to patches 1A, 1B and 2-10 (objtool patches and updated smap fix). The
other four patches should be fine too, but I am not well versed in the
speculation stuff.

Miroslav