Re: [RFC][PATCH 15/16] objtool: Implement noinstr validation

From: Josh Poimboeuf
Date: Mon Mar 16 2020 - 12:26:12 EST


On Mon, Mar 16, 2020 at 02:24:19PM +0100, Peter Zijlstra wrote:
> > And "read_instr_hints" reads as "read_instruction_hints()".
> >
> > Can we come up with a more readable name? Why not just "notrace"?
> >
> > The trace begin/end annotations could be
> >
> > trace_allow_begin()
> > trace_allow_end()
>
> notrace already exists and we didn't want to confuse things further.

Um, why would it confuse things to call a section of notrace code
".notrace.text"???

> > Also -- what happens when a function belongs in both .notrace.text and
> > in one of the other special-purpose sections like .sched.text,
> > .meminit.text or .entry.text?
>
> Hasn't happened yet, initially we were thinking of using .entry.text for
> this as a whole, but decided against that due to how .entry.text is
> special for PTI (although exposing most of this code really wouldn't
> matter).
>
> The thing with .sched.text is that we really should never call into
> scheduling from these contexts anyway. We've not ran into meminit yet.
> (all this finicky entry code is ran with IRQs disabled).
>
> The one that could potentially interfere is .cpuidle.text.
>
> > Maybe storing pointers to the functions, like NOKPROBE_SYMBOL does,
> > would be better than putting the functions in a separate section.
>
> Thing is, I really _hate_ that annotation style.

I do too, but I get the feeling this "put everything in its own section"
thing is going to bite us.

> > Also, maybe we can just hard-code the fact that vmlinux.o is always
> > noinstr-only. Over time we'll probably need to move more per-.o
> > functionalities to vmlinux.o and I think we should avoid creating a
> > bunch of cmdline options.
>
> but you're ruining things here, see, for a regular x86_64 config, we'd
> run this with:
>
> objtool check -fail vmlinux.o
>
> And I was hoping to get vmlinux.o objtool clean, surprisingly there
> really aren't that many complaints. But the -i thing makes it run
> significantly faster without duplicating all the bits we've already
> checked.

My suggestion is that the "-i" option would be hard-coded (for now). So
nothing extra would get checked.

--
Josh