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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <HK0PR06MB3202719AA188BB743BE0A36C80969@HK0PR06MB3202.apcprd06.prod.outlook.com>
Date:   Thu, 28 Jul 2022 08:58:01 +0000
From:   Neal Liu <neal_liu@...eedtech.com>
To:     Dhananjay Phadke <dphadke@...ux.microsoft.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

Okay, I'll leverage your suggestion with different crypto algos options and 1 debug option for all modules.
Thanks !