On Thu, Mar 12, 2020 at 4:57 AM Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote: > > This push fixes a build problem with x86/curve25519. Pulled. I do have a comment, though: this fix matches the existing pattern of checking for assembler support, but that existing pattern is absolutely horrible. Would some enterprising individual please look at making the CONFIG_AS_xyz flags use the _real_ config subsystem rather than ad-hoc Makefile rules? IOW, instead of having adx_instr := $(call as-instr,adox %r10$(comma)%r10,-DCONFIG_AS_ADX=1) .. adx_supported := $(call as-instr,adox %r10$(comma)%r10,yes,no) in the makefiles, and silently changing how the Kconfig variables work depending on those flags, make that DCONFIG_AS_ADX be a real config variable: config AS_ADX def_bool $(success,$(srctree)/scripts/as-instr.sh "adox %r10,%r10") or something like that? And then we can make that CRYPTO_CURVE25519_X86 config variable simply have a depends on AS_ADX in it, and the Kconfig system just takes care of these dependencies on its own. Anyway, the crypto change isn't _wrong_, but it does point out an ugly little horror in how the crypto layer silently basically changes the configuration depending on other things. For an example of why this is problematic: it means that if somebody sends you their config file, the actual configuration you get may be *completely* different from what they actually had, depending on tools. Added Masahiro to the cc, since he's used to the 'def_bool' model, and also is familiar with our existing 'as-instr' Makefile macro. So this is basically me throwing out a "I wish somebody would look at this". Not meant as a criticism of the commit in question. Linus