All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Add Crypto Engine support for SM6350
@ 2024-01-05 16:15 Luca Weiss
  2024-01-05 16:15 ` [PATCH 1/2] dt-bindings: qcom-qce: Add compatible " Luca Weiss
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Luca Weiss @ 2024-01-05 16:15 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Thara Gopinath,
	Herbert Xu, David S. Miller, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bhupesh Sharma
  Cc: ~postmarketos/upstreaming, phone-devel, linux-crypto,
	linux-arm-msm, devicetree, linux-kernel, Luca Weiss

Add the compatible and nodes for the QCE found on SM6350 SoC.

Not completely sure how to fully test it but "kcapi-speed --all" shows
no issues. Let me know if I can/should test this more.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
Luca Weiss (2):
      dt-bindings: qcom-qce: Add compatible for SM6350
      arm64: dts: qcom: sm6350: Add Crypto Engine

 .../devicetree/bindings/crypto/qcom-qce.yaml       |  1 +
 arch/arm64/boot/dts/qcom/sm6350.dtsi               | 31 ++++++++++++++++++++++
 2 files changed, 32 insertions(+)
---
base-commit: 610a9b8f49fbcf1100716370d3b5f6f884a2835a
change-id: 20240105-sm6350-qce-c6233abbf54f

Best regards,
-- 
Luca Weiss <luca.weiss@fairphone.com>


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] dt-bindings: qcom-qce: Add compatible for SM6350
  2024-01-05 16:15 [PATCH 0/2] Add Crypto Engine support for SM6350 Luca Weiss
@ 2024-01-05 16:15 ` Luca Weiss
  2024-01-06 17:12   ` Krzysztof Kozlowski
  2024-01-26  8:59   ` Herbert Xu
  2024-01-05 16:15 ` [PATCH 2/2] arm64: dts: qcom: sm6350: Add Crypto Engine Luca Weiss
  2024-01-08 12:40 ` [PATCH 0/2] Add Crypto Engine support for SM6350 Konrad Dybcio
  2 siblings, 2 replies; 13+ messages in thread
From: Luca Weiss @ 2024-01-05 16:15 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Thara Gopinath,
	Herbert Xu, David S. Miller, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bhupesh Sharma
  Cc: ~postmarketos/upstreaming, phone-devel, linux-crypto,
	linux-arm-msm, devicetree, linux-kernel, Luca Weiss

Add a compatible for the crypto block found on the SM6350 SoC.

Signed-off-by: Luca Weiss <luca.weiss@fairphone.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 8e665d910e6e..69d1c4929935 100644
--- a/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
+++ b/Documentation/devicetree/bindings/crypto/qcom-qce.yaml
@@ -44,6 +44,7 @@ properties:
 
       - items:
           - enum:
+              - qcom,sm6350-qce
               - qcom,sm8250-qce
               - qcom,sm8350-qce
               - qcom,sm8450-qce

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] arm64: dts: qcom: sm6350: Add Crypto Engine
  2024-01-05 16:15 [PATCH 0/2] Add Crypto Engine support for SM6350 Luca Weiss
  2024-01-05 16:15 ` [PATCH 1/2] dt-bindings: qcom-qce: Add compatible " Luca Weiss
@ 2024-01-05 16:15 ` Luca Weiss
  2024-01-05 16:30   ` Stephan Gerhold
  2024-01-08 12:40 ` [PATCH 0/2] Add Crypto Engine support for SM6350 Konrad Dybcio
  2 siblings, 1 reply; 13+ messages in thread
From: Luca Weiss @ 2024-01-05 16:15 UTC (permalink / raw)
  To: Andy Gross, Bjorn Andersson, Konrad Dybcio, Thara Gopinath,
	Herbert Xu, David S. Miller, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bhupesh Sharma
  Cc: ~postmarketos/upstreaming, phone-devel, linux-crypto,
	linux-arm-msm, devicetree, linux-kernel, Luca Weiss

