[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d13e2693-125e-4ab9-983a-f83b09e8acb4@oracle.com>
Date: Mon, 17 Nov 2025 08:50:45 +0100
From: Alexandre Chartre <alexandre.chartre@...cle.com>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: alexandre.chartre@...cle.com, linux-kernel@...r.kernel.org,
mingo@...nel.org, peterz@...radead.org
Subject: Re: [PATCH v4 00/28] objtool: Function validation tracing
On 11/14/25 22:34, Josh Poimboeuf wrote:
> On Fri, Nov 14, 2025 at 10:56:48AM +0100, Alexandre Chartre wrote:
>>> Other than '!', the meaning of the flags isn't intuitive. Maybe it
>>> should just show the source code names:
>>>
>>> ALT_NOT(X86_FEATURE_FOO)
>>>
>>> ALT_DIRECT_CALL(X86_FEATURE_BAR)
>>>
>>> ALT_UNKNOWN_FLAG(X86_FEATURE_BAZ)
>>>
>>
>> I think '?' is meaningful too, but I wasn't sure about '+'.
>>
>> I am using single characters to keep the alternative name short. It can already
>> be fairly long because of the feature name (like "X86_FEATURE_SPEC_STORE_BYPASS_DISABLE")
>>
>> Also I am assuming that flags can be combined (although that's not currently
>> the case) so that would be more difficult with full ALT_* names and the
>> result would be much longer.
>
> Ok.
>
>>>> - If an alternative is a jump table then "JUMP" is used as the feature
>>>> name.
>>>
>>> Hm, it's a bit confusing to label a jump label as an "alternative" as
>>> those are two distinct things (though I'm aware that objtool conflates
>>> the two).
>>>
>>>> - If an alternative is an exception table then "EXCEPTION" is used as the
>>>> feature name.
>>>
>>> Ditto.
>>>
>>
>> Yes, the wording is not good, I use it just because objtool handles jump
>> labels and exception tables as alternative. I will reword to something
>> better.
>
> I meant not only the wording here, but also the "<alternative.def9>"
> labels shown in the disassembly.
>
Right, I will change it <alternative.xxx>, <jump_label.xxx>, <ex_table.xxx>.
>>>> Disassembly can show default alternative jumping to .altinstr_aux
>>>> -----------------------------------------------------------------
>>>> Disassembly can show a default alternative jumping to .altinstr_aux. This
>>>> happens when the _static_cpu_has() function is used. Its default code
>>>> jumps to .altinstr_aux where a test sequence is executed (test; jnz; jmp).
>>>>
>>>> At runtime, this sequence is not used because the _static_cpu_has()
>>>> an alternative with the X86_FEATURE_ALWAYS feature.
>>>>
>>>>
>>>> debc: perf_get_x86_pmu_capability+0xc jmpq 0xdec1 <.altinstr_aux+0xfc> | NOP5 (X86_FEATURE_HYBRID_CPU) | jmpq 0x61a <perf_get_x86_pmu_capability+0x37> (X86_FEATURE_ALWAYS) # <alternative.debc>
>>>
>>> I'm finding this one-line format considerably more difficult to parse
>>> than the slightly longer two-line form:
>>>
>>> debc: perf_get_x86_pmu_capability+0xc <alternative.debc> | X86_FEATURE_HYBRID_CPU | X86_FEATURE_ALWAYS
>>> debc: perf_get_x86_pmu_capability+0xc jmpq 0xdec1 <.altinstr_aux+0xfc> | NOP5 | jmpq 0x61a <perf_get_x86_pmu_capability+0x37>
>>
>>
>> Another option could be:
>>
>> debc: perf_get_x86_pmu_capability+0xc jmpq 0xdec1 <.altinstr_aux+0xfc> (<alternative.debc>) |
>> NOP5 (X86_FEATURE_HYBRID_CPU) |
>> jmpq 0x61a <perf_get_x86_pmu_capability+0x37> (X86_FEATURE_ALWAYS)
>>
>> I think I will use this option when displaying alternative one after the other,
>> and your suggestion when displaying side-by-side, and add an option to select
>> the display.
>
> Hm, but how would that "one after the other" display look for an
> alternative with multiple instructions? A "compact vs wide" option is
> ok, but within each of those I think it's helpful to use a consistent
> format regardless of whether the alternative has one or multiple
> instructions.
>
David raises the issue that a side-by-side display requires a large window.
The compact display could be like this:
Alternative with single instruction:
bb8: do_one_initcall+0x1a8 <alternative.bb8>
= callq *0x0(%rip) # 0xbbe <pv_ops+0xf8> (if default)
= sti (if !X86_FEATURE_XENPV)
= callq BUG_func (if +X86_FEATURE_ALWAYS)
Alternative with multiple instructions:
82e7: __switch_to_asm+0x27 <alternative.82e7>
= DEFAULT
82e7: __switch_to_asm+0x27 | jmp 0x8312 <__switch_to_asm+0x52>
|
= !X86_FEATURE_ALWAYS
82e7: __switch_to_asm+0x27 | NOP1
82e8: __switch_to_asm+0x28 | NOP1
82e9: __switch_to_asm+0x29 | callq 0x82ef <__switch_to_asm+0x2f>
82ee: __switch_to_asm+0x2e | int3
82ef: __switch_to_asm+0x2f | add $0x8,%rsp
82f3: __switch_to_asm+0x33 | lfence
|
= X86_FEATURE_RSB_CTXSW
82e7: __switch_to_asm+0x27 | mov $0x10,%r12
82ee: __switch_to_asm+0x2e | callq 0x82f4 <__switch_to_asm+0x34>
82f3: __switch_to_asm+0x33 | int3
82f4: __switch_to_asm+0x34 | callq 0x82fa <__switch_to_asm+0x3a>
82f9: __switch_to_asm+0x39 | int3
82fa: __switch_to_asm+0x3a | add $0x10,%rsp
82fe: __switch_to_asm+0x3e | dec %r12
8301: __switch_to_asm+0x41 | jne 0x82ee <__switch_to_asm+0x2e>
8303: __switch_to_asm+0x43 | lfence
8306: __switch_to_asm+0x46 | movq $0xffffffffffffffff,%gs:0x0(%rip) # 0x20b <__x86_call_depth>
|
>>> Also, I wonder if we can make NOP5 lowercase (nop5), since it really is
>>> just an instruction, not something special like a feature.
>>
>> This indicates that this is a pseudo instruction, NOP5 is actually nopl 0x00(%eax,%eax,1).
>> Even NOP1 can be a simple nop but also xchg %rax,%rax.
>
> But it still represents a single instruction... I view it as a readable
> shorthand rather than a pseudo instruction.
>
Ok. I will change to nop<n>.
>>>> 82e7: __switch_to_asm+0x27 | <alternative.82e7> | !X86_FEATURE_ALWAYS | X86_FEATURE_RSB_CTXSW
>>>
>>> Are the alternatives swapped? I believe this comes from the following
>>> code, so the !X86_FEATURE_ALWAYS column should be last?
>>>
>>> .macro FILL_RETURN_BUFFER reg:req nr:req ftr:req ftr2=ALT_NOT(X86_FEATURE_ALWAYS)
>>> ALTERNATIVE_2 "jmp .Lskip_rsb_\@", \
>>> __stringify(__FILL_RETURN_BUFFER(\reg,\nr)), \ftr, \
>>> __stringify(nop;nop;__FILL_ONE_RETURN), \ftr2
>>>
>>> .Lskip_rsb_\@:
>>> .endm
>>
>> I will check but I process/print alternative in the order provided by
>> objtool (in struct alternative)
>
> It's quite possible objtool has them in reverse order. The order
> shouldn't matter for objtool validate_branch(), but definitely matters
> here.
>
That's probably the issue. I think objtool reads alternatives in the right
order but it adds each of them at the beginning of the same list. So they
end up in reverse order in the list.
Thanks,
alex.
Powered by blists - more mailing lists