linux-kernel - Re: [PATCH v8 5/5] crypto: aspeed: add HACE crypto driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <450dee2f-63bf-51a7-730e-b780b594c1c5@linux.microsoft.com>
Date:   Wed, 27 Jul 2022 23:18:22 -0700
From:   Dhananjay Phadke <dphadke@...ux.microsoft.com>
To:     Neal Liu <neal_liu@...eedtech.com>,
        Corentin Labbe <clabbe.montjoie@...il.com>,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        Randy Dunlap <rdunlap@...radead.org>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S . Miller" <davem@...emloft.net>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Joel Stanley <joel@....id.au>,
        Andrew Jeffery <andrew@...id.au>,
        Johnny Huang <johnny_huang@...eedtech.com>
Cc:     "linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        BMC-SW <BMC-SW@...eedtech.com>
Subject: Re: [PATCH v8 5/5] crypto: aspeed: add HACE crypto driver

On 7/26/2022 10:31 PM, Neal Liu wrote:
>> Why are separate options required for hash and crypto algorithms, if hace is
>> only hw crypto on the SoCs?
>>
>> Looks like that's requiring unnecessary __weak register / unregister functions
>> [see below].
>>
>> Couldn't just two options CONFIG_CRYPTO_DEV_ASPEED and
>> CONFIG_CRYPTO_DEV_ASPEED_DEBUG be simpler to set for downstream
>> defconfigs?
> I would like to separate different algorithms by different options for more convenient for further use and debug.
> We also have RSA engine named ACRY, and would upstream once hash & crypto being accepted.
> So combined them into one option seems not a good choice for multiple hw crypto, do you agree?

Not sure what's the use case of just enabling crypto or hash separately
out of same HW engine and esp. when there's no alternative accel 
available, but that's fine.

If ARCY is different HW engine (interface) then having separate config
sounds logical.

Multiplying DEBUG configs seems unnecessary though. With dynamic debug
any of the dev_dbg could be turned on. Suggest using single one for
the module, if not drop it altogether. Following code is still not
covered by Kconfig, it in common code.

 > +#ifdef ASPEED_HACE_DEBUG
 > +#define HACE_DBG(d, fmt, ...)	\
 > +	dev_info((d)->dev, "%s() " fmt, __func__, ##__VA_ARGS__)
 > +#else
 > +#define HACE_DBG(d, fmt, ...)	\
 > +	dev_dbg((d)->dev, "%s() " fmt, __func__, ##__VA_ARGS__)
 > +#endif

Regards,
Dhananjay