Add crypto engine (CE) and CE BAM related nodes and definitions for this
SoC.

For reference:

  [    2.297419] qcrypto 1dfa000.crypto: Crypto device found, version 5.5.1

Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
 arch/arm64/boot/dts/qcom/sm6350.dtsi | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi
index 8fd6f4d03490..516aadbb16bb 100644
--- a/arch/arm64/boot/dts/qcom/sm6350.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi
@@ -1212,6 +1212,37 @@ ufs_mem_phy_lanes: phy@1d87400 {
 			};
 		};
 
+		cryptobam: dma-controller@1dc4000 {
+			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
+			reg = <0 0x01dc4000 0 0x24000>;
+			interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			qcom,ee = <0>;
+			qcom,controlled-remotely;
+			num-channels = <16>;
+			qcom,num-ees = <4>;
+			iommus = <&apps_smmu 0x432 0x0000>,
+				 <&apps_smmu 0x438 0x0001>,
+				 <&apps_smmu 0x43f 0x0000>,
+				 <&apps_smmu 0x426 0x0011>,
+				 <&apps_smmu 0x436 0x0011>;
+		};
+
+		crypto: crypto@1dfa000 {
+			compatible = "qcom,sm6350-qce", "qcom,sm8150-qce", "qcom,qce";
+			reg = <0 0x01dfa000 0 0x6000>;
+			dmas = <&cryptobam 4>, <&cryptobam 5>;
+			dma-names = "rx", "tx";
+			iommus = <&apps_smmu 0x432 0x0000>,
+				 <&apps_smmu 0x438 0x0001>,
+				 <&apps_smmu 0x43f 0x0000>,
+				 <&apps_smmu 0x426 0x0011>,
+				 <&apps_smmu 0x436 0x0011>;
+			interconnects = <&aggre2_noc MASTER_CRYPTO_CORE_0 QCOM_ICC_TAG_ALWAYS
+					 &clk_virt SLAVE_EBI_CH0 QCOM_ICC_TAG_ALWAYS>;
+			interconnect-names = "memory";
+		};
+
 		ipa: ipa@1e40000 {
 			compatible = "qcom,sm6350-ipa";
 

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] arm64: dts: qcom: sm6350: Add Crypto Engine
  2024-01-05 16:15 ` [PATCH 2/2] arm64: dts: qcom: sm6350: Add Crypto Engine Luca Weiss
@ 2024-01-05 16:30   ` Stephan Gerhold
  2024-02-16 10:46     ` Luca Weiss
  0 siblings, 1 reply; 13+ messages in thread
From: Stephan Gerhold @ 2024-01-05 16:30 UTC (permalink / raw)
  To: Luca Weiss
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Thara Gopinath,
	Herbert Xu, David S. Miller, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bhupesh Sharma, ~postmarketos/upstreaming,
	phone-devel, linux-crypto, linux-arm-msm, devicetree,
	linux-kernel

On Fri, Jan 05, 2024 at 05:15:44PM +0100, Luca Weiss wrote:
> Add crypto engine (CE) and CE BAM related nodes and definitions for this
> SoC.
> 
> For reference:
> 
>   [    2.297419] qcrypto 1dfa000.crypto: Crypto device found, version 5.5.1
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  arch/arm64/boot/dts/qcom/sm6350.dtsi | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi
> index 8fd6f4d03490..516aadbb16bb 100644
> --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi
> @@ -1212,6 +1212,37 @@ ufs_mem_phy_lanes: phy@1d87400 {
>  			};
>  		};
>  
> +		cryptobam: dma-controller@1dc4000 {
> +			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
> +			reg = <0 0x01dc4000 0 0x24000>;
> +			interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
> +			#dma-cells = <1>;
> +			qcom,ee = <0>;
> +			qcom,controlled-remotely;
> +			num-channels = <16>;
> +			qcom,num-ees = <4>;
> +			iommus = <&apps_smmu 0x432 0x0000>,
> +				 <&apps_smmu 0x438 0x0001>,
> +				 <&apps_smmu 0x43f 0x0000>,
> +				 <&apps_smmu 0x426 0x0011>,
> +				 <&apps_smmu 0x436 0x0011>;

The last two lines look equivalent to me: 0x436 & ~0x0011 = 0x426.

It's also a bit weird that the mask has one more digit than the stream
ID. And ordered numerically (by stream ID, first number) it would be a
bit easier to read. :-)

Thanks,
Stephan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] dt-bindings: qcom-qce: Add compatible for SM6350
  2024-01-05 16:15 ` [PATCH 1/2] dt-bindings: qcom-qce: Add compatible " Luca Weiss
