Re: [PATCH v2 2/3] accel/amdpk: add driver for AMD PKI accelerator

[Lukas Wunner <lukas@wunner.de>]

On Fri, Apr 11, 2025 at 10:21:03AM +0530, Gupta, Nipun wrote:
> On 10-04-2025 13:06, Krzysztof Kozlowski wrote:
> > On Wed, Apr 09, 2025 at 11:00:32PM GMT, Nipun Gupta wrote:
> > > The AMD PKI accelerator driver provides a accel interface to interact
> > > with the device for offloading and accelerating asymmetric crypto
> > > operations.
> > > 
> > 
> > For me this is clearly a crypto driver and you are supposed to:
> > 1. Cc crypto maintaners,
> > 2. Put it actually into crypto and use crypto API.
> 
> added crypto maintainers for comments.
> IMO, as accel framework is designed to support any type of compute
> accelerators, the PKI crypto accelerator in accel framework is not
> completely out of place here, as also suggested at:
> https://lore.kernel.org/all/2025031352-gyration-deceit-5563@gregkh/

To be fair, Greg did suggest drivers/crypto/ as an alternative... :)

  "Great, then why isn't this in drivers/accel/ or drivers/crypto/ ?"
  https://lore.kernel.org/r/2025031236-siamese-graffiti-5b98@gregkh/

There are already six drivers for crypto accelerators in drivers/crypto/,
so that would seem to be a natural fit for your driver:

  aspeed/aspeed-acry.c
  caam/caampkc.c
  ccp/ccp-crypto-rsa.c                 <-- from AMD no less!
  hisilicon/hpre/hpre_crypto.c
  intel/qat/qat_common/qat_asym_algs.c
  starfive/jh7110-rsa.c

You can find these in the tree with:

  git grep 'cra_name = "rsa"'

So far there are only drivers to accelerate RSA encryption/decryption.
The kernel supports a single padding scheme, PKCS1, which is implemented
by crypto/rsa-pkcs1pad.c.  There is no support yet for OAEP.

So the padding of the hash (which is cheap) happens in software and then
crypto/rsa-pkcs1pad.c performs an RSA encrypt/decrypt operation which is
either performed in software by crypto/rsa.c, or in hardware if a crypto
accelerator is present.  Drivers for crypto accelerators register the
"rsa" algorithm with a higher cra_priority than the software
implementation, hence are generally preferred.

One benefit that you get from implementing a proper akcipher_alg in your
driver is that virtual machines may take advantage of the hardware
accelerator through the virtio support implemented by:
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c

Note that the crypto subsystem currently does not support hardware
acceleration of signature generation/verification (crypto_sig),
but only encryption/decryption (crypto_akcipher).  One reason is
that signature generation/verification is generally a synchronous
operation and doesn't benefit as much from hardware acceleration
due to the overhead of interacting with the hardware.

So there's no support e.g. for generating or verifying ECDSA signatures
in hardware.  I think that would only really make sense if private keys
are kept in hardware and cannot be retrieved.  So the use case wouldn't
be acceleration, but security of private keys.

That said, for RSA specifically, signature generation/verification does
involve an encrypt/decrypt operation internally.  The padding is once
again done in software (by crypto/rsassa-pkcs1.c -- no PSS support yet).
But the actual encrypt/decrypt operation will be performed in
hardware if a crypto accelerator is present.

The user space interface Herbert referred to is a set of system calls
which are usable e.g. via the keyutils library and command line utility:
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/keyutils.git/

HTH,

Lukas