| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <CY0P173KMGJY.25N4DSRYNK5A7@fairphone.com>
Date: Fri, 29 Dec 2023 10:00:54 +0100
From: "Luca Weiss" <luca.weiss@...rphone.com>
To: "Luca Weiss" <luca.weiss@...rphone.com>, "Om Prakash Singh"
<quic_omprsing@...cinc.com>
Cc: <neil.armstrong@...aro.org>, <konrad.dybcio@...aro.org>,
<agross@...nel.org>, <andersson@...nel.org>, <conor+dt@...nel.org>,
<davem@...emloft.net>, <devicetree@...r.kernel.org>,
<herbert@...dor.apana.org.au>, <krzysztof.kozlowski+dt@...aro.org>,
<linux-arm-msm@...r.kernel.org>, <linux-crypto@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <marijn.suijten@...ainline.org>,
<robh+dt@...nel.org>, <vkoul@...nel.org>,
<cros-qcom-dts-watchers@...omium.org>
Subject: Re: [PATCH V3 2/2] arm64: dts: qcom: sc7280: add QCrypto nodes
On Thu Dec 28, 2023 at 3:29 PM CET, Luca Weiss wrote:
> On Thu Dec 14, 2023 at 11:36 AM CET, Om Prakash Singh wrote:
> > Add the QCE and Crypto BAM DMA nodes.
> >
> > Signed-off-by: Om Prakash Singh <quic_omprsing@...cinc.com>
> > ---
> >
> > Changes in V3:
> > - V2 patch was sent without actual modification. Resending the patch with modified file.
> >
> > Changes in V2:
> > - Update DT node sequence as per register ascending order.
> > - Fix DT node properties as per convention.
> >
> > arch/arm64/boot/dts/qcom/sc7280.dtsi | 22 ++++++++++++++++++++++
> > 1 file changed, 22 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > index 66f1eb83cca7..b819724c1255 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> > @@ -2233,6 +2233,28 @@ pcie1_phy: phy@...e000 {
> > status = "disabled";
> > };
> >
> > + cryptobam: dma-controller@...4000 {
> > + compatible = "qcom,bam-v1.7.4", "qcom,bam-v1.7.0";
> > + reg = <0x0 0x01dc4000 0x0 0x28000>;
> > + interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
> > + #dma-cells = <1>;
> > + iommus = <&apps_smmu 0x4e4 0x0011>,
> > + <&apps_smmu 0x4e6 0x0011>;
> > + qcom,ee = <0>;
> > + qcom,controlled-remotely;
> > + };
>
> Hi,
>
> Unfortunately I seem to have boot failure / device crash with cryptobam
> enabled on my qcm6490-fairphone-fp5. Are you aware of any firmware
> differences that could cause this with QCM6490 LA firmware?
>
> Looking at downstream msm-5.4 dmesg I do see this BAM being used so it
> should generally be accessible from Linux.
>
> [ 5.217214] qce 1de0000.qcedev: Adding to iommu group 18
> [ 5.223741] QCE50: __qce_get_device_tree_data: CE operating frequency is not defined, setting to default 100MHZ
> [ 5.234986] qce 1de0000.qcedev: QTI Crypto 5.6.0 device found @0x1de0000
> [ 5.242981] sps_register_bam_device: sps:BAM 0x0000000001dc4000 is registered
> [ 5.251124] sps_bam_enable: sps:BAM 0x0000000001dc4000 (va:0x000000001db63156) enabled: ver:0x27, number of pipes:16
> [ 5.262783] QCE50: qce_sps_init: QTI MSM CE-BAM at 0x0000000001dc4000 irq 9
> [ 5.271820] qce 1de0000.qcedev:qcom_cedev_ns_cb: Adding to iommu group 19
> [ 5.281083] qce 1de0000.qcedev:qcom_cedev_s_cb: Adding to iommu group 20
> [ 5.289376] qcrypto 1de0000.qcrypto: Adding to iommu group 21
> [ 5.296326] QCE50: __qce_get_device_tree_data: CE operating frequency is not defined, setting to default 100MHZ
> [ 5.307675] qcrypto 1de0000.qcrypto: QTI Crypto 5.6.0 device found @0x1de0000
> [ 5.315867] QCE50: qce_sps_init: QTI MSM CE-BAM at 0x0000000001dc4000 irq 9
After some hints from Stephan Gerhold, it looks like when the bam driver
tries to read the registers for getting num-ees and num-channels we
need to have the interconnect up, otherwise the register read fails and
resets the device into EDL mode.
After applying the patch I've attached below the registers can be read
successfully.
The patch I've actually sent now[0] adds the values read from these
registers as static properties so that during cryptobam probe we don't
need to have the interconnect up because we don't read any registers.
Stephan Gerhold mentioned off-list that this is an adequate solution
because all the other times the cryptobam is used by the &crypto node
the interconnect will be up.
[0] https://lore.kernel.org/linux-arm-msm/20231229-sc7280-cryptobam-fixup-v1-1-bd8f68589b80@fairphone.com/T/#u
I imagine the cause why cryptobam & crypto are disabled in sm8350.dtsi
is the same, so the solution there probably also is just hardcoding
those properties and then using &crypto should work as expected.
Regards
Luca
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 83b5b76ba179..b2517631e884 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2345,6 +2345,9 @@ cryptobam: dma-controller@...4000 {
<&apps_smmu 0x4e6 0x0011>;
qcom,ee = <0>;
qcom,controlled-remotely;
+ interconnects = <&aggre2_noc MASTER_CRYPTO QCOM_ICC_TAG_ALWAYS
+ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
+ interconnect-names = "memory";
};
crypto: crypto@...a000 {
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 5e7d332731e0..b2f85c6bcbea 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -40,6 +40,7 @@
#include <linux/circ_buf.h>
#include <linux/clk.h>
#include <linux/dmaengine.h>
+#include <linux/interconnect.h>
#include <linux/pm_runtime.h>
#include "../dmaengine.h"
@@ -394,6 +395,7 @@ struct bam_device {
const struct reg_offset_data *layout;
struct clk *bamclk;
+ struct icc_path *mem_path;
int irq;
/* dma start transaction tasklet */
@@ -1298,6 +1300,14 @@ static int bam_dma_probe(struct platform_device *pdev)
return ret;
}
+ bdev->mem_path = devm_of_icc_get(bdev->dev, "memory");
+ if (IS_ERR(bdev->mem_path))
+ return PTR_ERR(bdev->mem_path);
+
+ ret = icc_set_bw(bdev->mem_path, 1, 1);
+ if (ret)
+ return ret;
+
ret = bam_init(bdev);
if (ret)
goto err_disable_clk;
Powered by blists - more mailing lists