@ 2024-01-06 17:12   ` Krzysztof Kozlowski
  2024-01-26  8:59   ` Herbert Xu
  1 sibling, 0 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2024-01-06 17:12 UTC (permalink / raw)
  To: Luca Weiss, Andy Gross, Bjorn Andersson, Konrad Dybcio,
	Thara Gopinath, Herbert Xu, David S. Miller, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bhupesh Sharma
  Cc: ~postmarketos/upstreaming, phone-devel, linux-crypto,
	linux-arm-msm, devicetree, linux-kernel

On 05/01/2024 17:15, Luca Weiss wrote:
> Add a compatible for the crypto block found on the SM6350 SoC.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Add Crypto Engine support for SM6350
  2024-01-05 16:15 [PATCH 0/2] Add Crypto Engine support for SM6350 Luca Weiss
  2024-01-05 16:15 ` [PATCH 1/2] dt-bindings: qcom-qce: Add compatible " Luca Weiss
  2024-01-05 16:15 ` [PATCH 2/2] arm64: dts: qcom: sm6350: Add Crypto Engine Luca Weiss
@ 2024-01-08 12:40 ` Konrad Dybcio
  2024-01-09 11:27   ` Luca Weiss
  2 siblings, 1 reply; 13+ messages in thread
From: Konrad Dybcio @ 2024-01-08 12:40 UTC (permalink / raw)
  To: Luca Weiss, Andy Gross, Bjorn Andersson, Thara Gopinath,
	Herbert Xu, David S. Miller, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bhupesh Sharma
  Cc: ~postmarketos/upstreaming, phone-devel, linux-crypto,
	linux-arm-msm, devicetree, linux-kernel

On 5.01.2024 17:15, Luca Weiss wrote:
> Add the compatible and nodes for the QCE found on SM6350 SoC.
> 
> Not completely sure how to fully test it but "kcapi-speed --all" shows
> no issues. Let me know if I can/should test this more.

I think I used `cryptsetup benchmark` with and without the ICE enabled
a couple years back. IIRC the CPU should be faaar faster but also chug
power while at it.

Konrad

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Add Crypto Engine support for SM6350
  2024-01-08 12:40 ` [PATCH 0/2] Add Crypto Engine support for SM6350 Konrad Dybcio
@ 2024-01-09 11:27   ` Luca Weiss
  2024-01-10 10:55     ` Konrad Dybcio
  0 siblings, 1 reply; 13+ messages in thread
From: Luca Weiss @ 2024-01-09 11:27 UTC (permalink / raw)
  To: Konrad Dybcio, Andy Gross, Bjorn Andersson, Thara Gopinath,
	Herbert Xu, David S. Miller, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bhupesh Sharma
  Cc: ~postmarketos/upstreaming, phone-devel, linux-crypto,
	linux-arm-msm, devicetree, linux-kernel

On Mon Jan 8, 2024 at 1:40 PM CET, Konrad Dybcio wrote:
> On 5.01.2024 17:15, Luca Weiss wrote:
> > Add the compatible and nodes for the QCE found on SM6350 SoC.
> > 
> > Not completely sure how to fully test it but "kcapi-speed --all" shows
> > no issues. Let me know if I can/should test this more.
>
> I think I used `cryptsetup benchmark` with and without the ICE enabled
> a couple years back. IIRC the CPU should be faaar faster but also chug
> power while at it.

Are you sure you mean QCE here (which this patch is about) and not ICE?

I'm not aware of them working together somehow but I wouldn't be
surprised if there's something since I don't know much of this area at
all.

Regards
Luca

>
> Konrad


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Add Crypto Engine support for SM6350
  2024-01-09 11:27   ` Luca Weiss
