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

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

[Krzysztof Kozlowski]

On 31/07/2026 07: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

The amount of patches you send towards removal of QCE is really
stunning. Or rather worrying. This is like third approach or so.

Your previous approaches received valid feedback, including even fixes
and committment of new maintainer.

But you even complained that it does receive fixes! [1]

We removed the driver temporarily from typical configurations, so no one
will be affected, by whatever is found now and not yet fixed. Still not
enough! For you this was reason to remove the driver (AGAIN!) [2]

This is beyond comprehension and very unpleasant, because you actively
work against the community with this approach.

NAK

Nacked-by: Krzysztof Kozlowski <krzk@kernel.org>


[1] https://lore.kernel.org/all/20260728152241.GA4281@quark/
[2] https://lore.kernel.org/all/20260724050645.223799-1-ebiggers@kernel.org/

> 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.

Best regards,
Krzysztof

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

[Eric Biggers]

On Fri, Jul 31, 2026 at 11:06:28AM +0200, Krzysztof Kozlowski wrote:
> On 31/07/2026 07: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
> 
> The amount of patches you send towards removal of QCE is really
> stunning. Or rather worrying. This is like third approach or so.

Note that the crypto maintainer has already accepted a patch from
another developer to mark this driver as BROKEN.

> This is beyond comprehension and very unpleasant, because you actively
> work against the community with this approach.

"The community" is more than the hardware vendor.

I think the results of independent testing of this driver are incredibly
clear.  The pending fixes do not fix the performance either, but
actually make it worse.

I have been asking for the inclusion of this driver to be justified for
many years.  As far as I've ever been able to tell it's just a "checkbox
feature", which isn't something that really belongs upstream.

I care about the technical merit of the kernel's cryptographic algorithm
implementations.

- Eric

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

[Demi Marie Obenour]

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

On 7/31/26 05:06, Krzysztof Kozlowski wrote:
> On 31/07/2026 07: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
> 
> The amount of patches you send towards removal of QCE is really
> stunning. Or rather worrying. This is like third approach or so.
> 
> Your previous approaches received valid feedback, including even fixes
> and committment of new maintainer.
> 
> But you even complained that it does receive fixes! [1]
> 
> We removed the driver temporarily from typical configurations, so no one
> will be affected, by whatever is found now and not yet fixed. Still not
> enough! For you this was reason to remove the driver (AGAIN!) [2]

A stub driver needs to be added for power management reasons.  It turns
out that if there is no driver, power consumption is very high.

> This is beyond comprehension and very unpleasant, because you actively
> work against the community with this approach.

I trust that Bartosz can fix the driver with enough work.  However,
even if the QCE driver had never had a single bug, it would still not
be worth using.  Its performance is so poor that one should always
use CPU-based crypto instead.  In the default configuration, that is
in fact what happens.

The QCE's algorithm implementations only provide a way people can
misconfigure their systems and ruin their performance.  There are
debug options that also severely harm performance, but they have
legitimate uses during development.  The current QCE driver doesn't.

I have no problem with a driver for the QCE that does something actually
useful.  While there are disagreements about whether restricted media
processing is a feature or an anti-feature, my view is that it is
better for it to be implemented upstream than in an out-of-tree driver.
However, the driver as it currently exists does not implement this.

I expect that such a driver would not use the crypto API at all.
Instead, I suspect it would use dmabufs for source and destination
buffers and integrate with the secure world firmware in some way.
That means that the driver doesn't belong under drivers/crypto.

These are the reasons I submitted a patch to mark the driver as BROKEN,
which Herbert Xu has since accepted.

> NAK
> 
> Nacked-by: Krzysztof Kozlowski <krzk@kernel.org>
> 
> 
> [1] https://lore.kernel.org/all/20260728152241.GA4281@quark/
> [2] https://lore.kernel.org/all/20260724050645.223799-1-ebiggers@kernel.org/
> 
>> 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.
> 
> Best regards,
> Krzysztof


-- 
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

[Krzysztof Kozlowski]

On 31/07/2026 20:31, Demi Marie Obenour wrote:
> On 7/31/26 05:06, Krzysztof Kozlowski wrote:
>> On 31/07/2026 07: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
>>
>> The amount of patches you send towards removal of QCE is really
>> stunning. Or rather worrying. This is like third approach or so.
>>
>> Your previous approaches received valid feedback, including even fixes
>> and committment of new maintainer.
>>
>> But you even complained that it does receive fixes! [1]
>>
>> We removed the driver temporarily from typical configurations, so no one
>> will be affected, by whatever is found now and not yet fixed. Still not
>> enough! For you this was reason to remove the driver (AGAIN!) [2]
> 
> A stub driver needs to be added for power management reasons.  It turns
> out that if there is no driver, power consumption is very high.
> 
>> This is beyond comprehension and very unpleasant, because you actively
>> work against the community with this approach.
> 
> I trust that Bartosz can fix the driver with enough work.  However,
> even if the QCE driver had never had a single bug, it would still not
> be worth using.  Its performance is so poor that one should always
> use CPU-based crypto instead.  In the default configuration, that is
> in fact what happens.
> 
> The QCE's algorithm implementations only provide a way people can
> misconfigure their systems and ruin their performance.  There are
> debug options that also severely harm performance, but they have
> legitimate uses during development.  The current QCE driver doesn't.
> 
> I have no problem with a driver for the QCE that does something actually
> useful.  While there are disagreements about whether restricted media
> processing is a feature or an anti-feature, my view is that it is
> better for it to be implemented upstream than in an out-of-tree driver.
> However, the driver as it currently exists does not implement this.
> 
> I expect that such a driver would not use the crypto API at all.
> Instead, I suspect it would use dmabufs for source and destination
> buffers and integrate with the secure world firmware in some way.
> That means that the driver doesn't belong under drivers/crypto.
> 
> These are the reasons I submitted a patch to mark the driver as BROKEN,
> which Herbert Xu has since accepted.

