Re: [PATCH v2 -tip] x86/percpu: Use C for arch_raw_cpu_ptr()

From: Josh Poimboeuf
Date: Thu Oct 12 2023 - 17:30:48 EST


On Thu, Oct 12, 2023 at 07:59:43PM +0200, Ingo Molnar wrote:
>
> * Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
>
> > On Thu, Oct 12, 2023 at 08:19:14AM +0200, Ingo Molnar wrote:
> > >
> > > * Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote:
> > >
> > > > Though, another problem is that .text has a crazy amount of padding
> > > > which makes it always the same size, due to the SRSO alias mitigation
> > > > alignment linker magic. We should fix that somehow.
> > >
> > > We could emit a non-aligned end-of-text symbol (we might have it already),
> > > and have a script or small .c program in scripts/ or tools/ that looks
> > > at vmlinux and displays a user-friendly and accurate list of text and
> > > data sizes in the kernel?
> > >
> > > And since objtool is technically an 'object files tool', and it already
> > > looks at sections & symbols, it could also grow a:
> > >
> > > objtool size <objfile>
> > >
> > > command that does the sane thing ... I'd definitely start using that, instead of 'size'.
> > >
> > > /me runs :-)
> >
> > Yeah, that's actually not a bad idea.
> >
> > I had been thinking a "simple" script would be fine, but I'm realizing
> > the scope of this thing could grow over time. In which case a script is
> > less than ideal. And objtool already has the ability to do this pretty
> > easily.
>
> Yeah, and speed actually matters here: I have scripts that generate object
> comparisons between commits, and every second of runtime counts - and a
> script would be slower and more fragile for something like allmodconfig
> builds or larger disto configs.

Ah, good to know.

> BTW., maybe the right objtool subcommand would be 'objtool sections', with
> an 'objtool sections size' sub-sub-command. Because I think this discussion
> shows that it would be good to have a bit of visibility into the sanity of
> our sections setup, with 'objtool sections check' for example doing a
> sanity check on whether there's anything extra in the text section that
> shouldn't be there? Or so ...

What would be an example of something "extra"? A sanity check might fit
better alongside the other checks already being done by the main objtool
"subcommand" which gets run by the kernel build.

BTW, I actually removed subcommands a while ago when I overhauled
objtool's interface to make it easier to combine options. That said,
I'm not opposed to re-adding them if we can find a sane way to do so.

Here's the current interface:

Usage: objtool <actions> [<options>] file.o

Actions:
-h, --hacks[=<jump_label,noinstr,skylake>]
patch toolchain bugs/limitations
-i, --ibt validate and annotate IBT
-l, --sls validate straight-line-speculation mitigations
-m, --mcount annotate mcount/fentry calls for ftrace
-n, --noinstr validate noinstr rules
-o, --orc generate ORC metadata
-r, --retpoline validate and annotate retpoline usage
-s, --stackval validate frame pointer rules
-t, --static-call annotate static calls
-u, --uaccess validate uaccess rules for SMAP
--cfi annotate kernel control flow integrity (kCFI) function preambles
--dump[=<orc>] dump metadata
--prefix <n> generate prefix symbols
--rethunk validate and annotate rethunk usage
--unret validate entry unret placement

Options:
-v, --verbose verbose warnings
--backtrace unwind on error
--backup create .orig files before modification
--dry-run don't write modifications
--link object is a linked object
--mnop nop out mcount call sites
--module object is part of a kernel module
--no-unreachable skip 'unreachable instruction' warnings
--sec-address print section addresses in warnings
--stats print statistics


Note how all the actions can be easily combined in a single execution
instance.

If we re-added subcommands, most of the existing functionality would be
part of a single subcommand. It used to be called "check", but it's no
longer a read-only operation so that's misleading. I'll call it "run"
for now.

Right now my preference would be to leave the existing interface as-is,
and then graft optional subcommands on top. If no subcommand is
specified then it would default to the "run" subcommand. It's a little
funky, but it would work well for the common case, where ~99% of the
functionality lives. And it doesn't break existing setups and
backports.

For example:

# current interface (no changes)
objtool --mcount --orc --retpoline --uaccess vmlinux.o

# same, with optional explicit "run" subcommand
objtool run --mcount --orc --retpoline --uaccess vmlinux.o

# new "size" subcommand
obtool size [options] vmlinux.o.before vmlinux.o.after

--
Josh