@ 2024-01-10 10:55     ` Konrad Dybcio
  0 siblings, 0 replies; 13+ messages in thread
From: Konrad Dybcio @ 2024-01-10 10:55 UTC (permalink / raw)
  To: Luca Weiss, Andy Gross, Bjorn Andersson, Thara Gopinath,
	Herbert Xu, David S. Miller, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bhupesh Sharma
  Cc: ~postmarketos/upstreaming, phone-devel, linux-crypto,
	linux-arm-msm, devicetree, linux-kernel



On 1/9/24 12:27, Luca Weiss wrote:
> On Mon Jan 8, 2024 at 1:40 PM CET, Konrad Dybcio wrote:
>> On 5.01.2024 17:15, Luca Weiss wrote:
>>> Add the compatible and nodes for the QCE found on SM6350 SoC.
>>>
>>> Not completely sure how to fully test it but "kcapi-speed --all" shows
>>> no issues. Let me know if I can/should test this more.
>>
>> I think I used `cryptsetup benchmark` with and without the ICE enabled
>> a couple years back. IIRC the CPU should be faaar faster but also chug
>> power while at it.
> 
> Are you sure you mean QCE here (which this patch is about) and not ICE?

I.. think I do. It's been a while.

> 
> I'm not aware of them working together somehow but I wouldn't be
> surprised if there's something since I don't know much of this area at
> all.

No idea

Konrad

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] dt-bindings: qcom-qce: Add compatible for SM6350
  2024-01-05 16:15 ` [PATCH 1/2] dt-bindings: qcom-qce: Add compatible " Luca Weiss
  2024-01-06 17:12   ` Krzysztof Kozlowski
@ 2024-01-26  8:59   ` Herbert Xu
  1 sibling, 0 replies; 13+ messages in thread
From: Herbert Xu @ 2024-01-26  8:59 UTC (permalink / raw)
  To: Luca Weiss
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Thara Gopinath,
	David S. Miller, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Bhupesh Sharma, ~postmarketos/upstreaming, phone-devel,
	linux-crypto, linux-arm-msm, devicetree, linux-kernel

On Fri, Jan 05, 2024 at 05:15:43PM +0100, Luca Weiss wrote:
> Add a compatible for the crypto block found on the SM6350 SoC.
> 
> Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> ---
>  Documentation/devicetree/bindings/crypto/qcom-qce.yaml | 1 +
>  1 file changed, 1 insertion(+)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] arm64: dts: qcom: sm6350: Add Crypto Engine
  2024-01-05 16:30   ` Stephan Gerhold
@ 2024-02-16 10:46     ` Luca Weiss
  2024-02-16 12:27       ` Stephan Gerhold
  2024-02-16 18:09       ` Bjorn Andersson
  0 siblings, 2 replies; 13+ messages in thread
From: Luca Weiss @ 2024-02-16 10:46 UTC (permalink / raw)
  To: Stephan Gerhold
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Thara Gopinath,
	Herbert Xu, David S. Miller, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bhupesh Sharma, ~postmarketos/upstreaming,
	phone-devel, linux-crypto, linux-arm-msm, devicetree,
	linux-kernel

