Re: [PATCH 0/2] crypto: intel-fcs: Add crypto service driver for Intel SoCFPGA

[Dmitry Baryshkov]

On 15/09/2022 16:52, wen.ping.teh@xxxxxxxxx wrote:
> From: wen.ping.teh@xxxxxxxxx
>
> > From: Dmitry Baryshkov @ 2022-09-15 11:46 UTC (permalink / raw)
> > > From: wen.ping.teh@xxxxxxxxx
> > >
> > > > This patch introduces a crypto service driver for Intel SoCFPGA
> > > > family. The FPGA Crypto Service (FCS) includes a large set of security
> > > > features that are provided by the Secure Device Manager(SDM) in FPGA.
> > > > The driver provide IOCTL interface for user to call the crypto services
> > > > and send them to SDM's mailbox.
> > > >
> > > > Teh Wen Ping (2):
> > > >    crypto: intel-fcs: crypto service driver for Intel SoCFPGA family
> > > >    arm64: defconfig: add CRYPTO_DEV_INTEL_FCS
> > > Hi,
> > >
> > > I just found out that there was a previous attempt to upstream this driver
> > > 2 years ago. It was NACK because it did not implement crypto API. Please
> > > drop this review.
> > > https://www.mail-archive.com/linux-crypto@xxxxxxxxxxxxxxx/msg44701.html
> > >
> > > I will move this driver to drivers/misc.
> >
> > I think the proper solution would be to implement the existing API first
> > rather than adding a set of custom proprietary IOCTLs that nobody else
> > is going to use.
>
> Could you explain what are the existing API that you are referring?
> The FCS driver doesn't have API. Instead it uses IOCTLs to interact with user-space application to perform Intel SoCFPGA crypto features.

The FCS driver doesn't. But Linux does.

For the hw random generators we have the struct hwrng/devm_hwrng_register().

For AES, EC, etc. there are corresponding Crypto API. Based on your 
patches I couldn't guess if your hardware is more of a TPM or a generic 
crypto "accelerator". However Linux has support for both kinds of 
hardware. Most likely the right interface is already there (or almost 
there). In 90% of cases 'a custom bunch of IOCTLs' is not a correct one.

--
With best wishes
Dmitry