Re: [PATCH v19 00/14] crypto/dmaengine: qce: introduce BAM locking and use DMA for register I/O

[Bartosz Golaszewski]

On Fri, May 29, 2026 at 6:24 PM Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
>
> On Tue, May 26, 2026 at 03:10:48PM +0200, Bartosz Golaszewski wrote:
> > I feel like I fell into the trap of trying to address pre-existing
> > issues reported by sashiko and in the process provoking more reports so
> > let this be the last iteration where I do this. Vinod can we get this
> > queued for v7.2 now and iron out any previously existing problems in
> > tree?
> >
> > Merging strategy: there are build-time dependencies between the crypto
> > and DMA patches so the best approach is for Vinod to create an immutable
> > branch with the DMA part pulled in by the crypto tree.
> >
> > This iteration continues to build on top of v12 but uses the BAM's NWD
> > bit on data descriptors as suggested by Stephan. To that end, there are
> > some more changes like reversing the order of command and data
> > descriptors queuedy by the QCE driver.
> >
> > Currently the QCE crypto driver accesses the crypto engine registers
> > directly via CPU. Trust Zone may perform crypto operations simultaneously
> > resulting in a race condition. To remedy that, let's introduce support
> > for BAM locking/unlocking to the driver. The BAM driver will now wrap
> > any existing issued descriptor chains with additional descriptors
> > performing the locking when the client starts the transaction
> > (dmaengine_issue_pending()). The client wanting to profit from locking
> > needs to switch to performing register I/O over DMA and communicate the
> > address to which to perform the dummy writes via a call to
> > dmaengine_desc_attach_metadata().
> >
> > In the specific case of the BAM DMA this translates to sending command
> > descriptors performing dummy writes with the relevant flags set. The BAM
> > will then lock all other pipes not related to the current pipe group, and
> > keep handling the current pipe only until it sees the the unlock bit.
> >
> > In order for the locking to work correctly, we also need to switch to
> > using DMA for all register I/O.
> >
> > On top of this, the series contains some additional tweaks and
> > refactoring.
> >
> > The goal of this is not to improve the performance but to prepare the
> > driver for supporting decryption into secure buffers in the future.
> >
> > Tested with tcrypt.ko, kcapi and cryptsetup.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxxxxxxxx>
>
> None of these fixes are Cc'ed to stable, so stable kernels will remain
> vulnerable to these race conditions.
>
> Shouldn't this be preceded by a patch, Cc'ed to stable, that marks the
> driver as BROKEN?  As discussed in the other thread
> (https://lore.kernel.org/linux-crypto/20260515-shikra_qcrypto-v1-0-80f07b345c29@xxxxxxxxxxxxxxxx/T/#u),
> none of the current functionality of this driver is actually useful in
> Linux.  It's just been causing problems.
>

I don't believe any of it should be backported. This is not a
regression, multiple EEs were never supported, so it's a new feature.
Also: backporting of over 500 diff lines across two subsystems doesn't
sound like a good idea to me.

On the other hand, if marking the driver as BROKEN for the time being
allows for a faster pace of queuing any changes to it, then I'm for
it. Let's unmark it once we fix it.

Bart