Re: [PATCH 1/3] crypto: sm4 - create SM4 library based on sm4 generic code

From: Jason A. Donenfeld
Date: Tue Mar 01 2022 - 08:22:41 EST


Hi Tianjia,

On Tue, Mar 1, 2022 at 12:50 PM Tianjia Zhang
<tianjia.zhang@xxxxxxxxxxxxxxxxx> wrote:
>
> Hi Jason,
>
> On 3/1/22 6:34 PM, Jason A. Donenfeld wrote:
> >> lib/crypto/Kconfig | 3 +
> >> lib/crypto/Makefile | 3 +
> >> lib/crypto/sm4.c | 184 +++++++++++++++++++++++++++++++++++++++++++
> >
> > If this is only used by the crypto API, it does not belong in
> > lib/crypto. I understand you want fallback generic code for the SIMD
> > implementation, but we've generally done that in crypto/ when the use
> > case is only the crypto API. Can you move this to the right place?
>
> This is not only used by the crypto API, but also used for SIMD
> acceleration under the x86 and arm architectures, mainly for processing
> the remaining blocks after SIMD acceleration. In general, the
> performance of SIMD processing a single block is not as good as that of
> general software implementations.

Yes, and those accelerated implementations are part of the crypto API,
and are not used by anything except the crypto API. Hence this should
be in crypto/, just like everything else that is /only/ used for the
cryto API. lib/crypto/ is for in-kernel users of crypto via normal
code paths. sm4.c does not belong in lib/crypto/ and should be moved.

You additional export symbols of those SIMD implementations in
arch/crypto/, which is not correct either, since nothing in the tree
uses those symbols. Please remove those EXPORT_SYMBOL directives as
well. Those functions can be static, and do not need to be declared in
the .h file.

Thanks,
Jason