On Mon, Sep 17, 2018 at 8:32 AM Jason A. Donenfeld <Jason@xxxxxxxxx> wrote: > > On Mon, Sep 17, 2018 at 4:52 PM Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote: > > I think the module organization needs to change. It needs to be possible to have chacha20 built in but AES or whatever as a module. > > Okay, I'll do that for v5. > > > I might have agreed before Spectre :(. Unfortunately, unless we do some magic, I think the code would look something like: > > > > if (static_branch_likely(have_simd)) arch_chacha20(); > > > > ...where arch_chacha20 is a *pointer*. And that will generate a retpoline and run very, very slowly. (I just rewrote some of the x86 entry code to eliminate one retpoline. I got a 5% speedup on some tests according to the kbuild bot.) > > Actually, the way it works now benefits from the compilers inliner and > the branch predictor. I benchmarked this without any retpoline > slowdowns, and the branch predictor becomes correct pretty much all > the time. We can tinker with this after the initial merge, if you > really want, but avoiding function pointers and instead using ordinary > branches really winds up being quite fast. Indeed. What I'm saying is that you shouldn't refactor it this way because it will be slow. I agree it would be conceptually nice to be able to blacklist a chacha20_x86_64 module to disable the asm, but I think it would be very hard to get good performance. --Andy
