Re: [PATCH 0/3] Add support for qcrypto on shikra

[Dmitry Baryshkov]

On Thu, May 21, 2026 at 09:49:12PM -0500, Eric Biggers wrote:
> On Thu, May 21, 2026 at 12:21:41PM +0530, Kuldeep Singh wrote:
> > On 15-05-2026 01:17, Eric Biggers wrote:
> > > On Fri, May 15, 2026 at 12:53:35AM +0530, Kuldeep Singh wrote:
> > >> Add qcrypto and cryptobam DT nodes for enabling qcrypto on kaanapali.
> > >> Shikra bam dma supports 7 iommus so update dt-bindings accordingly.
> > >>
> > >> The patchset depends on below. There's recursive dependency so referred
> > >> to base DT patch here.
> > >> - https://lore.kernel.org/all/20260512-shikra-dt-v1-0-716438330dd0@xxxxxxxxxxxxxxxx/
> > >>
> > >> Validations:
> > >> - make ARCH=arm64 DT_CHECKER_FLAGS=-m DT_SCHEMA_FILES=Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml dt_binding_check
> > >> - make ARCH=arm64 qcom/shikra-cqs-evk.dtb CHECK_DTBS=1 DT_SCHEMA_FILES=Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
> > >> - cryptobam and crypto driver probe
> > >> - kcapi test
> > >>
> > >> Signed-off-by: Kuldeep Singh <kuldeep.singh@xxxxxxxxxxxxxxxx>
> > > 
> > > What specific kernel features would this be useful for, and what
> > > specific performance improvements are you seeing with those features?
> > 
> > I hope you mean 7 iommu entries.
> > 
> > Please note, shikra is an old platform and differs with latest platforms
> > like kaanapali in terms of iommus#.
> > Kaanapali is optimised(in terms of iommus#) as same pipe index/sid i.e
> > 4/5 can be used for general purpose or for any other usecase like
> > DRM/HDCP etc.
> > Whereas for shikra, there's dedicated iommu entry for each usecase and
> > same pipe index/sid cannot be used for other usecases.
> > 
> > The performance will be be effectively similar.
> 
> It sounds like you don't actually have an answer to my questions, then.
> 
> Performance tests (e.g.
> https://lore.kernel.org/r/20250615031807.GA81869@sol/) have clearly
> shown that this driver is an order of magnitude slower than the CPU.

Are other harware crypto drivers faster or slower than the CPU
implementation? What about the CAAM (sorry, it's just the driver that I
worked with few years ago). Or Xilinx? My guess would be that for the
most of the modern ARM64 hardware the NEON implementation is faster than
the "hw IP" one. My assumtion has always been that we support crypto IP
for the sake of security (i.e. making sure that the key can't be found
in the cleartext in memory dumps or that it's impossible to tamper with
the hash values before singing/verification). From this point of view,
using priorities is expected and logical: most of the users will need a
quickest implementation. Some users will need to use protected keys or
other hw-only features.

Note, I'm not commenting on the driver being buggy. If the issues are
not fixed in a timely manner, it should be marked with 'depends on
BROKEN' and further removed if the issues contine to be non-fixed.

> This driver has historically been quite harmful.  People were using it
> accidentally and encountering very bad performance, as well as bugs such
> as crashes and filesystem hangs.  We fixed that by lowering its
> cra_priority.  But for the same reason, even when enabled on a platform,
> it's not actually used.  Linux would be better without this driver.
> 
> We seem to be seeing the usual drivers/crypto/ pattern here: this crypto
> offload driver is being pushed by the hardware manufacturer, with no
> awareness of the fact that it's actually useless in Linux.
> 
> I've had enough of this.  Please consider this series:
> 
>     Nacked-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> 
> FWIW: the approaches that are actually used and work well in Linux are
> ICE and the CPU-accelerated crypto.
> 
> - Eric

-- 
With best wishes
Dmitry