On Fri Jan 5, 2024 at 5:30 PM CET, Stephan Gerhold wrote:
> On Fri, Jan 05, 2024 at 05:15:44PM +0100, Luca Weiss wrote:
> > Add crypto engine (CE) and CE BAM related nodes and definitions for this
> > SoC.
> > 
> > For reference:
> > 
> >   [    2.297419] qcrypto 1dfa000.crypto: Crypto device found, version 5.5.1
> > 
> > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > ---
> >  arch/arm64/boot/dts/qcom/sm6350.dtsi | 31 +++++++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi
> > index 8fd6f4d03490..516aadbb16bb 100644
> > --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi
> > @@ -1212,6 +1212,37 @@ ufs_mem_phy_lanes: phy@1d87400 {
> >  			};
> >  		};
> >  
> > +		cryptobam: dma-controller@1dc4000 {
> > +			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
> > +			reg = <0 0x01dc4000 0 0x24000>;
> > +			interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
> > +			#dma-cells = <1>;
> > +			qcom,ee = <0>;
> > +			qcom,controlled-remotely;
> > +			num-channels = <16>;
> > +			qcom,num-ees = <4>;
> > +			iommus = <&apps_smmu 0x432 0x0000>,
> > +				 <&apps_smmu 0x438 0x0001>,
> > +				 <&apps_smmu 0x43f 0x0000>,
> > +				 <&apps_smmu 0x426 0x0011>,
> > +				 <&apps_smmu 0x436 0x0011>;
>
> The last two lines look equivalent to me: 0x436 & ~0x0011 = 0x426.

I don't understand the IOMMU SID + mask really, but I think I've seen
somewhere before like here that TZ can be a bit picky with the SIDs?

https://lore.kernel.org/linux-arm-msm/opqdrmyj3y64nqqqmakjydn5rkspizufyeavm7ec7c7ufqz4wk@ey2a7bq3shfj/
https://lore.kernel.org/linux-arm-msm/11b5db69-49f5-4d7b-81c9-687d66a5cb0d@linaro.org/

I don't quite want to risk having some obscure use case breaking because
we cleaned up the dts ;)

But if you're more sure than me that it won't break, let me know!

>
> It's also a bit weird that the mask has one more digit than the stream
> ID. And ordered numerically (by stream ID, first number) it would be a
> bit easier to read. :-)

Sorting them is no problem, can do that for v2.