And Bartosz and other people committed to work on this by improving,
fixing and in the long term providing you with the actual important user
of this. All this was already said.

And then after having all these discussions Eric sends AGAIN patch to
remove the driver. How many times this will have to be discussed the
same way? If we now reach agreement the driver stays, next month again
there will be a patch to remove it? And then one more month again?

The driver is marked as BROKEN, thus absolutely NO ONE is affected by
any issues the driver has.

It's some personal vendetta to keep coming after that - unimportant now
- driver.

Best regards,
Krzysztof

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

[Eric Biggers]

On Sat, Aug 01, 2026 at 11:18:46AM +0200, Krzysztof Kozlowski wrote:
> On 31/07/2026 20:31, Demi Marie Obenour wrote:
> > On 7/31/26 05:06, Krzysztof Kozlowski wrote:
> >> On 31/07/2026 07: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
> >>
> >> The amount of patches you send towards removal of QCE is really
> >> stunning. Or rather worrying. This is like third approach or so.
> >>
> >> Your previous approaches received valid feedback, including even fixes
> >> and committment of new maintainer.
> >>
> >> But you even complained that it does receive fixes! [1]
> >>
> >> We removed the driver temporarily from typical configurations, so no one
> >> will be affected, by whatever is found now and not yet fixed. Still not
> >> enough! For you this was reason to remove the driver (AGAIN!) [2]
> > 
> > A stub driver needs to be added for power management reasons.  It turns
> > out that if there is no driver, power consumption is very high.
> > 
> >> This is beyond comprehension and very unpleasant, because you actively
> >> work against the community with this approach.
> > 
> > I trust that Bartosz can fix the driver with enough work.  However,
> > even if the QCE driver had never had a single bug, it would still not
> > be worth using.  Its performance is so poor that one should always
> > use CPU-based crypto instead.  In the default configuration, that is
> > in fact what happens.
> > 
> > The QCE's algorithm implementations only provide a way people can
> > misconfigure their systems and ruin their performance.  There are
> > debug options that also severely harm performance, but they have
> > legitimate uses during development.  The current QCE driver doesn't.
> > 
> > I have no problem with a driver for the QCE that does something actually
> > useful.  While there are disagreements about whether restricted media
> > processing is a feature or an anti-feature, my view is that it is
> > better for it to be implemented upstream than in an out-of-tree driver.
> > However, the driver as it currently exists does not implement this.
> > 
> > I expect that such a driver would not use the crypto API at all.
> > Instead, I suspect it would use dmabufs for source and destination
> > buffers and integrate with the secure world firmware in some way.
> > That means that the driver doesn't belong under drivers/crypto.
> > 
> > These are the reasons I submitted a patch to mark the driver as BROKEN,
> > which Herbert Xu has since accepted.
> 
> And Bartosz and other people committed to work on this by improving,
> fixing and in the long term providing you with the actual important user
> of this. All this was already said.
> 
> And then after having all these discussions Eric sends AGAIN patch to
> remove the driver. How many times this will have to be discussed the
> same way? If we now reach agreement the driver stays, next month again
> there will be a patch to remove it? And then one more month again?
> 
> The driver is marked as BROKEN, thus absolutely NO ONE is affected by
> any issues the driver has.
> 
> It's some personal vendetta to keep coming after that - unimportant now
> - driver.

Well, there was also feedback that BROKEN drivers should not stay in the
tree and instead just be removed.  So I was considering that feedback
too, as well as the results of additional testing and review of this
driver, including evidence that the driver performs even worse than
thought and is actually getting even worse; the performance seems
irredeemable.  As for proposing the stub driver, that is addressing the
power management issue that was mentioned on the other thread.

