Re: [GIT PULL] Crypto Fixes for 5.6

[Masahiro Yamada]

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