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

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

[Kuldeep Singh]

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@oss.qualcomm.com/

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@oss.qualcomm.com>
---
Kuldeep Singh (3):
      dt-bindings: crypto: qcom-qce: Document the Shikra crypto engine
      dt-bindings: bam-dma: Increase maxItems to seven for iommus
      arm64: dts: qcom: shikra: Add qcrypto node support

 .../devicetree/bindings/crypto/qcom-qce.yaml       |  1 +
 .../devicetree/bindings/dma/qcom,bam-dma.yaml      |  2 +-
 arch/arm64/boot/dts/qcom/shikra.dtsi               | 35 ++++++++++++++++++++++
 3 files changed, 37 insertions(+), 1 deletion(-)
---
base-commit: 33c8e3305b65a2e757e68b10af521ad54ea051a6
change-id: 20260514-shikra_qcrypto-f61f4d363e6e

Best regards,
--  
Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>

[PATCH 1/3] dt-bindings: crypto: qcom-qce: Document the Shikra crypto engine

[Kuldeep Singh]

Document the crypto engine on the Shikra platform.

Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
index 69101cead3bc..ad0e1cd3a76a 100644
--- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
+++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
@@ -53,6 +53,7 @@ properties:
               - qcom,qcs8300-qce
               - qcom,sa8775p-qce
               - qcom,sc7280-qce
+              - qcom,shikra-qce
               - qcom,sm6350-qce
               - qcom,sm8250-qce
               - qcom,sm8350-qce

-- 
2.34.1

[PATCH 2/3] dt-bindings: bam-dma: Increase maxItems to seven for iommus

[Kuldeep Singh]

Shikra bam dma engine support seven iommu entries.
Increase maxItems property for iommus to pass dtbs_check errors.

Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
index 6493a6968bb4..ffdb308352c3 100644
--- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
+++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
@@ -46,7 +46,7 @@ properties:
 
   iommus:
     minItems: 1
-    maxItems: 6
+    maxItems: 7
 
   num-channels:
     $ref: /schemas/types.yaml#/definitions/uint32

-- 
2.34.1

[PATCH 3/3] arm64: dts: qcom: shikra: Add qcrypto node support

[Kuldeep Singh]

Add qcrypto and cryptobam support for shikra target.

Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/shikra.dtsi | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
index 262c488add1e..dbac0e901d6e 100644
--- a/arch/arm64/boot/dts/qcom/shikra.dtsi
+++ b/arch/arm64/boot/dts/qcom/shikra.dtsi
@@ -541,6 +541,41 @@ config_noc: interconnect@1900000 {
 			#interconnect-cells = <2>;
 		};
 