Anyway, it sounds like you support it being BROKEN.  Okay, but again, as
Greg mentioned
(https://lore.kernel.org/linux-crypto/2026071312-uncover-refining-8cac@gregkh/)
it is awkward to have BROKEN stuff in the tree since it cannot even be
built.  I'm not sure we can have it both ways!

- Eric

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

[Krzysztof Kozlowski]

On 01/08/2026 18:29, Eric Biggers wrote:
> On Sat, Aug 01, 2026 at 11:18:46AM +0200, Krzysztof Kozlowski wrote:
>> On 31/07/2026 20:31, Demi Marie Obenour wrote:
>>> On 7/31/26 05:06, Krzysztof Kozlowski wrote:
>>>> On 31/07/2026 07: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
>>>>
>>>> The amount of patches you send towards removal of QCE is really
>>>> stunning. Or rather worrying. This is like third approach or so.
>>>>
>>>> Your previous approaches received valid feedback, including even fixes
>>>> and committment of new maintainer.
>>>>
>>>> But you even complained that it does receive fixes! [1]
>>>>
>>>> We removed the driver temporarily from typical configurations, so no one
>>>> will be affected, by whatever is found now and not yet fixed. Still not
>>>> enough! For you this was reason to remove the driver (AGAIN!) [2]
>>>
>>> A stub driver needs to be added for power management reasons.  It turns
>>> out that if there is no driver, power consumption is very high.
>>>
>>>> This is beyond comprehension and very unpleasant, because you actively
>>>> work against the community with this approach.
>>>
>>> I trust that Bartosz can fix the driver with enough work.  However,
>>> even if the QCE driver had never had a single bug, it would still not
>>> be worth using.  Its performance is so poor that one should always
>>> use CPU-based crypto instead.  In the default configuration, that is
>>> in fact what happens.
>>>
>>> The QCE's algorithm implementations only provide a way people can
>>> misconfigure their systems and ruin their performance.  There are
>>> debug options that also severely harm performance, but they have
>>> legitimate uses during development.  The current QCE driver doesn't.
>>>
>>> I have no problem with a driver for the QCE that does something actually
>>> useful.  While there are disagreements about whether restricted media
>>> processing is a feature or an anti-feature, my view is that it is
>>> better for it to be implemented upstream than in an out-of-tree driver.
>>> However, the driver as it currently exists does not implement this.
>>>
>>> I expect that such a driver would not use the crypto API at all.
>>> Instead, I suspect it would use dmabufs for source and destination
>>> buffers and integrate with the secure world firmware in some way.
>>> That means that the driver doesn't belong under drivers/crypto.
>>>
>>> These are the reasons I submitted a patch to mark the driver as BROKEN,
>>> which Herbert Xu has since accepted.
>>
>> And Bartosz and other people committed to work on this by improving,
>> fixing and in the long term providing you with the actual important user
>> of this. All this was already said.
>>
>> And then after having all these discussions Eric sends AGAIN patch to
>> remove the driver. How many times this will have to be discussed the
>> same way? If we now reach agreement the driver stays, next month again
>> there will be a patch to remove it? And then one more month again?
>>
>> The driver is marked as BROKEN, thus absolutely NO ONE is affected by
>> any issues the driver has.
>>
>> It's some personal vendetta to keep coming after that - unimportant now
>> - driver.
> 
> Well, there was also feedback that BROKEN drivers should not stay in the
> tree and instead just be removed.  So I was considering that feedback
> too, as well as the results of additional testing and review of this
> driver, including evidence that the driver performs even worse than
> thought and is actually getting even worse; the performance seems
> irredeemable.  As for proposing the stub driver, that is addressing the
> power management issue that was mentioned on the other thread.
> 
> Anyway, it sounds like you support it being BROKEN.  Okay, but again, as
> Greg mentioned
> (https://lore.kernel.org/linux-crypto/2026071312-uncover-refining-8cac@gregkh/)
> it is awkward to have BROKEN stuff in the tree since it cannot even be
> built.  I'm not sure we can have it both ways!

Which was I think pointed already a few times that it will not stay
BROKEN but will get fixed and get proper useful use cases.

Best regards,
Krzysztof

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

[Eric Biggers]

On Sat, Aug 01, 2026 at 06:35:56PM +0200, Krzysztof Kozlowski wrote:
> On 01/08/2026 18:29, Eric Biggers wrote:
> > On Sat, Aug 01, 2026 at 11:18:46AM +0200, Krzysztof Kozlowski wrote:
> >> On 31/07/2026 20:31, Demi Marie Obenour wrote:
> >>> On 7/31/26 05:06, Krzysztof Kozlowski wrote:
> >>>> On 31/07/2026 07: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
> >>>>
> >>>> The amount of patches you send towards removal of QCE is really
> >>>> stunning. Or rather worrying. This is like third approach or so.
> >>>>
> >>>> Your previous approaches received valid feedback, including even fixes
> >>>> and committment of new maintainer.
> >>>>
> >>>> But you even complained that it does receive fixes! [1]
> >>>>
> >>>> We removed the driver temporarily from typical configurations, so no one
> >>>> will be affected, by whatever is found now and not yet fixed. Still not
> >>>> enough! For you this was reason to remove the driver (AGAIN!) [2]
> >>>
> >>> A stub driver needs to be added for power management reasons.  It turns
> >>> out that if there is no driver, power consumption is very high.
> >>>
> >>>> This is beyond comprehension and very unpleasant, because you actively
> >>>> work against the community with this approach.
> >>>
> >>> I trust that Bartosz can fix the driver with enough work.  However,
> >>> even if the QCE driver had never had a single bug, it would still not
> >>> be worth using.  Its performance is so poor that one should always
> >>> use CPU-based crypto instead.  In the default configuration, that is
> >>> in fact what happens.
> >>>
> >>> The QCE's algorithm implementations only provide a way people can
> >>> misconfigure their systems and ruin their performance.  There are
> >>> debug options that also severely harm performance, but they have
> >>> legitimate uses during development.  The current QCE driver doesn't.
> >>>
> >>> I have no problem with a driver for the QCE that does something actually
> >>> useful.  While there are disagreements about whether restricted media
> >>> processing is a feature or an anti-feature, my view is that it is
> >>> better for it to be implemented upstream than in an out-of-tree driver.
> >>> However, the driver as it currently exists does not implement this.
> >>>
> >>> I expect that such a driver would not use the crypto API at all.
> >>> Instead, I suspect it would use dmabufs for source and destination
> >>> buffers and integrate with the secure world firmware in some way.
> >>> That means that the driver doesn't belong under drivers/crypto.
> >>>
> >>> These are the reasons I submitted a patch to mark the driver as BROKEN,
> >>> which Herbert Xu has since accepted.
> >>
> >> And Bartosz and other people committed to work on this by improving,
> >> fixing and in the long term providing you with the actual important user
> >> of this. All this was already said.
> >>
> >> And then after having all these discussions Eric sends AGAIN patch to
> >> remove the driver. How many times this will have to be discussed the
> >> same way? If we now reach agreement the driver stays, next month again
> >> there will be a patch to remove it? And then one more month again?
> >>
> >> The driver is marked as BROKEN, thus absolutely NO ONE is affected by
> >> any issues the driver has.
> >>
> >> It's some personal vendetta to keep coming after that - unimportant now
> >> - driver.
> > 
> > Well, there was also feedback that BROKEN drivers should not stay in the
> > tree and instead just be removed.  So I was considering that feedback
> > too, as well as the results of additional testing and review of this
> > driver, including evidence that the driver performs even worse than
> > thought and is actually getting even worse; the performance seems
> > irredeemable.  As for proposing the stub driver, that is addressing the
> > power management issue that was mentioned on the other thread.
> > 
> > Anyway, it sounds like you support it being BROKEN.  Okay, but again, as
> > Greg mentioned
> > (https://lore.kernel.org/linux-crypto/2026071312-uncover-refining-8cac@gregkh/)
> > it is awkward to have BROKEN stuff in the tree since it cannot even be
> > built.  I'm not sure we can have it both ways!
> 
> Which was I think pointed already a few times that it will not stay
> BROKEN but will get fixed and get proper useful use cases.

Well, that again brings us back to the core issue which is the actual
current functionality of the driver, which is to register crypto_ahash,
crypto_skcipher, and crypto_aead algorithms with the crypto API.

It isn't useful functionality, but rather just a footgun that allows
users to misconfigure their systems, an issue I've seen happen multiple
times.  CPU-based implementations of *every one* of those algorithms
already exist.  On a typical SoC that has this hardware, the CPU-based
implementations are ~50x faster as shown in tests.  Pending patches make
the difference even greater at ~100x.  And the CPU-based implementations
actually use significantly less CPU time, as well.  There seems to be no
path forward for significantly fixing this issue, either.

Then, if these algorithms are removed, the rest of the code in the
driver (besides setting the interconnect bandwidth) becomes unused.  The
policy in the kernel for dead code is that it should be removed.  So
that is why it makes sense to drop that code as well, but really the
core issue is the crypto algorithm registrations, not the supporting
code per se.

An alternative we could consider is dropping the cra_priority further,
to further decrease the chance that these algorithms are used.  But I
feel it's hard to justify why they're there at all, if the rationale for
keeping them is "we made sure that no one can actually use them, so they
can't be causing problems anymore"...

- Eric

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

[Bartosz Golaszewski]

On Sat, 1 Aug 2026 19:12:42 +0200, Eric Biggers <ebiggers@kernel.org> said:

> Well, that again brings us back to the core issue which is the actual
> current functionality of the driver, which is to register crypto_ahash,
> crypto_skcipher, and crypto_aead algorithms with the crypto API.
>
> It isn't useful functionality, but rather just a footgun that allows
> users to misconfigure their systems, an issue I've seen happen multiple
> times.  CPU-based implementations of *every one* of those algorithms
> already exist.  On a typical SoC that has this hardware, the CPU-based
> implementations are ~50x faster as shown in tests.  Pending patches make
> the difference even greater at ~100x.  And the CPU-based implementations
> actually use significantly less CPU time, as well.  There seems to be no
> path forward for significantly fixing this issue, either.
>

You've repeated your point about performance several times. Nobody ever said
you're wrong. Performance is not the only reason for choosing one provider over
another.

> An alternative we could consider is dropping the cra_priority further,
> to further decrease the chance that these algorithms are used.  But I
> feel it's hard to justify why they're there at all, if the rationale for
> keeping them is "we made sure that no one can actually use them, so they
> can't be causing problems anymore"...
>

No, the rationale has never been this. FWIW it can be that it's used for
testing of the crypto module on a supported platform and that is already
enough of a reason to keep it upstream.

As I've said before: we don't just drop maintained drivers from linux.

Herbert: it's been a while, would you mind chiming in and deciding one way
or the other, as Greg seems to have deferred to your judgement?

Thanks,
Bartosz

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

[Eric Biggers]

On Tue, Aug 11, 2026 at 08:42:55AM -0500, Bartosz Golaszewski wrote:
> On Sat, 1 Aug 2026 19:12:42 +0200, Eric Biggers <ebiggers@kernel.org> said:
> 
> > Well, that again brings us back to the core issue which is the actual
> > current functionality of the driver, which is to register crypto_ahash,
> > crypto_skcipher, and crypto_aead algorithms with the crypto API.
> >
> > It isn't useful functionality, but rather just a footgun that allows
> > users to misconfigure their systems, an issue I've seen happen multiple
> > times.  CPU-based implementations of *every one* of those algorithms
> > already exist.  On a typical SoC that has this hardware, the CPU-based
> > implementations are ~50x faster as shown in tests.  Pending patches make
> > the difference even greater at ~100x.  And the CPU-based implementations
> > actually use significantly less CPU time, as well.  There seems to be no
> > path forward for significantly fixing this issue, either.
> >
> 
> You've repeated your point about performance several times. Nobody ever said
> you're wrong. Performance is not the only reason for choosing one provider over
> another.

That isn't a very practical viewpoint for the in-kernel crypto use
cases, where performance tends to be critical and users will do a lot to
get even a few percent improvement, let alone 10000%!  But as Demi and I
have explained, even if the performance aspect is ignored the driver
still isn't worth it, for multiple other reasons.

Also, you did give saving CPU cycles as a reason earlier
(https://lore.kernel.org/linux-crypto/CAMRc=Me55rUmjjR+ZzdWd2ss9JJMZzJch0zKd4GqONBjCzFMYQ@mail.gmail.com/).
That's one of the reasons I actually tested it and responded to that.
It sounds like you've now walked back your claim.  So great, we seem to
be on the same page regarding that point now.

> > An alternative we could consider is dropping the cra_priority further,
> > to further decrease the chance that these algorithms are used.  But I
> > feel it's hard to justify why they're there at all, if the rationale for
> > keeping them is "we made sure that no one can actually use them, so they
> > can't be causing problems anymore"...
> >
> 
> No, the rationale has never been this. FWIW it can be that it's used for
> testing of the crypto module on a supported platform and that is already
> enough of a reason to keep it upstream.
> 
> As I've said before: we don't just drop maintained drivers from linux.

We definitely do if the drivers are not useful or appropriate for
inclusion in the kernel, though the policy varies by subsystem.  Even
just last month an entire filesystem got dropped despite someone wanting
to maintain it.

- Eric

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

[Bartosz Golaszewski]

On Wed, 12 Aug 2026 00:41:10 +0200, Eric Biggers <ebiggers@kernel.org> said:
> On Tue, Aug 11, 2026 at 08:42:55AM -0500, Bartosz Golaszewski wrote:
>
>> No, the rationale has never been this. FWIW it can be that it's used for
>> testing of the crypto module on a supported platform and that is already
>> enough of a reason to keep it upstream.
>>
>> As I've said before: we don't just drop maintained drivers from linux.
>
> We definitely do if the drivers are not useful or appropriate for
> inclusion in the kernel, though the policy varies by subsystem.  Even
> just last month an entire filesystem got dropped despite someone wanting
> to maintain it.
>

I suppose you're talking about EFS? A thing that can't even be tested on real
hardware? This is a much more extreme case. Here you have several people
representing the vendor saying they want to keep and maintain it. I'm fine
having my own tree and sending PRs to Herbert if he agrees and you are of
course entirely free to disregard any patches targetting it.

Thanks,
Bartosz

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

[Demi Marie Obenour]

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

On 8/12/26 04:07, Bartosz Golaszewski wrote:
> On Wed, 12 Aug 2026 00:41:10 +0200, Eric Biggers <ebiggers@kernel.org> said:
>> On Tue, Aug 11, 2026 at 08:42:55AM -0500, Bartosz Golaszewski wrote:
>>
>>> No, the rationale has never been this. FWIW it can be that it's used for
>>> testing of the crypto module on a supported platform and that is already
>>> enough of a reason to keep it upstream.
>>>
>>> As I've said before: we don't just drop maintained drivers from linux.
>>
>> We definitely do if the drivers are not useful or appropriate for
>> inclusion in the kernel, though the policy varies by subsystem.  Even
>> just last month an entire filesystem got dropped despite someone wanting
>> to maintain it.
>>
> 
> I suppose you're talking about EFS? A thing that can't even be tested on real
> hardware? This is a much more extreme case. Here you have several people
> representing the vendor saying they want to keep and maintain it. I'm fine
> having my own tree and sending PRs to Herbert if he agrees and you are of
> course entirely free to disregard any patches targetting it.

Would you be okay with one of these options?

1. Make the stub driver as the default, with
   the real driver guarded by an off-by-default Kconfig option.

2. Keep the driver as is (with the power management issue fixed),
   but don't register any algorithm implementations with the crypto API.

3. Only register algorithm implementations with the crypto API if a
   non-default Kconfig option is selected.

4. Only register algorithm implementations with the crypto API if a
   non-default module parameter is selected.

5. Move the driver out of drivers/crypto, delete the algorithm
   registrations, and then add support for restricted media content
   decryption later.

The last option would be my preferred one.

Also, if Qualcomm has customers who use the QCE via the crypto API,
is Qualcomm aware of _why_ they made that choice?  From Eric's and
my perspective, that seems like a very strange decision, unless there
are factors that Eric and I are not aware of.

Finally, would you be willing to have a group call to discuss this
at some point?  That might be a more suitable venue for this kind of
discussion, and could potentially lead to a conclusion faster.  Also,
while at least I couldn't sign any NDAs, there might be information that
you could discuss there that you cannot post to a public mailing list.
-- 
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 Wed, 12 Aug 2026 18:33:45 +0200, Demi Marie Obenour
<demiobenour@gmail.com> said:
> On 8/12/26 04:07, Bartosz Golaszewski wrote:
>> On Wed, 12 Aug 2026 00:41:10 +0200, Eric Biggers <ebiggers@kernel.org> said:
>>> On Tue, Aug 11, 2026 at 08:42:55AM -0500, Bartosz Golaszewski wrote:
>>>
>>>> No, the rationale has never been this. FWIW it can be that it's used for
>>>> testing of the crypto module on a supported platform and that is already
>>>> enough of a reason to keep it upstream.
>>>>
>>>> As I've said before: we don't just drop maintained drivers from linux.
>>>
>>> We definitely do if the drivers are not useful or appropriate for
>>> inclusion in the kernel, though the policy varies by subsystem.  Even
>>> just last month an entire filesystem got dropped despite someone wanting
>>> to maintain it.
>>>
>>
>> I suppose you're talking about EFS? A thing that can't even be tested on real
>> hardware? This is a much more extreme case. Here you have several people
>> representing the vendor saying they want to keep and maintain it. I'm fine
>> having my own tree and sending PRs to Herbert if he agrees and you are of
>> course entirely free to disregard any patches targetting it.
>
> Would you be okay with one of these options?
>
> 1. Make the stub driver as the default, with
>    the real driver guarded by an off-by-default Kconfig option.
>
> 2. Keep the driver as is (with the power management issue fixed),
>    but don't register any algorithm implementations with the crypto API.
>
> 3. Only register algorithm implementations with the crypto API if a
>    non-default Kconfig option is selected.
>
> 4. Only register algorithm implementations with the crypto API if a
>    non-default module parameter is selected.
>
> 5. Move the driver out of drivers/crypto, delete the algorithm
>    registrations, and then add support for restricted media content
>    decryption later.
>
> The last option would be my preferred one.
>

I actually have a slightly different spin on the stub driver. I'd like to
keep the driver future-proof and avoid all kinds of needless churn when we
do get to the secure dma bufs and restricted media playback (and we will get
there eventually :) ).

Can we make the QCE into an MFD driver with the core binding to the DT node
and handling the interconnect bandwidth, while the part handling crypto
algorithms becomes its cell and is moved into a separate module? This one can
be disabled in defconfig by default and I don't even mind it being guarded with
CONFIG_EXPERT. The core module would remain in arm64 defconfig.

With that: any future extensions providing support for more functionalities of
this IP can simply be implemented as more MFD cells while we can also keep
chipping away at the existing code and use it for testing the BAM pipe locking.

> Also, if Qualcomm has customers who use the QCE via the crypto API,
> is Qualcomm aware of _why_ they made that choice?  From Eric's and
> my perspective, that seems like a very strange decision, unless there
> are factors that Eric and I are not aware of.
>

SM8650 with the QCE is FIPS 140-2 certified and apparently this makes
a difference. Don't ask me if that makes sense, I don't claim to understand
the legal aspects of it.

> Finally, would you be willing to have a group call to discuss this
> at some point?  That might be a more suitable venue for this kind of
> discussion, and could potentially lead to a conclusion faster.  Also,
> while at least I couldn't sign any NDAs, there might be information that
> you could discuss there that you cannot post to a public mailing list.

Sure, I'm fine to have a call. If you think about coming to Linux Plumbers this
year, I'll be there all week and it's a good place for such a discussion too.

FYI I've only been online for a couple days this week and I'm going offline
again until the end of August with the exception of August 26th when I'll be
able to chime in again. Any call would too have to happen in September at the
earliest.

Thanks,
Bartosz

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

[Demi Marie Obenour]

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

On 8/13/26 07:53, Bartosz Golaszewski wrote:
> On Wed, 12 Aug 2026 18:33:45 +0200, Demi Marie Obenour
> <demiobenour@gmail.com> said:
>> On 8/12/26 04:07, Bartosz Golaszewski wrote:
>>> On Wed, 12 Aug 2026 00:41:10 +0200, Eric Biggers <ebiggers@kernel.org> said:
>>>> On Tue, Aug 11, 2026 at 08:42:55AM -0500, Bartosz Golaszewski wrote:
>>>>
>>>>> No, the rationale has never been this. FWIW it can be that it's used for
>>>>> testing of the crypto module on a supported platform and that is already
>>>>> enough of a reason to keep it upstream.
>>>>>
>>>>> As I've said before: we don't just drop maintained drivers from linux.
>>>>
>>>> We definitely do if the drivers are not useful or appropriate for
>>>> inclusion in the kernel, though the policy varies by subsystem.  Even
>>>> just last month an entire filesystem got dropped despite someone wanting
>>>> to maintain it.
>>>>
>>>
>>> I suppose you're talking about EFS? A thing that can't even be tested on real
>>> hardware? This is a much more extreme case. Here you have several people
>>> representing the vendor saying they want to keep and maintain it. I'm fine
>>> having my own tree and sending PRs to Herbert if he agrees and you are of
>>> course entirely free to disregard any patches targetting it.
>>
>> Would you be okay with one of these options?
>>
>> 1. Make the stub driver as the default, with
>>    the real driver guarded by an off-by-default Kconfig option.
>>
>> 2. Keep the driver as is (with the power management issue fixed),
>>    but don't register any algorithm implementations with the crypto API.
>>
>> 3. Only register algorithm implementations with the crypto API if a
>>    non-default Kconfig option is selected.
>>
>> 4. Only register algorithm implementations with the crypto API if a
>>    non-default module parameter is selected.
>>
>> 5. Move the driver out of drivers/crypto, delete the algorithm
>>    registrations, and then add support for restricted media content
>>    decryption later.
>>
>> The last option would be my preferred one.
>>
> 
> I actually have a slightly different spin on the stub driver. I'd like to
> keep the driver future-proof and avoid all kinds of needless churn when we
> do get to the secure dma bufs and restricted media playback (and we will get
> there eventually :) ).
> 
> Can we make the QCE into an MFD driver with the core binding to the DT node
> and handling the interconnect bandwidth, while the part handling crypto
> algorithms becomes its cell and is moved into a separate module? This one can
> be disabled in defconfig by default and I don't even mind it being guarded with
> CONFIG_EXPERT. The core module would remain in arm64 defconfig.
> 
> With that: any future extensions providing support for more functionalities of
> this IP can simply be implemented as more MFD cells while we can also keep
> chipping away at the existing code and use it for testing the BAM pipe locking.

I'm not familiar with MFD drivers, but this sounds like the best
of both worlds.  The driver stays upstream, but anyone who doesn't
explicitly need it can ignore it.

>> Also, if Qualcomm has customers who use the QCE via the crypto API,
>> is Qualcomm aware of _why_ they made that choice?  From Eric's and
>> my perspective, that seems like a very strange decision, unless there
>> are factors that Eric and I are not aware of.
>>
> 
> SM8650 with the QCE is FIPS 140-2 certified and apparently this makes
> a difference. Don't ask me if that makes sense, I don't claim to understand
> the legal aspects of it.

That sounds like the kind of thing that would cause someone to use
a worse implementation of cryptographic algorithms 🙂.

For what it is worth, many common uses of kernel cryptography can't
use the QCE.  This notably applies to the CSPRNG (/dev/urandom etc),
storage encryption, and AF_ALG.  Therefore, I expect that anyone who
needs FIPS 140 compliance with Linux needs to ensure that the software
crypto is certified, as that's what actually gets used in most cases.
>> Finally, would you be willing to have a group call to discuss this
>> at some point?  That might be a more suitable venue for this kind of
>> discussion, and could potentially lead to a conclusion faster.  Also,
>> while at least I couldn't sign any NDAs, there might be information that
>> you could discuss there that you cannot post to a public mailing list.
> 
> Sure, I'm fine to have a call. If you think about coming to Linux Plumbers this
> year, I'll be there all week and it's a good place for such a discussion too.
> 
> FYI I've only been online for a couple days this week and I'm going offline
> again until the end of August with the exception of August 26th when I'll be
> able to chime in again. Any call would too have to happen in September at the
> earliest.
That's fine.
-- 
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

[Eric Biggers]

On Thu, Aug 13, 2026 at 03:09:36PM -0400, Demi Marie Obenour wrote:
> > SM8650 with the QCE is FIPS 140-2 certified and apparently this makes
> > a difference. Don't ask me if that makes sense, I don't claim to understand
> > the legal aspects of it.
> 
> That sounds like the kind of thing that would cause someone to use
> a worse implementation of cryptographic algorithms 🙂.
> 
> For what it is worth, many common uses of kernel cryptography can't
> use the QCE.  This notably applies to the CSPRNG (/dev/urandom etc),
> storage encryption, and AF_ALG.  Therefore, I expect that anyone who
> needs FIPS 140 compliance with Linux needs to ensure that the software
> crypto is certified, as that's what actually gets used in most cases.

Well there's that, and also the Inline Crypto Engine (which is actually
being used and is unrelated to QCE) has its own FIPS certification.

Which upstream kernel feature, if any, is QCE being used with?  I have
asked this multiple times and still not gotten an answer.

Does the perceived value perhaps come merely from "offering" FIPS
certified cryptography on the system -- then not actually using it in
practice?  That can maybe be understandable in some cases, but it's also
not really something that belongs upstream where actual technical merit
is expected.  Also, could ICE be enough to check that checkbox?

- Eric

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

[Demi Marie Obenour]

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

On 8/13/26 16:10, Eric Biggers wrote:
> On Thu, Aug 13, 2026 at 03:09:36PM -0400, Demi Marie Obenour wrote:
>>> SM8650 with the QCE is FIPS 140-2 certified and apparently this makes
>>> a difference. Don't ask me if that makes sense, I don't claim to understand
>>> the legal aspects of it.
>>
>> That sounds like the kind of thing that would cause someone to use
>> a worse implementation of cryptographic algorithms 🙂.
>>
>> For what it is worth, many common uses of kernel cryptography can't
>> use the QCE.  This notably applies to the CSPRNG (/dev/urandom etc),
>> storage encryption, and AF_ALG.  Therefore, I expect that anyone who
>> needs FIPS 140 compliance with Linux needs to ensure that the software
>> crypto is certified, as that's what actually gets used in most cases.
> 
> Well there's that, and also the Inline Crypto Engine (which is actually
> being used and is unrelated to QCE) has its own FIPS certification.
> 
> Which upstream kernel feature, if any, is QCE being used with?  I have
> asked this multiple times and still not gotten an answer.
Historically AF_ALG has been used to access FIPS-certified crypto
from userspace, but that no longer can be used with QCE.  Most kernel
features reject CRYPTO_ALG_ASYNC, CRYPTO_ALG_ALLOCATES_MEMORY, or both.
If I exclude tests and fallback paths of other drivers, that leaves
crypto/asymmetric_keys/public_key.c (hashing), BPF, MACsec, OpenVPN
data channel offload, ecryptfs, Ceph, SMB, IPsec, mac802154, TIPC, TLS,
and crypto/kpp.c.  AF_ALG only allows offload drivers for cbc(paes),
which QCE doesn't implement.  QCE is broken with IPsec and probably
some other networking protocols, as it would wind up taking a mutex
in softirq context.

