Re: [PATCH v2 10/11] x86/alternatives: Simplify ALTERNATIVE_n()

From: Nikolay Borisov
Date: Tue Aug 15 2023 - 16:50:50 EST




On 14.08.23 г. 14:44 ч., Peter Zijlstra wrote:
Instead of making increasingly complicated ALTERNATIVE_n()
implementations, use a nested alternative expression.

The only difference between:

ALTERNATIVE_2(oldinst, newinst1, flag1, newinst2, flag2)

and

ALTERNATIVE(ALTERNATIVE(oldinst, newinst1, flag1),
newinst2, flag2)

is that the outer alternative can add additional padding when the
inner alternative is the shorter one, which then results in
alt_instr::instrlen being inconsistent.

However, this is easily remedied since the alt_instr entries will be
consecutive and it is trivial to compute the max(alt_instr::instrlen)
at runtime while patching.

Specifically, after this patch the ALTERNATIVE_2 macro, after CPP
expansion (and manual layout), looks like this:

.macro ALTERNATIVE_2 oldinstr, newinstr1, ft_flags1, newinstr2, ft_flags2
140:

140: \oldinstr ;
141: .skip -(((144f-143f)-(141b-140b)) > 0) * ((144f-143f)-(141b-140b)),0x90 ;
142: .pushsection .altinstructions,"a" ;
altinstr_entry 140b,143f,\ft_flags1,142b-140b,144f-143f ;
.popsection ; .pushsection .altinstr_replacement,"ax" ;
143: \newinstr1 ;
144: .popsection ; ;

141: .skip -(((144f-143f)-(141b-140b)) > 0) * ((144f-143f)-(141b-140b)),0x90 ;
142: .pushsection .altinstructions,"a" ;
altinstr_entry 140b,143f,\ft_flags2,142b-140b,144f-143f ;
.popsection ;
.pushsection .altinstr_replacement,"ax" ;
143: \newinstr2 ;
144: .popsection ;
.endm

The only label that is ambiguous is 140, however they all reference
the same spot, so that doesn't matter.

NOTE: obviously only @oldinstr may be an alternative; making @newinstr
an alternative would mean patching .altinstr_replacement which very
likely isn't what is intended, also the labels will be confused in
that case.


Reviewed-by: Nikolay Borisov <nik.borisov@xxxxxxxx>

Ps. I feel very "enlightened" knowing that GAS uses -1 to represent true ...