Hi Linus, On Fri, Mar 13, 2020 at 1:41 AM Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > 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. Thanks for the heads-up. In fact, as-instr is already used in Kconfig. arch/arm64/Kconfig: line 1396 arm / arm64 are simple cases because 32, 64-bit is separated by directory. There is one thing we need to be careful about. The x86 GCC is usually biarch. So, when evaluating 64-bit assembly code with a default 32-bit compiler, -m64 must be passed. I will keep this conversion in my mind. Thanks. > 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 -- Best Regards Masahiro Yamada