Is there anything on that list that you don't plan to convert
to the crypto library?
> Does the perceived value perhaps come merely from "offering" FIPS
> certified cryptography on the system -- then not actually using it in
> practice?  That can maybe be understandable in some cases, but it's also
> not really something that belongs upstream where actual technical merit
> is expected.  Also, could ICE be enough to check that checkbox?

I agree with all of these points.
-- 
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

[Eric Biggers]

On Thu, Aug 13, 2026 at 05:00:28PM -0400, Demi Marie Obenour wrote:
> On 8/13/26 16:10, Eric Biggers wrote:
> > On Thu, Aug 13, 2026 at 03:09:36PM -0400, Demi Marie Obenour wrote:
> >>> SM8650 with the QCE is FIPS 140-2 certified and apparently this makes
> >>> a difference. Don't ask me if that makes sense, I don't claim to understand
> >>> the legal aspects of it.
> >>
> >> That sounds like the kind of thing that would cause someone to use
> >> a worse implementation of cryptographic algorithms 🙂.
> >>
> >> For what it is worth, many common uses of kernel cryptography can't
> >> use the QCE.  This notably applies to the CSPRNG (/dev/urandom etc),
> >> storage encryption, and AF_ALG.  Therefore, I expect that anyone who
> >> needs FIPS 140 compliance with Linux needs to ensure that the software
> >> crypto is certified, as that's what actually gets used in most cases.
> > 
> > Well there's that, and also the Inline Crypto Engine (which is actually
> > being used and is unrelated to QCE) has its own FIPS certification.
> > 
> > Which upstream kernel feature, if any, is QCE being used with?  I have
> > asked this multiple times and still not gotten an answer.
> Historically AF_ALG has been used to access FIPS-certified crypto
> from userspace, but that no longer can be used with QCE.  Most kernel
> features reject CRYPTO_ALG_ASYNC, CRYPTO_ALG_ALLOCATES_MEMORY, or both.
> If I exclude tests and fallback paths of other drivers, that leaves
> crypto/asymmetric_keys/public_key.c (hashing), BPF, MACsec, OpenVPN
> data channel offload, ecryptfs, Ceph, SMB, IPsec, mac802154, TIPC, TLS,
> and crypto/kpp.c.  AF_ALG only allows offload drivers for cbc(paes),
> which QCE doesn't implement.  QCE is broken with IPsec and probably
> some other networking protocols, as it would wind up taking a mutex
> in softirq context.

