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