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

[Demi Marie Obenour <demiobenour@gmail.com>]

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