On 21/01/2020 11:34, Mark Rutland wrote: > Hi Szabolcs, > > Answers from a linux dev perspective below.
thanks. > On Mon, Jan 20, 2020 at 10:53:33AM +0000, Szabolcs Nagy wrote: >> i have to ask some linux developers which way they prefer: >> >> e.g. -fpatchable-function-entry=3,1 is >> >> .section __patchable_function_entries >> .8byte .Lpatch >> .text >> .Lpatch: >> nop >> func: >> nop >> nop >> ... >> >> with bti the code will be emitted as: >> >> .Lpatch: >> nop >> func: >> bti c >> nop >> nop >> ... > > That looks good to me. > >> but e.g. -fpatchable-function-entry=2,0 has two reasonable >> approaches with bti: >> >> (a) >> >> func: >> .Lpatch: >> bti c >> nop >> nop >> ... >> >> (b) >> >> func: >> bti c >> .Lpatch: >> nop >> nop >> ... > > I had assumed (b); that means that .Lpatch consistently points to the > first NOP. To my mental model, that seems more consistent than (a). > > However, I can work with either so long as it's consistent. ... > As above, my weak preference is (b), but I can work with either. I just > need the behaviour to be consistent. > > Was there a rationale for LLVM choosing (a) rather than (b), e.g. was > that also ease of implementation? If there isn't a rationale otherwise, > and if LLVM could also do (b), that would be nice from my PoV. > > How big is "a bit more changes" for GCC? ".Lpatch: nop; nop" is generated by generic code now which a backend can override, but then i have to copy that logic (~30 lines) to add something after the .Lpatch, or i have to change generic code to have a new hook after the .Lpatch:. to provide more argument for (b) and expand on consistency: right now bti is not emitted if a function is not called indirectly (compiler knows this for some local functions) or bti can be turned off by a target attribute per function. so a mix of bti and non-bti functions are possible, so the user patching code has to deal with this (e.g. read the instructions before patching etc) in the M>0, M!=N case bti is emitted in the middle of a patch area (e.g. 3,1 case above), but some functions will not have bti. i can make it so that if patchable function entry is requested then bti is always added, irrespective of any other option or attribute, but that seems to be a big hammer, without such big hammer solution users will have a difficult time to deal with the additional bti. if the patch area is entirely before the function label, then no changes are needed (M=N), if patch area is after the function label (M=0) then using fix (b) would allow simple user code. in other cases user code will be complicated no matter what. so if linux uses M=0 then i think (b) is preferable. hope this helps.
