Re: [PATCH] crypto: qce - Replace with stub driver

Re: [PATCH] crypto: qce - Replace with stub driver

[Demi Marie Obenour]

[-- Attachment #1.1: Type: text/plain, Size: 4853 bytes --]

On 7/31/26 01:08, Eric Biggers wrote:
> None of the algorithms the QCE driver registers with the crypto API are
> even close to being useful.  They're massively outperformed by the
> CPU-based crypto (even actually using far more CPU cycles than the
> CPU-based crypto, due to the massive driver overhead), don't provide any
> security benefits over the CPU-based crypto, and have various other
> issues.  They're also unrelated to the Inline Crypto Engine which works
> far better and is what is actually being used these days.
> 
> Due to the major performance problems people experienced when
> accidentally using this driver, the algorithms were already demoted to
> below the priority of ARMv8 CE in the crypto API several years ago.
> Regardless of priority, these algorithms also aren't compatible with the
> most common uses of in-kernel crypto; for example, they don't support
> dm-crypt because they allocate memory, and they don't support IPsec
> since they take a mutex which isn't compatible with softirq context.
> 
> It's time to move on and just drop this obsolete functionality entirely.
> 
> However, the driver does serve one purpose which is to bind to the
> devicetree node and allow the interconnects to scale down (though even
> that part is broken, as it holds static interconnect bandwidth votes).
> 
> Thus, clear out the crypto functionality, leaving just a stub driver
> that drops the interconnection bandwidth votes to zero.
> 
> Tested on the SM8650 HDK.
> 
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>

Some thoughts:

1. Should this be put under a different subdirectory?  This
   is now a power management driver, not a crypto driver.
   IMO it should just fall under ARCH_QCOM, without a separate
   Kconfig entry (or at least a "default y" one).

2. Should it be re-added to the defconfigs?  Most distros
   will want to turn it on, as it saves power and is minuscule.

3. Will this the smallest driver in the whole kernel?
> ---
>  drivers/crypto/Kconfig        | 117 +----
>  drivers/crypto/qce/Makefile   |   7 -
>  drivers/crypto/qce/aead.c     | 787 ----------------------------------
>  drivers/crypto/qce/aead.h     |  56 ---
>  drivers/crypto/qce/cipher.h   |  56 ---
>  drivers/crypto/qce/common.c   | 564 ------------------------
>  drivers/crypto/qce/common.h   |  94 ----
>  drivers/crypto/qce/core.c     | 271 ------------
>  drivers/crypto/qce/core.h     |  64 ---
>  drivers/crypto/qce/dma.c      | 135 ------
>  drivers/crypto/qce/dma.h      |  47 --
>  drivers/crypto/qce/qcrypto.c  |  43 ++
>  drivers/crypto/qce/regs-v5.h  | 322 --------------
>  drivers/crypto/qce/sha.c      | 498 ---------------------
>  drivers/crypto/qce/sha.h      |  71 ---
>  drivers/crypto/qce/skcipher.c | 434 -------------------
>  16 files changed, 50 insertions(+), 3516 deletions(-)
>  delete mode 100644 drivers/crypto/qce/aead.c
>  delete mode 100644 drivers/crypto/qce/aead.h
>  delete mode 100644 drivers/crypto/qce/cipher.h
>  delete mode 100644 drivers/crypto/qce/common.c
>  delete mode 100644 drivers/crypto/qce/common.h
>  delete mode 100644 drivers/crypto/qce/core.c
>  delete mode 100644 drivers/crypto/qce/core.h
>  delete mode 100644 drivers/crypto/qce/dma.c
>  delete mode 100644 drivers/crypto/qce/dma.h
>  create mode 100644 drivers/crypto/qce/qcrypto.c
>  delete mode 100644 drivers/crypto/qce/regs-v5.h
>  delete mode 100644 drivers/crypto/qce/sha.c
>  delete mode 100644 drivers/crypto/qce/sha.h
>  delete mode 100644 drivers/crypto/qce/skcipher.c
> 
> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index 0189dfdcbbe1..03113ad3467c 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -527,119 +527,16 @@ source "drivers/crypto/marvell/Kconfig"
>  source "drivers/crypto/intel/Kconfig"
>  
>  config CRYPTO_DEV_QCE
> -	tristate "Qualcomm crypto engine accelerator"
> -	depends on (BROKEN && ARCH_QCOM) || COMPILE_TEST
> +	tristate "Qualcomm crypto engine"
> +	depends on ARCH_QCOM || COMPILE_TEST
>  	depends on HAS_IOMEM
>  	help
> -	  This driver supports Qualcomm crypto engine accelerator
> -	  hardware. To compile this driver as a module, choose M here. The
> -	  module will be called qcrypto.
> -
> -	  This driver does not have exclusive access to the
> -	  hardware, causing races with the secure world.  It
> -	  is also slower than the CPU.
> +	  This driver provides a stub for the Qualcomm crypto engine. It exists
> +	  only to bind to the devicetree node and allow interconnects to scale
> +	  down, saving power. It doesn't expose any cryptographic functionality.

Should this mention why the cryptographic functionality
available in the hardware has been proven unsuitable for use?
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] crypto: qce - Replace with stub driver

[Bartosz Golaszewski]

On Fri, 31 Jul 2026 07:08:38 +0200, Eric Biggers <ebiggers@kernel.org> said:
> None of the algorithms the QCE driver registers with the crypto API are
> even close to being useful.  They're massively outperformed by the
> CPU-based crypto (even actually using far more CPU cycles than the
> CPU-based crypto, due to the massive driver overhead), don't provide any
> security benefits over the CPU-based crypto, and have various other
> issues.  They're also unrelated to the Inline Crypto Engine which works
> far better and is what is actually being used these days.
>
> Due to the major performance problems people experienced when
> accidentally using this driver, the algorithms were already demoted to
> below the priority of ARMv8 CE in the crypto API several years ago.
> Regardless of priority, these algorithms also aren't compatible with the
> most common uses of in-kernel crypto; for example, they don't support
> dm-crypt because they allocate memory, and they don't support IPsec
> since they take a mutex which isn't compatible with softirq context.
>
> It's time to move on and just drop this obsolete functionality entirely.
>
> However, the driver does serve one purpose which is to bind to the
> devicetree node and allow the interconnects to scale down (though even
> that part is broken, as it holds static interconnect bandwidth votes).
>
> Thus, clear out the crypto functionality, leaving just a stub driver
> that drops the interconnection bandwidth votes to zero.
>
> Tested on the SM8650 HDK.
>
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---

I'll be on vacation for most of August so my lack of responses won't mean I
no longer care, only that I'm disconnected. Let me just leave this here for
completness:

Nacked-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

I don't understand why the push to fast track the removal of this driver now,
when there's clearly an effort to get it fixed from the vendor itself. I've
also never seen a driver for an IP on a supported platform being removed from
mainline linux.

Thanks,
Bartosz