>
> Thanks,
> Stephan


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] arm64: dts: qcom: sm6350: Add Crypto Engine
  2024-02-16 10:46     ` Luca Weiss
@ 2024-02-16 12:27       ` Stephan Gerhold
  2024-02-16 18:09       ` Bjorn Andersson
  1 sibling, 0 replies; 13+ messages in thread
From: Stephan Gerhold @ 2024-02-16 12:27 UTC (permalink / raw)
  To: Luca Weiss
  Cc: Andy Gross, Bjorn Andersson, Konrad Dybcio, Thara Gopinath,
	Herbert Xu, David S. Miller, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bhupesh Sharma, ~postmarketos/upstreaming,
	phone-devel, linux-crypto, linux-arm-msm, devicetree,
	linux-kernel

On Fri, Feb 16, 2024 at 11:46:49AM +0100, Luca Weiss wrote:
> On Fri Jan 5, 2024 at 5:30 PM CET, Stephan Gerhold wrote:
> > On Fri, Jan 05, 2024 at 05:15:44PM +0100, Luca Weiss wrote:
> > > Add crypto engine (CE) and CE BAM related nodes and definitions for this
> > > SoC.
> > > 
> > > For reference:
> > > 
> > >   [    2.297419] qcrypto 1dfa000.crypto: Crypto device found, version 5.5.1
> > > 
> > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > ---
> > >  arch/arm64/boot/dts/qcom/sm6350.dtsi | 31 +++++++++++++++++++++++++++++++
> > >  1 file changed, 31 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi
> > > index 8fd6f4d03490..516aadbb16bb 100644
> > > --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi
> > > @@ -1212,6 +1212,37 @@ ufs_mem_phy_lanes: phy@1d87400 {
> > >  			};
> > >  		};
> > >  
> > > +		cryptobam: dma-controller@1dc4000 {
> > > +			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
> > > +			reg = <0 0x01dc4000 0 0x24000>;
> > > +			interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
> > > +			#dma-cells = <1>;
> > > +			qcom,ee = <0>;
> > > +			qcom,controlled-remotely;
> > > +			num-channels = <16>;
> > > +			qcom,num-ees = <4>;
> > > +			iommus = <&apps_smmu 0x432 0x0000>,
> > > +				 <&apps_smmu 0x438 0x0001>,
> > > +				 <&apps_smmu 0x43f 0x0000>,
> > > +				 <&apps_smmu 0x426 0x0011>,
> > > +				 <&apps_smmu 0x436 0x0011>;
> >
> > The last two lines look equivalent to me: 0x436 & ~0x0011 = 0x426.
> 
> I don't understand the IOMMU SID + mask really, but I think I've seen
> somewhere before like here that TZ can be a bit picky with the SIDs?
> 
> https://lore.kernel.org/linux-arm-msm/opqdrmyj3y64nqqqmakjydn5rkspizufyeavm7ec7c7ufqz4wk@ey2a7bq3shfj/
> https://lore.kernel.org/linux-arm-msm/11b5db69-49f5-4d7b-81c9-687d66a5cb0d@linaro.org/
> 
> I don't quite want to risk having some obscure use case breaking because
> we cleaned up the dts ;)
> 
> But if you're more sure than me that it won't break, let me know!
> 

I'm afraid I can't really help with this kind of certainty. My knowledge
about proprietary Qualcomm firmware is probably even more limited than
yours. However, my personal feeling is that the "TZ wants X" arguments
are most often just as badly based on superficial knowledge.

In simplified terms, the SMMU has a number of components connected to it
(the crypto BAM, USB controller, UFS, ...). When the components make
memory requests they are identified by a number of Stream IDs (SIDs).
The purpose of "iommus" in the device tree is to describe all SIDs that
belong to a particular device. These SIDs are then all assigned to a
context bank that allows the device to access selected regions in RAM.

It shouldn't matter *how* the SIDs are matched inside the SMMU, as long
as they end up at the correct context bank. The SMMU will look through
the configured Stream Match Registers (SMRs = SID + Mask) to find the
context bank that is assigned to the SID. The docs say "If MASK[i]==1,
ID[i] is ignored.". This means a SMR with ID=0x426 MASK=0x0011 is
definitely identical to ID=0x436 MASK=0x0011. Having the extra entry
will make absolutely no difference to the SMMU aside from wasting a
pointless SMR.

The links you posted suggest "TZ" looks at the SMRs allocated by Linux.
If that is really the case then I would expect that to be fundamentally
broken. In my opinion there is absolutely no guarantee how or in which
order Linux allocates the SMRs. Such functionality would either be
extremely complex or broken in tons of edge cases.

TL;DR: I cannot provide proof that removing this entry makes a
difference. I can just say that I doubt it does, and if it does, then we
have far more serious problems. The device tree is supposed to describe
the hardware ("This device makes memory requests with the following
SIDs") and not fundamentally broken peculiarities of the proprietary TZ
firmware ("registers must be programmed exactly with these values").

Thanks,
Stephan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] arm64: dts: qcom: sm6350: Add Crypto Engine
  2024-02-16 10:46     ` Luca Weiss
  2024-02-16 12:27       ` Stephan Gerhold
@ 2024-02-16 18:09       ` Bjorn Andersson
  2024-02-19 10:35         ` Luca Weiss
  1 sibling, 1 reply; 13+ messages in thread
From: Bjorn Andersson @ 2024-02-16 18:09 UTC (permalink / raw)
  To: Luca Weiss
  Cc: Stephan Gerhold, Andy Gross, Konrad Dybcio, Thara Gopinath,
	Herbert Xu, David S. Miller, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bhupesh Sharma, ~postmarketos/upstreaming,
	phone-devel, linux-crypto, linux-arm-msm, devicetree,
	linux-kernel

On Fri, Feb 16, 2024 at 11:46:49AM +0100, Luca Weiss wrote:
> On Fri Jan 5, 2024 at 5:30 PM CET, Stephan Gerhold wrote:
> > On Fri, Jan 05, 2024 at 05:15:44PM +0100, Luca Weiss wrote:
> > > Add crypto engine (CE) and CE BAM related nodes and definitions for this
> > > SoC.
> > > 
> > > For reference:
> > > 
> > >   [    2.297419] qcrypto 1dfa000.crypto: Crypto device found, version 5.5.1
> > > 
> > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > ---
> > >  arch/arm64/boot/dts/qcom/sm6350.dtsi | 31 +++++++++++++++++++++++++++++++
> > >  1 file changed, 31 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi
> > > index 8fd6f4d03490..516aadbb16bb 100644
> > > --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi
> > > @@ -1212,6 +1212,37 @@ ufs_mem_phy_lanes: phy@1d87400 {
> > >  			};
> > >  		};
> > >  
> > > +		cryptobam: dma-controller@1dc4000 {
> > > +			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
> > > +			reg = <0 0x01dc4000 0 0x24000>;
> > > +			interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
> > > +			#dma-cells = <1>;
> > > +			qcom,ee = <0>;
> > > +			qcom,controlled-remotely;
> > > +			num-channels = <16>;
> > > +			qcom,num-ees = <4>;
> > > +			iommus = <&apps_smmu 0x432 0x0000>,
> > > +				 <&apps_smmu 0x438 0x0001>,
> > > +				 <&apps_smmu 0x43f 0x0000>,
> > > +				 <&apps_smmu 0x426 0x0011>,
> > > +				 <&apps_smmu 0x436 0x0011>;
> >
> > The last two lines look equivalent to me: 0x436 & ~0x0011 = 0x426.
> 
> I don't understand the IOMMU SID + mask really, but I think I've seen
> somewhere before like here that TZ can be a bit picky with the SIDs?
> 
> https://lore.kernel.org/linux-arm-msm/opqdrmyj3y64nqqqmakjydn5rkspizufyeavm7ec7c7ufqz4wk@ey2a7bq3shfj/
> https://lore.kernel.org/linux-arm-msm/11b5db69-49f5-4d7b-81c9-687d66a5cb0d@linaro.org/
> 
> I don't quite want to risk having some obscure use case breaking because
> we cleaned up the dts ;)
> 
> But if you're more sure than me that it won't break, let me know!
> 
> >
> > It's also a bit weird that the mask has one more digit than the stream
> > ID. And ordered numerically (by stream ID, first number) it would be a
> > bit easier to read. :-)
> 
> Sorting them is no problem, can do that for v2.
> 

Where you able to do this? I don't see a v2 in my inbox, am I just
searching poorly?

Regards,
Bjorn

> >
> > Thanks,
> > Stephan
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] arm64: dts: qcom: sm6350: Add Crypto Engine
  2024-02-16 18:09       ` Bjorn Andersson