+		cryptobam: dma-controller@1b04000 {
+			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
+			reg = <0x0 0x01b04000 0x0 0x24000>;
+			interrupts = <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			iommus = <&apps_smmu 0x84 0x0011>,
+				 <&apps_smmu 0x86 0x0011>,
+				 <&apps_smmu 0x92 0x0>,
+				 <&apps_smmu 0x94 0x0011>,
+				 <&apps_smmu 0x96 0x0011>,
+				 <&apps_smmu 0x98 0x0001>,
+				 <&apps_smmu 0x9F 0x0>;
+			qcom,ee = <0>;
+			qcom,controlled-remotely;
+			num-channels = <16>;
+			qcom,num-ees = <4>;
+		};
+
+		crypto: crypto@1b3a000 {
+			compatible = "qcom,shikra-qce", "qcom,sm8150-qce", "qcom,qce";
+			reg = <0x0 0x01b3a000 0x0 0x6000>;
+			dmas = <&cryptobam 4>, <&cryptobam 5>;
+			dma-names = "rx", "tx";
+			iommus = <&apps_smmu 0x84 0x0011>,
+				 <&apps_smmu 0x86 0x0011>,
+				 <&apps_smmu 0x92 0x0>,
+				 <&apps_smmu 0x94 0x0011>,
+				 <&apps_smmu 0x96 0x0011>,
+				 <&apps_smmu 0x98 0x0001>,
+				 <&apps_smmu 0x9F 0x0>;
+			interconnects = <&system_noc MASTER_CRYPTO_CORE0 0
+					 &mc_virt SLAVE_EBI_CH0 0>;
+			interconnect-names = "memory";
+		};
+
 		qfprom: efuse@1b44000 {
 			compatible = "qcom,shikra-qfprom", "qcom,qfprom";
 			reg = <0x0 0x01b44000 0x0 0x3000>;

-- 
2.34.1

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

[Eric Biggers]

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@oss.qualcomm.com/
> 
> 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@oss.qualcomm.com>

What specific kernel features would this be useful for, and what
specific performance improvements are you seeing with those features?

- Eric

Re: [PATCH 3/3] arm64: dts: qcom: shikra: Add qcrypto node support

[Konrad Dybcio]

On 5/14/26 9:23 PM, Kuldeep Singh wrote:
> Add qcrypto and cryptobam support for shikra target.
> 
> Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/shikra.dtsi | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
> index 262c488add1e..dbac0e901d6e 100644
> --- a/arch/arm64/boot/dts/qcom/shikra.dtsi
> +++ b/arch/arm64/boot/dts/qcom/shikra.dtsi
> @@ -541,6 +541,41 @@ config_noc: interconnect@1900000 {
>  			#interconnect-cells = <2>;
>  		};
>  
> +		cryptobam: dma-controller@1b04000 {
> +			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
> +			reg = <0x0 0x01b04000 0x0 0x24000>;
> +			interrupts = <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>;
> +			#dma-cells = <1>;
> +			iommus = <&apps_smmu 0x84 0x0011>,
> +				 <&apps_smmu 0x86 0x0011>,
> +				 <&apps_smmu 0x92 0x0>,

> +				 <&apps_smmu 0x94 0x0011>,
> +				 <&apps_smmu 0x96 0x0011>,

These two entries are logically the same (SID & ~mask) as the first two,
does it still work if you remove them?


> +				 <&apps_smmu 0x98 0x0001>,
> +				 <&apps_smmu 0x9F 0x0>;

Let's keep lowercase hex

Konrad

Re: [PATCH 1/3] dt-bindings: crypto: qcom-qce: Document the Shikra crypto engine

[Krzysztof Kozlowski]

On 14/05/2026 21:23, Kuldeep Singh wrote:
> Document the crypto engine on the Shikra platform.
> 
> Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
> ---

Same comments as for IPQ, Nord. I gave the same feedback internally more
than once.

NAK

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: crypto: qcom-qce: Document the Shikra crypto engine

[Kuldeep Singh]

On 15-05-2026 16:30, Krzysztof Kozlowski wrote:
> On 14/05/2026 21:23, Kuldeep Singh wrote:
>> Document the crypto engine on the Shikra platform.
>>
>> Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
>> ---
> 
> Same comments as for IPQ, Nord. I gave the same feedback internally more
> than once.

If i understand you correctly, you are looking for more descriptive
commit message?

-- 
Regards
Kuldeep

Re: [PATCH 1/3] dt-bindings: crypto: qcom-qce: Document the Shikra crypto engine

[Krzysztof Kozlowski]

On 19/05/2026 09:09, Kuldeep Singh wrote:
> On 15-05-2026 16:30, Krzysztof Kozlowski wrote:
>> On 14/05/2026 21:23, Kuldeep Singh wrote:
>>> Document the crypto engine on the Shikra platform.
>>>
>>> Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
>>> ---
>>
>> Same comments as for IPQ, Nord. I gave the same feedback internally more
>> than once.
> 
> If i understand you correctly, you are looking for more descriptive
> commit message?
> 

No, about proper patch organizing into one paychset instead of sending 5
different patchsets with oneliners. That's literally the last
thread/feedback on internal Open source forum chat, so easy to find.

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: crypto: qcom-qce: Document the Shikra crypto engine

[Kuldeep Singh]

> No, about proper patch organizing into one paychset instead of sending 5
> different patchsets with oneliners. That's literally the last
> thread/feedback on internal Open source forum chat, so easy to find.

Sure!
I'll align all 3 ssg modules(ice-ufs/emmc, rng and crypto) in one
patchseries for shikra and send together for ease in review.
Will ensure to follow same pattern in upcoming submissions too.

One doubt, there are mostly dts and dt-bindings patches so i think it's
best to organise per module patches together in big patchseries.

Kindly suggest for alternate opinion.

-- 
Regards
Kuldeep

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

[Kuldeep Singh]

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@oss.qualcomm.com/
>>
>> 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@oss.qualcomm.com>
> 
> 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.

-- 
Regards
Kuldeep

Re: [PATCH 3/3] arm64: dts: qcom: shikra: Add qcrypto node support

[Kuldeep Singh]

On 15-05-2026 15:58, Konrad Dybcio wrote:
> On 5/14/26 9:23 PM, Kuldeep Singh wrote:
>> Add qcrypto and cryptobam support for shikra target.
>>
>> Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
>> ---
>>  arch/arm64/boot/dts/qcom/shikra.dtsi | 35 +++++++++++++++++++++++++++++++++++
>>  1 file changed, 35 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
>> index 262c488add1e..dbac0e901d6e 100644
>> --- a/arch/arm64/boot/dts/qcom/shikra.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/shikra.dtsi
>> @@ -541,6 +541,41 @@ config_noc: interconnect@1900000 {
>>  			#interconnect-cells = <2>;
>>  		};
>>  
>> +		cryptobam: dma-controller@1b04000 {
>> +			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
>> +			reg = <0x0 0x01b04000 0x0 0x24000>;
>> +			interrupts = <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>;
>> +			#dma-cells = <1>;
>> +			iommus = <&apps_smmu 0x84 0x0011>,
>> +				 <&apps_smmu 0x86 0x0011>,
>> +				 <&apps_smmu 0x92 0x0>,
> 
>> +				 <&apps_smmu 0x94 0x0011>,
>> +				 <&apps_smmu 0x96 0x0011>,
> 
> These two entries are logically the same (SID & ~mask) as the first two,
> does it still work if you remove them?

Yes, resulting sid is same for 84/94 and 86/92.
Basically, the resulting sid could be same, it's an optimization which
smmu is doing which can result in same SMR(Stream matching register)
routing 2 different sid to same context bank.
So, 2 sid can be used even though resulting sid remains same.

Also, DT usually dictates what hw capabilities are supported and hence,
captured all apps entries here to match the hardware description.

I hope this answers your query.
> 
> 
>> +				 <&apps_smmu 0x98 0x0001>,
>> +				 <&apps_smmu 0x9F 0x0>;
> 
> Let's keep lowercase hex
Sure, will update in next rev.
Please note, I'll be clubbing patches together in one series as
suggested by krzysztof and fix this too that time.

-- 
Regards
Kuldeep

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

[Eric Biggers]

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@oss.qualcomm.com/
> >>
> >> 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@oss.qualcomm.com>
> > 
> > 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.

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@kernel.org>

FWIW: the approaches that are actually used and work well in Linux are
ICE and the CPU-accelerated crypto.

- Eric

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

[Kuldeep Singh]

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

+Bartosz, Gaurav, Neeraj

Hi Eric,

GPCE is relevant in terms of providing hardware security.
There are multiple usecases coming up for example to handle DRM/secure
buffer usecases to improve overall throughput for secure content.

Regarding performance, it's currently slower compared to arm CE but
provides an edge by giving hardware security which is considered more
secure.

Btw, there's been performance improvement with new targets and we are
expecting to achieve far more better performance with new SoCs family.
Pakala:    GPCE - 550MBps, ARMv8 - 8GBps
Kaanapali: GPCE - 3GBps,   ARMv8 - 10GBps

Please note, there's almost 5x improvement in kaanapali compared to
pakala. Though overall is still slower compared to arm but as mentioned,
expecting better performance with hardware improvements as we progress.

Also, currently qce driver exhibit stability issues and that's what we
are putting effort in stabilizing the software on immediate basis.

There's parallel effort ongoing by Bartosz to introduce baseline for
secure buffer usecases.
https://lore.kernel.org/lkml/20260522-qcom-qce-cmd-descr-v18-0-99103926bafc@oss.qualcomm.com/
There's active development ongoing and i believe lowering cra_priority
for qce is fine as of now and can scale values once qce becomes
performance efficient.

Please share your thoughts. Thanks!

-- 
Regards
Kuldeep

Re: [PATCH 3/3] arm64: dts: qcom: shikra: Add qcrypto node support

[Dmitry Baryshkov]

On Thu, May 21, 2026 at 02:15:45PM +0530, Kuldeep Singh wrote:
> On 15-05-2026 15:58, Konrad Dybcio wrote:
> > On 5/14/26 9:23 PM, Kuldeep Singh wrote:
> >> Add qcrypto and cryptobam support for shikra target.
> >>
> >> Signed-off-by: Kuldeep Singh <kuldeep.singh@oss.qualcomm.com>
> >> ---
> >>  arch/arm64/boot/dts/qcom/shikra.dtsi | 35 +++++++++++++++++++++++++++++++++++
> >>  1 file changed, 35 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/shikra.dtsi b/arch/arm64/boot/dts/qcom/shikra.dtsi
> >> index 262c488add1e..dbac0e901d6e 100644
> >> --- a/arch/arm64/boot/dts/qcom/shikra.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/shikra.dtsi
> >> @@ -541,6 +541,41 @@ config_noc: interconnect@1900000 {
> >>  			#interconnect-cells = <2>;
> >>  		};
> >>  
> >> +		cryptobam: dma-controller@1b04000 {
> >> +			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
> >> +			reg = <0x0 0x01b04000 0x0 0x24000>;
> >> +			interrupts = <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>;
> >> +			#dma-cells = <1>;
> >> +			iommus = <&apps_smmu 0x84 0x0011>,
> >> +				 <&apps_smmu 0x86 0x0011>,
> >> +				 <&apps_smmu 0x92 0x0>,
> > 
> >> +				 <&apps_smmu 0x94 0x0011>,
> >> +				 <&apps_smmu 0x96 0x0011>,
> > 
> > These two entries are logically the same (SID & ~mask) as the first two,
> > does it still work if you remove them?
> 
> Yes, resulting sid is same for 84/94 and 86/92.
> Basically, the resulting sid could be same, it's an optimization which
> smmu is doing which can result in same SMR(Stream matching register)
> routing 2 different sid to same context bank.
> So, 2 sid can be used even though resulting sid remains same.
> 
> Also, DT usually dictates what hw capabilities are supported and hence,
> captured all apps entries here to match the hardware description.
> 
> I hope this answers your query.

It doesn't. Can we drop them?

> > 
> > 
> >> +				 <&apps_smmu 0x98 0x0001>,
> >> +				 <&apps_smmu 0x9F 0x0>;
> > 
> > Let's keep lowercase hex
> Sure, will update in next rev.
> Please note, I'll be clubbing patches together in one series as
> suggested by krzysztof and fix this too that time.
> 
> -- 
> Regards
> Kuldeep
> 

-- 
With best wishes
Dmitry

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@oss.qualcomm.com/
> > >>
> > >> 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@oss.qualcomm.com>
> > > 
> > > 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@kernel.org>
> 
> FWIW: the approaches that are actually used and work well in Linux are
> ICE and the CPU-accelerated crypto.
> 
> - Eric

-- 
With best wishes
Dmitry

Re: [PATCH 3/3] arm64: dts: qcom: shikra: Add qcrypto node support

[Kuldeep Singh]

>>> These two entries are logically the same (SID & ~mask) as the first two,
>>> does it still work if you remove them?
>>
>> Yes, resulting sid is same for 84/94 and 86/92.
>> Basically, the resulting sid could be same, it's an optimization which
>> smmu is doing which can result in same SMR(Stream matching register)
>> routing 2 different sid to same context bank.
>> So, 2 sid can be used even though resulting sid remains same.
>>
>> Also, DT usually dictates what hw capabilities are supported and hence,
>> captured all apps entries here to match the hardware description.
>>
>> I hope this answers your query.
> 
> It doesn't. Can we drop them?

Could you please explain more on what's missing?

-- 
Regards
Kuldeep

Re: [PATCH 3/3] arm64: dts: qcom: shikra: Add qcrypto node support

[Dmitry Baryshkov]

On Mon, May 25, 2026 at 03:39:17PM +0530, Kuldeep Singh wrote:
> >>> These two entries are logically the same (SID & ~mask) as the first two,
> >>> does it still work if you remove them?
> >>
> >> Yes, resulting sid is same for 84/94 and 86/92.
> >> Basically, the resulting sid could be same, it's an optimization which
> >> smmu is doing which can result in same SMR(Stream matching register)
> >> routing 2 different sid to same context bank.
> >> So, 2 sid can be used even though resulting sid remains same.
> >>
> >> Also, DT usually dictates what hw capabilities are supported and hence,
> >> captured all apps entries here to match the hardware description.
> >>
> >> I hope this answers your query.
> > 
> > It doesn't. Can we drop them?
> 
> Could you please explain more on what's missing?

Usually we don't have duplciate SIDs in DT. Why is it not the case for
this device?

-- 
With best wishes
Dmitry

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

[Eric Biggers]

On Mon, May 25, 2026 at 11:10:10AM +0530, Kuldeep Singh wrote:
> > 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.
> > 
> > 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.
> >
> 
> +Bartosz, Gaurav, Neeraj
> 
> Hi Eric,
> 
> GPCE is relevant in terms of providing hardware security.
> There are multiple usecases coming up for example to handle DRM/secure
> buffer usecases to improve overall throughput for secure content.
> 
> Regarding performance, it's currently slower compared to arm CE but
> provides an edge by giving hardware security which is considered more
> secure.
> 
> Btw, there's been performance improvement with new targets and we are
> expecting to achieve far more better performance with new SoCs family.
> Pakala:    GPCE - 550MBps, ARMv8 - 8GBps
> Kaanapali: GPCE - 3GBps,   ARMv8 - 10GBps
> 
> Please note, there's almost 5x improvement in kaanapali compared to
> pakala. Though overall is still slower compared to arm but as mentioned,
> expecting better performance with hardware improvements as we progress.
> 
> Also, currently qce driver exhibit stability issues and that's what we
> are putting effort in stabilizing the software on immediate basis.
> 
> There's parallel effort ongoing by Bartosz to introduce baseline for
> secure buffer usecases.
> https://lore.kernel.org/lkml/20260522-qcom-qce-cmd-descr-v18-0-99103926bafc@oss.qualcomm.com/
> There's active development ongoing and i believe lowering cra_priority
> for qce is fine as of now and can scale values once qce becomes
> performance efficient.
> 
> Please share your thoughts. Thanks!

ARMv8 Crypto Extensions are "hardware" as well, just in the CPU.  They
provide constant-time execution, for example.

Granted, they don't protect from power analysis and electromagnetic
emanation attacks.  Does QCE actually provide those protections, though?

Either way, it doesn't really matter in this case.  There are multiple
aspects to security, and before even considering these advanced
protections, the basics of security need to be absolutely solid.  That
is, the driver needs to always compute the crypto algorithms correctly,
and it needs to be completely robust when fuzzed by unprivileged
userspace (because it can accessed in that way).

Yet, this driver "exhibits stability issues", fails the self-tests, and
doesn't even have exclusive access to the hardware!  These are all
security bugs.  That very much defeats the claimed point.  (Plus, due to
the performance issues no one wants to use it in Linux anyway.)

As for "decrypting into secure buffers": if added that would be a new
feature, separate from the driver's current features.  It's not even
supported by the crypto API.  So regardless of whether this would be a
useful feature or not (it's unclear it would be), using it as an
argument that the current features of the driver are useful is nonsense.

As for performance getting higher over time, it's still irrelevant when
it's still much slower than the CPU, especially in practical conditions.
Yet, somehow this driver is documented as an "accelerator":

        config CRYPTO_DEV_QCE                                                            
            tristate "Qualcomm crypto engine accelerator"

The CPU is just much a better approach performance-wise.  It has no
overhead in setting up DMA, waking/notifying the hardware, and context
switching.  The CPU has a lot of room to improve too, via further
optimizations to hardware and the ISA, and in some cases software
optimizations such as interleaving.  We've already seen this play out on
x86_64, where Vector AES boosted the AES throughput by a further 2-4x
from its already-super-fast performance.

- Eric

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

[Eric Biggers]

On Mon, May 25, 2026 at 01:07:49PM +0300, Dmitry Baryshkov wrote:
> 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.

Yes, QCE is hardly unique here.  It's just the one we're discussing now.

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

Only a few drivers support protected keys ("paes", "phmac"); QCE is
*not* one of them.  There are also no explicit users of protected keys
in the kernel, so even if supported by the driver, it's almost never
used in practice in Linux.  The only way this feature could potentially
be used in Linux is if one of these drivers is present *and* userspace
explicitly chooses to use it with one of the few kernel features that
might implicitly support it, e.g. dm-crypt.  AFAIK that's extremely
rare, and at least in Linux it's really just a checkbox feature.

(HW-wrapped inline crypto keys do get used, but those don't use the
crypto API or these drivers at all.)

As for making it "impossible to tamper with the hash values before
signing/verification", these drivers don't provide anything there.

- Eric

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

[Eric Biggers]

On Mon, May 25, 2026 at 09:28:43AM -0500, Eric Biggers wrote:
> ARMv8 Crypto Extensions are "hardware" as well, just in the CPU.  They
> provide constant-time execution, for example.
> 
> Granted, they don't protect from power analysis and electromagnetic
> emanation attacks.  Does QCE actually provide those protections, though?
> 
> Either way, it doesn't really matter in this case.  There are multiple
> aspects to security, and before even considering these advanced
> protections, the basics of security need to be absolutely solid.  That
> is, the driver needs to always compute the crypto algorithms correctly,
> and it needs to be completely robust when fuzzed by unprivileged
> userspace (because it can accessed in that way).

Looks like these protections are not even present either.  From
https://csrc.nist.gov/CSRC/media/projects/cryptographic-module-validation-program/documents/security-policies/140sp5077.pdf :

    > The Qualcomm Crypto Engine Core does not support any non-invasive
    > security techniques. Therefore, this section is not applicable.
    [...]
    > The Qualcomm Crypto Engine Core does not implement security
    > mechanisms to mitigate other attacks.

- Eric