lkml.org 
[lkml]   [2020]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [GIT PULL] Crypto Fixes for 5.6
On Thu, Mar 12, 2020 at 4:57 AM Herbert Xu <herbert@gondor.apana.org.au> 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

\
 
 \ /
  Last update: 2020-03-12 17:41    [W:0.331 / U:0.340 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site