It's a far more narrow list than even that.  asymmetric_keys uses the
shash API, so it cannot use QCE.  BPF crypto uses the lskcipher API, so
it cannot use QCE.  MACsec uses AES-GCM which QCE driver doesn't have.
OpenVPN uses AES-GCM or ChaCha20Poly1305, neither of which the QCE
driver has.  The non-legacy Ceph protocol uses AES-GCM, which again the
QCE driver doesn't have.  IPsec does decryption in softirq context,
which the QCE driver doesn't support.  QCE driver does not implement kpp
algorithms.  SMB, TIPC, TLS, all usually use AES-GCM as well.  Until
just a couple months ago ksmbd crashed if passed an async algorithm,
which proves it was never used with QCE.  There is really not much left,
maybe ecryptfs AES-CBC file contents encryption?  But even for that the
cra_priority makes it not actually used.

But we are going into the weeds here with trying to theorize some way
the code could theoretically be reached.  It seems it's really just not.
Or at least if there is a use it is not upstream and/or is not actually
the appropriate code to be using.

- Eric

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1838 bytes --]

Hello,

On Thu, Jul 30, 2026 at 10:08:38PM -0700, Eric Biggers wrote:
> diff --git a/drivers/crypto/qce/qcrypto.c b/drivers/crypto/qce/qcrypto.c
> new file mode 100644
> index 000000000000..d526902b3060
> --- /dev/null
> +++ b/drivers/crypto/qce/qcrypto.c
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2010-2014, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/interconnect.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/platform_device.h>