@ 2024-02-19 10:35         ` Luca Weiss
  0 siblings, 0 replies; 13+ messages in thread
From: Luca Weiss @ 2024-02-19 10:35 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Stephan Gerhold, Andy Gross, Konrad Dybcio, Thara Gopinath,
	Herbert Xu, David S. Miller, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Bhupesh Sharma, ~postmarketos/upstreaming,
	phone-devel, linux-crypto, linux-arm-msm, devicetree,
	linux-kernel

On Fri Feb 16, 2024 at 7:09 PM CET, Bjorn Andersson wrote:
> On Fri, Feb 16, 2024 at 11:46:49AM +0100, Luca Weiss wrote:
> > On Fri Jan 5, 2024 at 5:30 PM CET, Stephan Gerhold wrote:
> > > On Fri, Jan 05, 2024 at 05:15:44PM +0100, Luca Weiss wrote:
> > > > Add crypto engine (CE) and CE BAM related nodes and definitions for this
> > > > SoC.
> > > > 
> > > > For reference:
> > > > 
> > > >   [    2.297419] qcrypto 1dfa000.crypto: Crypto device found, version 5.5.1
> > > > 
> > > > Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
> > > > ---
> > > >  arch/arm64/boot/dts/qcom/sm6350.dtsi | 31 +++++++++++++++++++++++++++++++
> > > >  1 file changed, 31 insertions(+)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/qcom/sm6350.dtsi b/arch/arm64/boot/dts/qcom/sm6350.dtsi
> > > > index 8fd6f4d03490..516aadbb16bb 100644
> > > > --- a/arch/arm64/boot/dts/qcom/sm6350.dtsi
> > > > +++ b/arch/arm64/boot/dts/qcom/sm6350.dtsi
> > > > @@ -1212,6 +1212,37 @@ ufs_mem_phy_lanes: phy@1d87400 {
> > > >  			};
> > > >  		};
> > > >  
> > > > +		cryptobam: dma-controller@1dc4000 {
> > > > +			compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
> > > > +			reg = <0 0x01dc4000 0 0x24000>;
> > > > +			interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
> > > > +			#dma-cells = <1>;
> > > > +			qcom,ee = <0>;
> > > > +			qcom,controlled-remotely;
> > > > +			num-channels = <16>;
> > > > +			qcom,num-ees = <4>;
> > > > +			iommus = <&apps_smmu 0x432 0x0000>,
> > > > +				 <&apps_smmu 0x438 0x0001>,
> > > > +				 <&apps_smmu 0x43f 0x0000>,
> > > > +				 <&apps_smmu 0x426 0x0011>,
> > > > +				 <&apps_smmu 0x436 0x0011>;
> > >
> > > The last two lines look equivalent to me: 0x436 & ~0x0011 = 0x426.
> > 
> > I don't understand the IOMMU SID + mask really, but I think I've seen
> > somewhere before like here that TZ can be a bit picky with the SIDs?
> > 
> > https://lore.kernel.org/linux-arm-msm/opqdrmyj3y64nqqqmakjydn5rkspizufyeavm7ec7c7ufqz4wk@ey2a7bq3shfj/
> > https://lore.kernel.org/linux-arm-msm/11b5db69-49f5-4d7b-81c9-687d66a5cb0d@linaro.org/
> > 
> > I don't quite want to risk having some obscure use case breaking because
> > we cleaned up the dts ;)
> > 
> > But if you're more sure than me that it won't break, let me know!
> > 
> > >
> > > It's also a bit weird that the mask has one more digit than the stream
> > > ID. And ordered numerically (by stream ID, first number) it would be a
> > > bit easier to read. :-)
> > 
> > Sorting them is no problem, can do that for v2.
> > 
>
> Where you able to do this? I don't see a v2 in my inbox, am I just
> searching poorly?

Only sent v2 some minutes ago, didn't have any more time on Friday.

Regards
Luca

>
> Regards,
> Bjorn
>
> > >
> > > Thanks,
> > > Stephan
> > 


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-02-19 10:35 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-05 16:15 [PATCH 0/2] Add Crypto Engine support for SM6350 Luca Weiss
2024-01-05 16:15 ` [PATCH 1/2] dt-bindings: qcom-qce: Add compatible " Luca Weiss
2024-01-06 17:12   ` Krzysztof Kozlowski
2024-01-26  8:59   ` Herbert Xu
2024-01-05 16:15 ` [PATCH 2/2] arm64: dts: qcom: sm6350: Add Crypto Engine Luca Weiss
2024-01-05 16:30   ` Stephan Gerhold
2024-02-16 10:46     ` Luca Weiss
2024-02-16 12:27       ` Stephan Gerhold
2024-02-16 18:09       ` Bjorn Andersson
2024-02-19 10:35         ` Luca Weiss
2024-01-08 12:40 ` [PATCH 0/2] Add Crypto Engine support for SM6350 Konrad Dybcio
2024-01-09 11:27   ` Luca Weiss
2024-01-10 10:55     ` Konrad Dybcio

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.