<linux/platform_device.h> already makes sure that of_device_id is
available, so don't include <linux/mod_devicetable.h> which will go away
soon.

Also I wonder why/if you need <linux/device.h> here.

> +static int qce_crypto_probe(struct platform_device *pdev)
> +{
> +	struct icc_path *mem_path;
> +
> +	mem_path = devm_of_icc_get(&pdev->dev, "memory");
> +	if (IS_ERR(mem_path))
> +		return PTR_ERR(mem_path);
> +
> +	return icc_set_bw(mem_path, 0, 0);
> +}
> +
> +static const struct of_device_id qce_crypto_of_match[] = {
> +	{ .compatible = "qcom,crypto-v5.1", },

No , before a closing } please

> +	{ .compatible = "qcom,crypto-v5.4", },
> +	{ .compatible = "qcom,qce", },
> +	{}

A single space between { and } is the most usual form, please stick to
that.

> +};
> +MODULE_DEVICE_TABLE(of, qce_crypto_of_match);
> +
> +static struct platform_driver qce_crypto_driver = {
> +	.probe = qce_crypto_probe,
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +		.of_match_table = qce_crypto_of_match,
> +	},
> +};
> +module_platform_driver(qce_crypto_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Qualcomm crypto engine stub driver");
> +MODULE_ALIAS("platform:" KBUILD_MODNAME);
> +MODULE_AUTHOR("The Linux Foundation");

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]