[PATCH v2 00/16] Add cmd descriptor support

[PATCH v2 00/16] Add cmd descriptor support

[Md Sadre Alam]

This series of patches will add command descriptor
support to read/write crypto engine register via
BAM/DMA

We need this support because if there is multiple EE's
(Execution Environment) accessing the same CE then there
will be race condition. To avoid this race condition
BAM HW hsving LOC/UNLOCK feature on BAM pipes and this
LOCK/UNLOCK will be set via command descriptor only.

Since each EE's having their dedicated BAM pipe, BAM allows
Locking and Unlocking on BAM pipe. So if one EE's requesting
for CE5 access then that EE's first has to LOCK the BAM pipe
while setting LOCK bit on command descriptor and then access
it. After finishing the request EE's has to UNLOCK the BAM pipe
so in this way we race condition will not happen.

tested with "tcrypt.ko" and "kcapi" tool.

Need help to test these all the patches on msm platform

v2:
 * Addressed all the comments from v1
 * Added the dt-binding
 * Added locking/unlocking mechanism in bam driver

v1:
 * https://lore.kernel.org/lkml/20231214114239.2635325-1-quic_mdalam@quicinc.com/
 * Initial set of patches for cmd descriptor support

Md Sadre Alam (16):
  dt-bindings: dma: qcom,bam: Add bam pipe lock
  dmaengine: qcom: bam_dma: add bam_pipe_lock dt property
  dmaengine: qcom: bam_dma: add LOCK & UNLOCK flag support
  crypto: qce - Add support for crypto address read
  crypto: qce - Add bam dma support for crypto register r/w
  crypto: qce - Convert register r/w for skcipher via BAM/DMA
  crypto: qce - Convert register r/w for sha via BAM/DMA
  crypto: qce - Convert register r/w for aead via BAM/DMA
  crypto: qce - Add LOCK and UNLOCK flag support
  crypto: qce - Add support for lock aquire,lock release api.
  crypto: qce - Add support for lock/unlock in skcipher
  crypto: qce - Add support for lock/unlock in sha
  crypto: qce - Add support for lock/unlock in aead
  arm64: dts: qcom: ipq9574: enable bam pipe locking/unlocking
  arm64: dts: qcom: ipq8074: enable bam pipe locking/unlocking
  arm64: dts: qcom: ipq6018: enable bam pipe locking/unlocking

 .../devicetree/bindings/dma/qcom,bam-dma.yaml |   8 +
 arch/arm64/boot/dts/qcom/ipq6018.dtsi         |   1 +
 arch/arm64/boot/dts/qcom/ipq8074.dtsi         |   1 +
 arch/arm64/boot/dts/qcom/ipq9574.dtsi         |   1 +
 drivers/crypto/qce/aead.c                     |   4 +
 drivers/crypto/qce/common.c                   | 142 +++++++----
 drivers/crypto/qce/core.c                     |  13 +-
 drivers/crypto/qce/core.h                     |  12 +
 drivers/crypto/qce/dma.c                      | 232 ++++++++++++++++++
 drivers/crypto/qce/dma.h                      |  26 +-
 drivers/crypto/qce/sha.c                      |   4 +
 drivers/crypto/qce/skcipher.c                 |   4 +
 drivers/dma/qcom/bam_dma.c                    |  14 +-
 include/linux/dmaengine.h                     |   6 +
 14 files changed, 424 insertions(+), 44 deletions(-)

-- 
2.34.1

[PATCH v2 01/16] dt-bindings: dma: qcom,bam: Add bam pipe lock

[Md Sadre Alam]

BAM having pipe locking mechanism. The Lock and Un-Lock bit
should be set on CMD descriptor only. Upon encountering a
descriptor with Lock bit set, the BAM will lock all other
pipes not related to the current pipe group, and keep
handling the current pipe only until it sees the Un-Lock
set.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v2]

* Added initial support for dt-binding

Change in [v1]

* This patch was not included in [v1]

 Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
index 3ad0d9b1fbc5..91cc2942aa62 100644
--- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
+++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
@@ -77,6 +77,12 @@ properties:
       Indicates that the bam is powered up by a remote processor but must be
       initialized by the local processor.
 
+  qcom,bam_pipe_lock:
+    type: boolean
+    description:
+      Indicates that the bam pipe needs locking or not based on client driver
+      sending the LOCK or UNLOK bit set on command descriptor.
+
   reg:
     maxItems: 1
 
@@ -92,6 +98,8 @@ anyOf:
       - qcom,powered-remotely
   - required:
       - qcom,controlled-remotely
+  - required:
+      - qcom,bam_pipe_lock
   - required:
       - clocks
       - clock-names
-- 
2.34.1

[PATCH v2 02/16] dmaengine: qcom: bam_dma: add bam_pipe_lock dt property

[Md Sadre Alam]

bam having locking and unlocking mechanism of bam pipes.
Upon encountering a descriptor with Lock bit set, the
BAM will lock all other pipes not related to the current
pipe group, and keep handling the current pipe only until
it sees the Un-Lock set , then it will release all locked
pipes. The actual locking is done on the new descriptor
fetching for publishing, i.e. locked pipe will not fetch
new descriptors even if it got event/events adding more
descriptors for this pipe.

Adding the bam_pipe_lock flag in bam driver to handle
Lock and Un-Lock bit set on command descriptor.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v2]

* Added bam_pipe_lock dt property

Change in [v1]

* This patch was not included in [v1]

 drivers/dma/qcom/bam_dma.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 5e7d332731e0..1ac7e250bdaa 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -389,6 +389,7 @@ struct bam_device {
 	u32 ee;
 	bool controlled_remotely;
 	bool powered_remotely;
+	bool bam_pipe_lock;
 	u32 active_channels;
 
 	const struct reg_offset_data *layout;
@@ -1272,6 +1273,9 @@ static int bam_dma_probe(struct platform_device *pdev)
 	bdev->powered_remotely = of_property_read_bool(pdev->dev.of_node,
 						"qcom,powered-remotely");
 
+	bdev->bam_pipe_lock = of_property_read_bool(pdev->dev.of_node,
+						    "qcom,bam_pipe_lock");
+
 	if (bdev->controlled_remotely || bdev->powered_remotely)
 		bdev->bamclk = devm_clk_get_optional(bdev->dev, "bam_clk");
 	else
-- 
2.34.1

[PATCH v2 03/16] dmaengine: qcom: bam_dma: add LOCK & UNLOCK flag support

[Md Sadre Alam]

Add lock and unlock flag support on command descriptor.
Once lock set in requester pipe, then the bam controller
will lock all others pipe and process the request only
from requester pipe. Unlocking only can be performed from
the same pipe.

If DMA_PREP_LOCK flag passed in command descriptor then requester
of this transaction wanted to lock the BAM controller for this
transaction so BAM driver should set LOCK bit for the HW descriptor.

If DMA_PREP_UNLOCK flag passed in command descriptor then requester
of this transaction wanted to unlock the BAM controller.so BAM driver
should set UNLOCK bit for the HW descriptor.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v2]

* Added LOCK and UNLOCK flag in bam driver

Change in [v1]

* This patch was not included in [v1]

 drivers/dma/qcom/bam_dma.c | 10 +++++++++-
 include/linux/dmaengine.h  |  6 ++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 1ac7e250bdaa..ab3b5319aa68 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -58,6 +58,8 @@ struct bam_desc_hw {
 #define DESC_FLAG_EOB BIT(13)
 #define DESC_FLAG_NWD BIT(12)
 #define DESC_FLAG_CMD BIT(11)
+#define DESC_FLAG_LOCK BIT(10)
+#define DESC_FLAG_UNLOCK BIT(9)
 
 struct bam_async_desc {
 	struct virt_dma_desc vd;
@@ -692,9 +694,15 @@ static struct dma_async_tx_descriptor *bam_prep_slave_sg(struct dma_chan *chan,
 		unsigned int curr_offset = 0;
 
 		do {
-			if (flags & DMA_PREP_CMD)
+			if (flags & DMA_PREP_CMD) {
 				desc->flags |= cpu_to_le16(DESC_FLAG_CMD);
 
+				if (bdev->bam_pipe_lock && flags & DMA_PREP_LOCK)
+					desc->flags |= cpu_to_le16(DESC_FLAG_LOCK);
+				else if (bdev->bam_pipe_lock && flags & DMA_PREP_UNLOCK)
+					desc->flags |= cpu_to_le16(DESC_FLAG_UNLOCK);
+			}
+
 			desc->addr = cpu_to_le32(sg_dma_address(sg) +
 						 curr_offset);
 
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index b137fdb56093..70f23068bfdc 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -200,6 +200,10 @@ struct dma_vec {
  *  transaction is marked with DMA_PREP_REPEAT will cause the new transaction
  *  to never be processed and stay in the issued queue forever. The flag is
  *  ignored if the previous transaction is not a repeated transaction.
+ *  @DMA_PREP_LOCK: tell the driver that there is a lock bit set on command
+ *  descriptor.
+ *  @DMA_PREP_UNLOCK: tell the driver that there is a un-lock bit set on command
+ *  descriptor.
  */
 enum dma_ctrl_flags {
 	DMA_PREP_INTERRUPT = (1 << 0),
@@ -212,6 +216,8 @@ enum dma_ctrl_flags {
 	DMA_PREP_CMD = (1 << 7),
 	DMA_PREP_REPEAT = (1 << 8),
 	DMA_PREP_LOAD_EOT = (1 << 9),
+	DMA_PREP_LOCK = (1 << 10),
+	DMA_PREP_UNLOCK = (1 << 11),
 };
 
 /**
-- 
2.34.1

[PATCH v2 04/16] crypto: qce - Add support for crypto address read

[Md Sadre Alam]

Get crypto base address from DT. This will use for
command descriptor support for crypto register r/w
via BAM/DMA

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---
Change in [v2]

* Addressed all comments from v1

Change in [v1]

* Added support to read crypto base address from dt

 drivers/crypto/qce/core.c | 13 ++++++++++++-
 drivers/crypto/qce/core.h |  1 +
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
index 28b5fd823827..9b23a948078a 100644
--- a/drivers/crypto/qce/core.c
+++ b/drivers/crypto/qce/core.c
@@ -192,6 +192,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct qce_device *qce;
+	struct resource *res;
 	int ret;
 
 	qce = devm_kzalloc(dev, sizeof(*qce), GFP_KERNEL);
@@ -201,10 +202,16 @@ static int qce_crypto_probe(struct platform_device *pdev)
 	qce->dev = dev;
 	platform_set_drvdata(pdev, qce);
 
-	qce->base = devm_platform_ioremap_resource(pdev, 0);
+	qce->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
 	if (IS_ERR(qce->base))
 		return PTR_ERR(qce->base);
 
+	qce->base_dma = dma_map_resource(dev, res->start,
+					 resource_size(res),
+					 DMA_BIDIRECTIONAL, 0);
+	if (dma_mapping_error(dev, qce->base_dma))
+		return -ENXIO;
+
 	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
 	if (ret < 0)
 		return ret;
@@ -280,6 +287,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
 static void qce_crypto_remove(struct platform_device *pdev)
 {
 	struct qce_device *qce = platform_get_drvdata(pdev);
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
 	tasklet_kill(&qce->done_tasklet);
 	qce_unregister_algs(qce);
@@ -287,6 +295,9 @@ static void qce_crypto_remove(struct platform_device *pdev)
 	clk_disable_unprepare(qce->bus);
 	clk_disable_unprepare(qce->iface);
 	clk_disable_unprepare(qce->core);
+
+	dma_unmap_resource(&pdev->dev, qce->base_dma, resource_size(res),
+			   DMA_BIDIRECTIONAL, 0);
 }
 
 static const struct of_device_id qce_crypto_of_match[] = {
diff --git a/drivers/crypto/qce/core.h b/drivers/crypto/qce/core.h
index 228fcd69ec51..25e2af45c047 100644
--- a/drivers/crypto/qce/core.h
+++ b/drivers/crypto/qce/core.h
@@ -39,6 +39,7 @@ struct qce_device {
 	struct qce_dma_data dma;
 	int burst_size;
 	unsigned int pipe_pair_id;
+	dma_addr_t base_dma;
 	int (*async_req_enqueue)(struct qce_device *qce,
 				 struct crypto_async_request *req);
 	void (*async_req_done)(struct qce_device *qce, int ret);
-- 
2.34.1

[PATCH v2 05/16] crypto: qce - Add bam dma support for crypto register r/w

[Md Sadre Alam]

Add BAM/DMA support for crypto register read/write.
With this change multiple crypto register will get
Written/Read using bam in one go.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---
Change in [v2]

* Addressed all the comments from v1

Change in [v1]

* Added initial support to read/write crypto
  regitser via bam

 drivers/crypto/qce/core.h |   9 ++
 drivers/crypto/qce/dma.c  | 227 ++++++++++++++++++++++++++++++++++++++
 drivers/crypto/qce/dma.h  |  24 +++-
 3 files changed, 259 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/qce/core.h b/drivers/crypto/qce/core.h
index 25e2af45c047..bf28dedd1509 100644
--- a/drivers/crypto/qce/core.h
+++ b/drivers/crypto/qce/core.h
@@ -40,6 +40,8 @@ struct qce_device {
 	int burst_size;
 	unsigned int pipe_pair_id;
 	dma_addr_t base_dma;
+	__le32 *reg_read_buf;
+	dma_addr_t reg_buf_phys;
 	int (*async_req_enqueue)(struct qce_device *qce,
 				 struct crypto_async_request *req);
 	void (*async_req_done)(struct qce_device *qce, int ret);
@@ -59,4 +61,11 @@ struct qce_algo_ops {
 	int (*async_req_handle)(struct crypto_async_request *async_req);
 };
 
+int qce_write_reg_dma(struct qce_device *qce, unsigned int offset, u32 val,
+		      int cnt);
+int qce_read_reg_dma(struct qce_device *qce, unsigned int offset, void *buff,
+		     int cnt);
+void qce_clear_bam_transaction(struct qce_device *qce);
+int qce_submit_cmd_desc(struct qce_device *qce, unsigned long flags);
+struct qce_bam_transaction *qce_alloc_bam_txn(struct qce_dma_data *dma);
 #endif /* _CORE_H_ */
diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
index 46db5bf366b4..e4e672d65302 100644
--- a/drivers/crypto/qce/dma.c
+++ b/drivers/crypto/qce/dma.c
@@ -4,12 +4,214 @@
  */
 
 #include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
 #include <crypto/scatterwalk.h>
 
+#include "core.h"
 #include "dma.h"
 
+#define QCE_REG_BUF_DMA_ADDR(qce, vaddr) \
+	((qce)->reg_buf_phys + \
+	 ((uint8_t *)(vaddr) - (uint8_t *)(qce)->reg_read_buf))
+
+void qce_clear_bam_transaction(struct qce_device *qce)
+{
+	struct qce_bam_transaction *qce_bam_txn = qce->dma.qce_bam_txn;
+
+	memset(&qce_bam_txn->qce_bam_ce_index, 0, sizeof(u32) * 8);
+}
+
+static int qce_dma_prep_cmd_sg(struct qce_device *qce, struct dma_chan *chan,
+			       struct scatterlist *qce_bam_sgl,
+			       int qce_sgl_cnt, unsigned long flags,
+			       enum dma_transfer_direction dir_eng,
+			       dma_async_tx_callback cb, void *cb_param)
+{
+	struct dma_async_tx_descriptor *dma_desc;
+	struct qce_desc_info *desc;
+	dma_cookie_t cookie;
+
+	desc = qce->dma.qce_bam_txn->qce_desc;
+
+	if (dir_eng == DMA_MEM_TO_DEV)
+		desc->dir = DMA_TO_DEVICE;
+	if (dir_eng == DMA_DEV_TO_MEM)
+		desc->dir = DMA_FROM_DEVICE;
+
+	if (!qce_bam_sgl || !qce_sgl_cnt)
+		return -EINVAL;
+
+	if (!dma_map_sg(qce->dev, qce_bam_sgl,
+			qce_sgl_cnt, desc->dir)) {
+		dev_err(qce->dev, "failure in mapping sgl for cmd desc\n");
+		return -ENOMEM;
+	}
+
+	dma_desc = dmaengine_prep_slave_sg(chan, qce_bam_sgl, qce_sgl_cnt,
+					   dir_eng, flags);
+	if (!dma_desc) {
+		pr_err("%s:failure in prep cmd desc\n", __func__);
+		dma_unmap_sg(qce->dev, qce_bam_sgl, qce_sgl_cnt, desc->dir);
+		kfree(desc);
+		return -EINVAL;
+	}
+
+	desc->dma_desc = dma_desc;
+	desc->dma_desc->callback = cb;
+	desc->dma_desc->callback_param = cb_param;
+
+	cookie = dmaengine_submit(desc->dma_desc);
+
+	return dma_submit_error(cookie);
+}
+
+int qce_submit_cmd_desc(struct qce_device *qce, unsigned long flags)
+{
+	struct qce_bam_transaction *qce_bam_txn = qce->dma.qce_bam_txn;
+	struct dma_chan *chan = qce->dma.rxchan;
+	unsigned long desc_flags;
+	int ret = 0;
+
+	desc_flags = DMA_PREP_CMD;
+
+	/* For command descriptor always use consumer pipe
+	 * it recomended as per HPG
+	 */
+
+	if (qce_bam_txn->qce_read_sgl_cnt) {
+		ret = qce_dma_prep_cmd_sg(qce, chan, qce_bam_txn->qce_reg_read_sgl,
+					  qce_bam_txn->qce_read_sgl_cnt,
+					  desc_flags, DMA_DEV_TO_MEM,
+					  NULL, NULL);
+		if (ret) {
+			pr_err("error while submiting cmd desc for rx\n");
+			return ret;
+		}
+	}
+
+	if (qce_bam_txn->qce_write_sgl_cnt) {
+		ret = qce_dma_prep_cmd_sg(qce, chan, qce_bam_txn->qce_reg_write_sgl,
+					  qce_bam_txn->qce_write_sgl_cnt,
+					  desc_flags, DMA_MEM_TO_DEV,
+					  NULL, NULL);
+	}
+
+	if (ret) {
+		pr_err("error while submiting cmd desc for tx\n");
+		return ret;
+	}
+
+	qce_dma_issue_pending(&qce->dma);
+
+	if (qce_bam_txn->qce_read_sgl_cnt)
+		dma_unmap_sg(qce->dev, qce_bam_txn->qce_reg_read_sgl,
+			     qce_bam_txn->qce_read_sgl_cnt,
+			     DMA_FROM_DEVICE);
+	if (qce_bam_txn->qce_write_sgl_cnt)
+		dma_unmap_sg(qce->dev, qce_bam_txn->qce_reg_write_sgl,
+			     qce_bam_txn->qce_write_sgl_cnt,
+			     DMA_TO_DEVICE);
+
+	return ret;
+}
+
+static void qce_prep_dma_command_desc(struct qce_device *qce, struct qce_dma_data *dma,
+				      bool read, unsigned int addr, void *buff, int size)
+{
+	struct qce_bam_transaction *qce_bam_txn = dma->qce_bam_txn;
+	struct bam_cmd_element *qce_bam_ce_buffer;
+	int qce_bam_ce_size, cnt, index;
+
+	index = qce_bam_txn->qce_bam_ce_index;
+	qce_bam_ce_buffer = &qce_bam_txn->qce_bam_ce[index];
+	if (read)
+		bam_prep_ce(qce_bam_ce_buffer, addr, BAM_READ_COMMAND,
+			    QCE_REG_BUF_DMA_ADDR(qce,
+						 (unsigned int *)buff));
+	else
+		bam_prep_ce_le32(qce_bam_ce_buffer, addr, BAM_WRITE_COMMAND,
+				 *((__le32 *)buff));
+
+	if (read) {
+		cnt = qce_bam_txn->qce_read_sgl_cnt;
+		qce_bam_ce_buffer = &qce_bam_txn->qce_bam_ce
+			[qce_bam_txn->qce_pre_bam_ce_index];
+		qce_bam_txn->qce_bam_ce_index += size;
+		qce_bam_ce_size = (qce_bam_txn->qce_bam_ce_index -
+				qce_bam_txn->qce_pre_bam_ce_index) *
+			sizeof(struct bam_cmd_element);
+
+		sg_set_buf(&qce_bam_txn->qce_reg_read_sgl[cnt],
+			   qce_bam_ce_buffer,
+				qce_bam_ce_size);
+
+		++qce_bam_txn->qce_read_sgl_cnt;
+		qce_bam_txn->qce_pre_bam_ce_index =
+					qce_bam_txn->qce_bam_ce_index;
+	} else {
+		cnt = qce_bam_txn->qce_write_sgl_cnt;
+		qce_bam_ce_buffer = &qce_bam_txn->qce_bam_ce
+			[qce_bam_txn->qce_pre_bam_ce_index];
+		qce_bam_txn->qce_bam_ce_index += size;
+		qce_bam_ce_size = (qce_bam_txn->qce_bam_ce_index -
+				qce_bam_txn->qce_pre_bam_ce_index) *
+			sizeof(struct bam_cmd_element);
+
+		sg_set_buf(&qce_bam_txn->qce_reg_write_sgl[cnt],
+			   qce_bam_ce_buffer,
+				qce_bam_ce_size);
+
+		++qce_bam_txn->qce_write_sgl_cnt;
+		qce_bam_txn->qce_pre_bam_ce_index =
+					qce_bam_txn->qce_bam_ce_index;
+	}
+}
+
+int qce_write_reg_dma(struct qce_device *qce,
+		      unsigned int offset, u32 val, int cnt)
+{
+	qce_prep_dma_command_desc(qce, &qce->dma, false, (qce->base_dma + offset),
+				  &val, cnt);
+	return 0;
+}
+
+int qce_read_reg_dma(struct qce_device *qce,
+		     unsigned int offset, void *buff, int cnt)
+{
+	qce_prep_dma_command_desc(qce, &qce->dma, true, (qce->base_dma + offset),
+				  qce->reg_read_buf, cnt);
+	memcpy(buff, qce->reg_read_buf, 4);
+
+	return 0;
+}
+
+struct qce_bam_transaction *qce_alloc_bam_txn(struct qce_dma_data *dma)
+{
+	struct qce_bam_transaction *qce_bam_txn;
+
+	dma->qce_bam_txn = kmalloc(sizeof(*qce_bam_txn), GFP_KERNEL);
+	if (!dma->qce_bam_txn)
+		return NULL;
+
+	dma->qce_bam_txn->qce_desc = kzalloc(sizeof(*dma->qce_bam_txn->qce_desc),
+					     GFP_KERNEL);
+	if (!dma->qce_bam_txn->qce_desc) {
+		kfree(dma->qce_bam_txn);
+		return NULL;
+	}
+
+	sg_init_table(dma->qce_bam_txn->qce_reg_write_sgl,
+		      QCE_BAM_CMD_SGL_SIZE);
+
+	sg_init_table(dma->qce_bam_txn->qce_reg_read_sgl,
+		      QCE_BAM_CMD_SGL_SIZE);
+
+	return dma->qce_bam_txn;
+}
+
 int qce_dma_request(struct device *dev, struct qce_dma_data *dma)
 {
+	struct qce_device *qce = container_of(dma, struct qce_device, dma);
 	int ret;
 
 	dma->txchan = dma_request_chan(dev, "tx");
@@ -31,7 +233,22 @@ int qce_dma_request(struct device *dev, struct qce_dma_data *dma)
 
 	dma->ignore_buf = dma->result_buf + QCE_RESULT_BUF_SZ;
 
+	dma->qce_bam_txn = qce_alloc_bam_txn(dma);
+	if (!dma->qce_bam_txn) {
+		pr_err("Failed to allocate bam transaction\n");
+		return -ENOMEM;
+	}
+
+	qce->reg_read_buf = dmam_alloc_coherent(qce->dev, QCE_MAX_REG_READ *
+						sizeof(*qce->reg_read_buf),
+						&qce->reg_buf_phys, GFP_KERNEL);
+	if (!qce->reg_read_buf) {
+		pr_err("Failed to allocate reg_read_buf\n");
+		return -ENOMEM;
+	}
+
 	return 0;
+
 error_nomem:
 	dma_release_channel(dma->rxchan);
 error_rx:
@@ -41,9 +258,19 @@ int qce_dma_request(struct device *dev, struct qce_dma_data *dma)
 
 void qce_dma_release(struct qce_dma_data *dma)
 {
+	struct qce_device *qce = container_of(dma,
+			struct qce_device, dma);
+
 	dma_release_channel(dma->txchan);
 	dma_release_channel(dma->rxchan);
 	kfree(dma->result_buf);
+	if (qce->reg_read_buf)
+		dmam_free_coherent(qce->dev, QCE_MAX_REG_READ *
+				   sizeof(*qce->reg_read_buf),
+				   qce->reg_read_buf,
+				   qce->reg_buf_phys);
+	kfree(dma->qce_bam_txn->qce_desc);
+	kfree(dma->qce_bam_txn);
 }
 
 struct scatterlist *
diff --git a/drivers/crypto/qce/dma.h b/drivers/crypto/qce/dma.h
index 786402169360..f10991590b3f 100644
--- a/drivers/crypto/qce/dma.h
+++ b/drivers/crypto/qce/dma.h
@@ -7,6 +7,7 @@
 #define _DMA_H_
 
 #include <linux/dmaengine.h>
+#include <linux/dma/qcom_bam_dma.h>
 
 /* maximum data transfer block size between BAM and CE */
 #define QCE_BAM_BURST_SIZE		64
@@ -14,6 +15,10 @@
 #define QCE_AUTHIV_REGS_CNT		16
 #define QCE_AUTH_BYTECOUNT_REGS_CNT	4
 #define QCE_CNTRIV_REGS_CNT		4
+#define QCE_BAM_CMD_SGL_SIZE           64
+#define QCE_BAM_CMD_ELEMENT_SIZE       64
+#define QCE_DMA_DESC_FLAG_BAM_NWD      (0x0004)
+#define QCE_MAX_REG_READ               8
 
 struct qce_result_dump {
 	u32 auth_iv[QCE_AUTHIV_REGS_CNT];
@@ -27,13 +32,30 @@ struct qce_result_dump {
 #define QCE_RESULT_BUF_SZ	\
 		ALIGN(sizeof(struct qce_result_dump), QCE_BAM_BURST_SIZE)
 
+struct qce_bam_transaction {
+	struct bam_cmd_element qce_bam_ce[QCE_BAM_CMD_ELEMENT_SIZE];
+	struct scatterlist qce_reg_write_sgl[QCE_BAM_CMD_SGL_SIZE];
+	struct scatterlist qce_reg_read_sgl[QCE_BAM_CMD_SGL_SIZE];
+	struct qce_desc_info *qce_desc;
+	u32 qce_bam_ce_index;
+	u32 qce_pre_bam_ce_index;
+	u32 qce_write_sgl_cnt;
+	u32 qce_read_sgl_cnt;
+};
+
 struct qce_dma_data {
 	struct dma_chan *txchan;
 	struct dma_chan *rxchan;
 	struct qce_result_dump *result_buf;
+	struct qce_bam_transaction *qce_bam_txn;
 	void *ignore_buf;
 };
 
+struct qce_desc_info {
+	struct dma_async_tx_descriptor *dma_desc;
+	enum dma_data_direction dir;
+};
+
 int qce_dma_request(struct device *dev, struct qce_dma_data *dma);
 void qce_dma_release(struct qce_dma_data *dma);
 int qce_dma_prep_sgs(struct qce_dma_data *dma, struct scatterlist *sg_in,
@@ -44,5 +66,5 @@ int qce_dma_terminate_all(struct qce_dma_data *dma);
 struct scatterlist *
 qce_sgtable_add(struct sg_table *sgt, struct scatterlist *sg_add,
 		unsigned int max_len);
-
+void qce_dma_issue_cmd_desc_pending(struct qce_dma_data *dma, bool read);
 #endif /* _DMA_H_ */
-- 
2.34.1

[PATCH v2 06/16] crypto: qce - Convert register r/w for skcipher via BAM/DMA

[Md Sadre Alam]

Convert register read/write for skcipher via BAM/DMA.
with this change all the crypto register configuration
will be done via BAM/DMA. This change will prepare command
descriptor for all register and write it once.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---
Change in [v2]

* No change

Change in [v1]

* Added crypto register read/write via bam for skcipher

 drivers/crypto/qce/common.c | 42 ++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
index 04253a8d3340..d1da6b1938f3 100644
--- a/drivers/crypto/qce/common.c
+++ b/drivers/crypto/qce/common.c
@@ -34,7 +34,7 @@ static inline void qce_write_array(struct qce_device *qce, u32 offset,
 	int i;
 
 	for (i = 0; i < len; i++)
-		qce_write(qce, offset + i * sizeof(u32), val[i]);
+		qce_write_reg_dma(qce, offset + i * sizeof(u32), val[i], 1);
 }
 
 static inline void
@@ -43,7 +43,7 @@ qce_clear_array(struct qce_device *qce, u32 offset, unsigned int len)
 	int i;
 
 	for (i = 0; i < len; i++)
-		qce_write(qce, offset + i * sizeof(u32), 0);
+		qce_write_reg_dma(qce, offset + i * sizeof(u32), 0, 1);
 }
 
 static u32 qce_config_reg(struct qce_device *qce, int little)
@@ -86,16 +86,16 @@ static void qce_setup_config(struct qce_device *qce)
 	config = qce_config_reg(qce, 0);
 
 	/* clear status */
-	qce_write(qce, REG_STATUS, 0);
-	qce_write(qce, REG_CONFIG, config);
+	qce_write_reg_dma(qce, REG_STATUS, 0, 1);
+	qce_write_reg_dma(qce, REG_CONFIG, config, 1);
 }
 
 static inline void qce_crypto_go(struct qce_device *qce, bool result_dump)
 {
 	if (result_dump)
-		qce_write(qce, REG_GOPROC, BIT(GO_SHIFT) | BIT(RESULTS_DUMP_SHIFT));
+		qce_write_reg_dma(qce, REG_GOPROC, BIT(GO_SHIFT) | BIT(RESULTS_DUMP_SHIFT), 1);
 	else
-		qce_write(qce, REG_GOPROC, BIT(GO_SHIFT));
+		qce_write_reg_dma(qce, REG_GOPROC, BIT(GO_SHIFT), 1);
 }
 
 #if defined(CONFIG_CRYPTO_DEV_QCE_SHA) || defined(CONFIG_CRYPTO_DEV_QCE_AEAD)
@@ -308,7 +308,7 @@ static void qce_xtskey(struct qce_device *qce, const u8 *enckey,
 	/* Set data unit size to cryptlen. Anything else causes
 	 * crypto engine to return back incorrect results.
 	 */
-	qce_write(qce, REG_ENCR_XTS_DU_SIZE, cryptlen);
+	qce_write_reg_dma(qce, REG_ENCR_XTS_DU_SIZE, cryptlen, 1);
 }
 
 static int qce_setup_regs_skcipher(struct crypto_async_request *async_req)
@@ -325,7 +325,9 @@ static int qce_setup_regs_skcipher(struct crypto_async_request *async_req)
 	u32 encr_cfg = 0, auth_cfg = 0, config;
 	unsigned int ivsize = rctx->ivsize;
 	unsigned long flags = rctx->flags;
+	int ret;
 
+	qce_clear_bam_transaction(qce);
 	qce_setup_config(qce);
 
 	if (IS_XTS(flags))
@@ -336,7 +338,7 @@ static int qce_setup_regs_skcipher(struct crypto_async_request *async_req)
 	qce_cpu_to_be32p_array(enckey, ctx->enc_key, keylen);
 	enckey_words = keylen / sizeof(u32);
 
-	qce_write(qce, REG_AUTH_SEG_CFG, auth_cfg);
+	qce_write_reg_dma(qce, REG_AUTH_SEG_CFG, auth_cfg, 1);
 
 	encr_cfg = qce_encr_cfg(flags, keylen);
 
@@ -369,25 +371,31 @@ static int qce_setup_regs_skcipher(struct crypto_async_request *async_req)
 	if (IS_ENCRYPT(flags))
 		encr_cfg |= BIT(ENCODE_SHIFT);
 
-	qce_write(qce, REG_ENCR_SEG_CFG, encr_cfg);
-	qce_write(qce, REG_ENCR_SEG_SIZE, rctx->cryptlen);
-	qce_write(qce, REG_ENCR_SEG_START, 0);
+	qce_write_reg_dma(qce, REG_ENCR_SEG_CFG, encr_cfg, 1);
+	qce_write_reg_dma(qce, REG_ENCR_SEG_SIZE, rctx->cryptlen, 1);
+	qce_write_reg_dma(qce, REG_ENCR_SEG_START, 0, 1);
 
 	if (IS_CTR(flags)) {
-		qce_write(qce, REG_CNTR_MASK, ~0);
-		qce_write(qce, REG_CNTR_MASK0, ~0);
-		qce_write(qce, REG_CNTR_MASK1, ~0);
-		qce_write(qce, REG_CNTR_MASK2, ~0);
+		qce_write_reg_dma(qce, REG_CNTR_MASK, ~0, 1);
+		qce_write_reg_dma(qce, REG_CNTR_MASK0, ~0, 1);
+		qce_write_reg_dma(qce, REG_CNTR_MASK1, ~0, 1);
+		qce_write_reg_dma(qce, REG_CNTR_MASK2, ~0, 1);
 	}
 
-	qce_write(qce, REG_SEG_SIZE, rctx->cryptlen);
+	qce_write_reg_dma(qce, REG_SEG_SIZE, rctx->cryptlen, 1);
 
 	/* get little endianness */
 	config = qce_config_reg(qce, 1);
-	qce_write(qce, REG_CONFIG, config);
+	qce_write_reg_dma(qce, REG_CONFIG, config, 1);
 
 	qce_crypto_go(qce, true);
 
+	ret = qce_submit_cmd_desc(qce, 0);
+	if (ret) {
+		dev_err(qce->dev, "Error in skcipher cmd descriptor\n");
+		return ret;
+	}
+
 	return 0;
 }
 #endif
-- 
2.34.1

[PATCH v2 07/16] crypto: qce - Convert register r/w for sha via BAM/DMA

[Md Sadre Alam]

Convert register read/write for sha via BAM/DMA.
with this change all the crypto register configuration
will be done via BAM/DMA. This change will prepare command
descriptor for all register and write it once.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v2]

* No change

Change in [v1]

* Added initial support for register read/write via bam 
  for SHA

 drivers/crypto/qce/common.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
index d1da6b1938f3..d485762a3fdc 100644
--- a/drivers/crypto/qce/common.c
+++ b/drivers/crypto/qce/common.c
@@ -157,17 +157,19 @@ static int qce_setup_regs_ahash(struct crypto_async_request *async_req)
 	__be32 mackey[QCE_SHA_HMAC_KEY_SIZE / sizeof(__be32)] = {0};
 	u32 auth_cfg = 0, config;
 	unsigned int iv_words;
+	int ret;
 
 	/* if not the last, the size has to be on the block boundary */
 	if (!rctx->last_blk && req->nbytes % blocksize)
 		return -EINVAL;
 
+	qce_clear_bam_transaction(qce);
 	qce_setup_config(qce);
 
 	if (IS_CMAC(rctx->flags)) {
-		qce_write(qce, REG_AUTH_SEG_CFG, 0);
-		qce_write(qce, REG_ENCR_SEG_CFG, 0);
-		qce_write(qce, REG_ENCR_SEG_SIZE, 0);
+		qce_write_reg_dma(qce, REG_AUTH_SEG_CFG, 0, 1);
+		qce_write_reg_dma(qce, REG_ENCR_SEG_CFG, 0, 1);
+		qce_write_reg_dma(qce, REG_ENCR_SEG_SIZE, 0, 1);
 		qce_clear_array(qce, REG_AUTH_IV0, 16);
 		qce_clear_array(qce, REG_AUTH_KEY0, 16);
 		qce_clear_array(qce, REG_AUTH_BYTECNT0, 4);
@@ -213,18 +215,24 @@ static int qce_setup_regs_ahash(struct crypto_async_request *async_req)
 		auth_cfg &= ~BIT(AUTH_FIRST_SHIFT);
 
 go_proc:
-	qce_write(qce, REG_AUTH_SEG_CFG, auth_cfg);
-	qce_write(qce, REG_AUTH_SEG_SIZE, req->nbytes);
-	qce_write(qce, REG_AUTH_SEG_START, 0);
-	qce_write(qce, REG_ENCR_SEG_CFG, 0);
-	qce_write(qce, REG_SEG_SIZE, req->nbytes);
+	qce_write_reg_dma(qce, REG_AUTH_SEG_CFG, auth_cfg, 1);
+	qce_write_reg_dma(qce, REG_AUTH_SEG_SIZE, req->nbytes, 1);
+	qce_write_reg_dma(qce, REG_AUTH_SEG_START, 0, 1);
+	qce_write_reg_dma(qce, REG_ENCR_SEG_CFG, 0, 1);
+	qce_write_reg_dma(qce, REG_SEG_SIZE, req->nbytes, 1);
 
 	/* get little endianness */
 	config = qce_config_reg(qce, 1);
-	qce_write(qce, REG_CONFIG, config);
+	qce_write_reg_dma(qce, REG_CONFIG, config, 1);
 
 	qce_crypto_go(qce, true);
 
+	ret = qce_submit_cmd_desc(qce, 0);
+	if (ret) {
+		dev_err(qce->dev, "Error in sha cmd descriptor\n");
+		return ret;
+	}
+
 	return 0;
 }
 #endif
-- 
2.34.1

[PATCH v2 08/16] crypto: qce - Convert register r/w for aead via BAM/DMA

[Md Sadre Alam]

Convert register read/write for aead via BAM/DMA.
with this change all the crypto register configuration
will be done via BAM/DMA. This change will prepare command
descriptor for all register and write it once.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v2]

* updated commit message 

Change in [v1]

* Added initial support for reagister read/write via bam
  for aead

 drivers/crypto/qce/common.c | 38 ++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
index d485762a3fdc..ff96f6ba1fc5 100644
--- a/drivers/crypto/qce/common.c
+++ b/drivers/crypto/qce/common.c
@@ -454,7 +454,9 @@ static int qce_setup_regs_aead(struct crypto_async_request *async_req)
 	unsigned long flags = rctx->flags;
 	u32 encr_cfg, auth_cfg, config, totallen;
 	u32 iv_last_word;
+	int ret;
 
+	qce_clear_bam_transaction(qce);
 	qce_setup_config(qce);
 
 	/* Write encryption key */
@@ -467,12 +469,12 @@ static int qce_setup_regs_aead(struct crypto_async_request *async_req)
 
 	if (IS_CCM(rctx->flags)) {
 		iv_last_word = enciv[enciv_words - 1];
-		qce_write(qce, REG_CNTR3_IV3, iv_last_word + 1);
+		qce_write_reg_dma(qce, REG_CNTR3_IV3, iv_last_word + 1, 1);
 		qce_write_array(qce, REG_ENCR_CCM_INT_CNTR0, (u32 *)enciv, enciv_words);
-		qce_write(qce, REG_CNTR_MASK, ~0);
-		qce_write(qce, REG_CNTR_MASK0, ~0);
-		qce_write(qce, REG_CNTR_MASK1, ~0);
-		qce_write(qce, REG_CNTR_MASK2, ~0);
+		qce_write_reg_dma(qce, REG_CNTR_MASK, ~0, 1);
+		qce_write_reg_dma(qce, REG_CNTR_MASK0, ~0, 1);
+		qce_write_reg_dma(qce, REG_CNTR_MASK1, ~0, 1);
+		qce_write_reg_dma(qce, REG_CNTR_MASK2, ~0, 1);
 	}
 
 	/* Clear authentication IV and KEY registers of previous values */
@@ -508,7 +510,7 @@ static int qce_setup_regs_aead(struct crypto_async_request *async_req)
 	encr_cfg = qce_encr_cfg(flags, enc_keylen);
 	if (IS_ENCRYPT(flags))
 		encr_cfg |= BIT(ENCODE_SHIFT);
-	qce_write(qce, REG_ENCR_SEG_CFG, encr_cfg);
+	qce_write_reg_dma(qce, REG_ENCR_SEG_CFG, encr_cfg, 1);
 
 	/* Set up AUTH_SEG_CFG */
 	auth_cfg = qce_auth_cfg(rctx->flags, auth_keylen, ctx->authsize);
@@ -525,34 +527,40 @@ static int qce_setup_regs_aead(struct crypto_async_request *async_req)
 		else
 			auth_cfg |= AUTH_POS_BEFORE << AUTH_POS_SHIFT;
 	}
-	qce_write(qce, REG_AUTH_SEG_CFG, auth_cfg);
+	qce_write_reg_dma(qce, REG_AUTH_SEG_CFG, auth_cfg, 1);
 
 	totallen = rctx->cryptlen + rctx->assoclen;
 
 	/* Set the encryption size and start offset */
 	if (IS_CCM(rctx->flags) && IS_DECRYPT(rctx->flags))
-		qce_write(qce, REG_ENCR_SEG_SIZE, rctx->cryptlen + ctx->authsize);
+		qce_write_reg_dma(qce, REG_ENCR_SEG_SIZE, rctx->cryptlen + ctx->authsize, 1);
 	else
-		qce_write(qce, REG_ENCR_SEG_SIZE, rctx->cryptlen);
-	qce_write(qce, REG_ENCR_SEG_START, rctx->assoclen & 0xffff);
+		qce_write_reg_dma(qce, REG_ENCR_SEG_SIZE, rctx->cryptlen, 1);
+	qce_write_reg_dma(qce, REG_ENCR_SEG_START, rctx->assoclen & 0xffff, 1);
 
 	/* Set the authentication size and start offset */
-	qce_write(qce, REG_AUTH_SEG_SIZE, totallen);
-	qce_write(qce, REG_AUTH_SEG_START, 0);
+	qce_write_reg_dma(qce, REG_AUTH_SEG_SIZE, totallen, 1);
+	qce_write_reg_dma(qce, REG_AUTH_SEG_START, 0, 1);
 
 	/* Write total length */
 	if (IS_CCM(rctx->flags) && IS_DECRYPT(rctx->flags))
-		qce_write(qce, REG_SEG_SIZE, totallen + ctx->authsize);
+		qce_write_reg_dma(qce, REG_SEG_SIZE, totallen + ctx->authsize, 1);
 	else
-		qce_write(qce, REG_SEG_SIZE, totallen);
+		qce_write_reg_dma(qce, REG_SEG_SIZE, totallen, 1);
 
 	/* get little endianness */
 	config = qce_config_reg(qce, 1);
-	qce_write(qce, REG_CONFIG, config);
+	qce_write_reg_dma(qce, REG_CONFIG, config, 1);
 
 	/* Start the process */
 	qce_crypto_go(qce, !IS_CCM(flags));
 
+	ret = qce_submit_cmd_desc(qce, 0);
+	if (ret) {
+		dev_err(qce->dev, "Error in aead cmd descriptor\n");
+		return ret;
+	}
+
 	return 0;
 }
 #endif
-- 
2.34.1

[PATCH v2 09/16] crypto: qce - Add LOCK and UNLOCK flag support

[Md Sadre Alam]

Add LOCK and UNLOCK flag support while preapring
command descriptor for writing crypto register.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v2]

* No change

Change in [v1]

* Added initial support for lock/un-lock flag

 drivers/crypto/qce/dma.c | 7 ++++++-
 drivers/crypto/qce/dma.h | 2 ++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/qce/dma.c b/drivers/crypto/qce/dma.c
index e4e672d65302..93d455d1d5b4 100644
--- a/drivers/crypto/qce/dma.c
+++ b/drivers/crypto/qce/dma.c
@@ -72,7 +72,12 @@ int qce_submit_cmd_desc(struct qce_device *qce, unsigned long flags)
 	unsigned long desc_flags;
 	int ret = 0;
 
-	desc_flags = DMA_PREP_CMD;
+	if (flags & QCE_DMA_DESC_FLAG_LOCK)
+		desc_flags = DMA_PREP_CMD | DMA_PREP_LOCK;
+	else if (flags & QCE_DMA_DESC_FLAG_UNLOCK)
+		desc_flags = DMA_PREP_CMD | DMA_PREP_UNLOCK;
+	else
+		desc_flags = DMA_PREP_CMD;
 
 	/* For command descriptor always use consumer pipe
 	 * it recomended as per HPG
diff --git a/drivers/crypto/qce/dma.h b/drivers/crypto/qce/dma.h
index f10991590b3f..ad8a18a720b1 100644
--- a/drivers/crypto/qce/dma.h
+++ b/drivers/crypto/qce/dma.h
@@ -19,6 +19,8 @@
 #define QCE_BAM_CMD_ELEMENT_SIZE       64
 #define QCE_DMA_DESC_FLAG_BAM_NWD      (0x0004)
 #define QCE_MAX_REG_READ               8
+#define QCE_DMA_DESC_FLAG_LOCK          (0x0002)
+#define QCE_DMA_DESC_FLAG_UNLOCK        (0x0001)
 
 struct qce_result_dump {
 	u32 auth_iv[QCE_AUTHIV_REGS_CNT];
-- 
2.34.1

[PATCH v2 10/16] crypto: qce - Add support for lock aquire,lock release api.

[Md Sadre Alam]

Add support for lock acquire and lock release api.
When multiple EE's(Execution Environment) want to access
CE5 then there will be race condition b/w multiple EE's.

Since each EE's having their dedicated BAM pipe, BAM allows
Locking and Unlocking on BAM pipe. So if one EE's requesting
for CE5 access then that EE's first has to LOCK the BAM pipe
while setting LOCK bit on command descriptor and then access
it. After finishing the request EE's has to UNLOCK the BAM pipe
so in this way we race condition will not happen.

Added these two API qce_bam_acquire_lock() and qce_bam_release_lock()
for the same.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v2]

* No chnage

Change in [v1]

* Added initial support for lock_acquire and lock_release
  api.

 drivers/crypto/qce/common.c | 36 ++++++++++++++++++++++++++++++++++++
 drivers/crypto/qce/core.h   |  2 ++
 2 files changed, 38 insertions(+)

diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
index ff96f6ba1fc5..a8eaffe41101 100644
--- a/drivers/crypto/qce/common.c
+++ b/drivers/crypto/qce/common.c
@@ -617,3 +617,39 @@ void qce_get_version(struct qce_device *qce, u32 *major, u32 *minor, u32 *step)
 	*minor = (val & CORE_MINOR_REV_MASK) >> CORE_MINOR_REV_SHIFT;
 	*step = (val & CORE_STEP_REV_MASK) >> CORE_STEP_REV_SHIFT;
 }
+
+int qce_bam_acquire_lock(struct qce_device *qce)
+{
+	int ret;
+
+	qce_clear_bam_transaction(qce);
+
+	/* This is just a dummy write to acquire lock on bam pipe */
+	qce_write_reg_dma(qce, REG_AUTH_SEG_CFG, 0, 1);
+
+	ret = qce_submit_cmd_desc(qce, QCE_DMA_DESC_FLAG_LOCK);
+	if (ret) {
+		dev_err(qce->dev, "Error in Locking cmd descriptor\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+int qce_bam_release_lock(struct qce_device *qce)
+{
+	int ret;
+
+	qce_clear_bam_transaction(qce);
+
+	/* This just dummy write to release lock on bam pipe*/
+	qce_write_reg_dma(qce, REG_AUTH_SEG_CFG, 0, 1);
+
+	ret = qce_submit_cmd_desc(qce, QCE_DMA_DESC_FLAG_UNLOCK);
+	if (ret) {
+		dev_err(qce->dev, "Error in Un-Locking cmd descriptor\n");
+		return ret;
+	}
+
+	return 0;
+}
diff --git a/drivers/crypto/qce/core.h b/drivers/crypto/qce/core.h
index bf28dedd1509..d01d810b60ad 100644
--- a/drivers/crypto/qce/core.h
+++ b/drivers/crypto/qce/core.h
@@ -68,4 +68,6 @@ int qce_read_reg_dma(struct qce_device *qce, unsigned int offset, void *buff,
 void qce_clear_bam_transaction(struct qce_device *qce);
 int qce_submit_cmd_desc(struct qce_device *qce, unsigned long flags);
 struct qce_bam_transaction *qce_alloc_bam_txn(struct qce_dma_data *dma);
+int qce_bam_acquire_lock(struct qce_device *qce);
+int qce_bam_release_lock(struct qce_device *qce);
 #endif /* _CORE_H_ */
-- 
2.34.1

[PATCH v2 11/16] crypto: qce - Add support for lock/unlock in skcipher

[Md Sadre Alam]

Add support for lock/unlock on bam pipe in skcipher.
If multiple EE's(Execution Environment) try to access
the same crypto engine then before accessing the crypto
engine EE's has to lock the bam pipe and then submit the
request to crypto engine. Once request done then EE's has
to unlock the bam pipe so that others EE's can access the
crypto engine.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v2]

* Removed unmap_sg() from crypto done api

Change in [v1]

* Added qce_bam_acquire_lock and qce_bam_release_lock 
  api in skcipher

 drivers/crypto/qce/skcipher.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/crypto/qce/skcipher.c b/drivers/crypto/qce/skcipher.c
index 5b493fdc1e74..a4e09562b5f4 100644
--- a/drivers/crypto/qce/skcipher.c
+++ b/drivers/crypto/qce/skcipher.c
@@ -52,6 +52,8 @@ static void qce_skcipher_done(void *data)
 
 	sg_free_table(&rctx->dst_tbl);
 
+	qce_bam_release_lock(qce);
+
 	error = qce_check_status(qce, &status);
 	if (error < 0)
 		dev_dbg(qce->dev, "skcipher operation error (%x)\n", status);
@@ -82,6 +84,8 @@ qce_skcipher_async_req_handle(struct crypto_async_request *async_req)
 	dir_src = diff_dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL;
 	dir_dst = diff_dst ? DMA_FROM_DEVICE : DMA_BIDIRECTIONAL;
 
+	qce_bam_acquire_lock(qce);
+
 	rctx->src_nents = sg_nents_for_len(req->src, req->cryptlen);
 	if (diff_dst)
 		rctx->dst_nents = sg_nents_for_len(req->dst, req->cryptlen);
-- 
2.34.1

[PATCH v2 12/16] crypto: qce - Add support for lock/unlock in sha

[Md Sadre Alam]

Add support for lock/unlock on bam pipe in sha.
If multiple EE's(Execution Environment) try to access
the same crypto engine then before accessing the crypto
engine EE's has to lock the bam pipe and then submit the
request to crypto engine. Once request done then EE's has
to unlock the bam pipe so that others EE's can access the
crypto engine.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v2]

* Removed unmap_sg() from crypto done api

Change in [v1]

* Added qce_bam_acquire_lock and qce_bam_release_lock
  api in sha

 drivers/crypto/qce/sha.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/crypto/qce/sha.c b/drivers/crypto/qce/sha.c
index fc72af8aa9a7..abfa63ff18d7 100644
--- a/drivers/crypto/qce/sha.c
+++ b/drivers/crypto/qce/sha.c
@@ -60,6 +60,8 @@ static void qce_ahash_done(void *data)
 	rctx->byte_count[0] = cpu_to_be32(result->auth_byte_count[0]);
 	rctx->byte_count[1] = cpu_to_be32(result->auth_byte_count[1]);
 
+	qce_bam_release_lock(qce);
+
 	error = qce_check_status(qce, &status);
 	if (error < 0)
 		dev_dbg(qce->dev, "ahash operation error (%x)\n", status);
@@ -90,6 +92,8 @@ static int qce_ahash_async_req_handle(struct crypto_async_request *async_req)
 		rctx->authklen = AES_KEYSIZE_128;
 	}
 
+	qce_bam_acquire_lock(qce);
+
 	rctx->src_nents = sg_nents_for_len(req->src, req->nbytes);
 	if (rctx->src_nents < 0) {
 		dev_err(qce->dev, "Invalid numbers of src SG.\n");
-- 
2.34.1

[PATCH v2 13/16] crypto: qce - Add support for lock/unlock in aead

[Md Sadre Alam]

Add support for lock/unlock on bam pipe in aead.
If multiple EE's(Execution Environment) try to access
the same crypto engine then before accessing the crypto
engine EE's has to lock the bam pipe and then submit the
request to crypto engine. Once request done then EE's has
to unlock the bam pipe so that others EE's can access the
crypto engine.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v2]

* Removed unmap_sg() from crypto done api

Change in [v1]

* Added qce_bam_acquire_lock and qce_bam_release_lock
  api in aead

 drivers/crypto/qce/aead.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/crypto/qce/aead.c b/drivers/crypto/qce/aead.c
index 7d811728f047..13fb7af69f54 100644
--- a/drivers/crypto/qce/aead.c
+++ b/drivers/crypto/qce/aead.c
@@ -63,6 +63,8 @@ static void qce_aead_done(void *data)
 		sg_free_table(&rctx->dst_tbl);
 	}
 
+	qce_bam_release_lock(qce);
+
 	error = qce_check_status(qce, &status);
 	if (error < 0 && (error != -EBADMSG))
 		dev_err(qce->dev, "aead operation error (%x)\n", status);
@@ -433,6 +435,8 @@ qce_aead_async_req_handle(struct crypto_async_request *async_req)
 	else
 		rctx->assoclen = req->assoclen;
 
+	qce_bam_acquire_lock(qce);
+
 	diff_dst = (req->src != req->dst) ? true : false;
 	dir_src = diff_dst ? DMA_TO_DEVICE : DMA_BIDIRECTIONAL;
 	dir_dst = diff_dst ? DMA_FROM_DEVICE : DMA_BIDIRECTIONAL;
-- 
2.34.1

[PATCH v2 14/16] arm64: dts: qcom: ipq9574: enable bam pipe locking/unlocking

[Md Sadre Alam]

enable bam pipe locking/unlocking for ipq9507 SoC.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v2]

* enabled locking/unlocking support for ipq9574

Change in [v1]

* This patch was not included in [v1]

 arch/arm64/boot/dts/qcom/ipq9574.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 48dfafea46a7..dacaec62ec39 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -262,6 +262,7 @@ cryptobam: dma-controller@704000 {
 			#dma-cells = <1>;
 			qcom,ee = <1>;
 			qcom,controlled-remotely;
+			qcom,bam_pipe_lock;
 		};
 
 		crypto: crypto@73a000 {
-- 
2.34.1

[PATCH v2 15/16] arm64: dts: qcom: ipq8074: enable bam pipe locking/unlocking

[Md Sadre Alam]

enable bam pipe locking/unlocking for ipq8074 SoC.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v2]

* enabled locking/unlocking support for ipq8074

Change in [v1]

* This patch was not included in [v1]

 arch/arm64/boot/dts/qcom/ipq8074.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index 284a4553070f..e53d75e3ecef 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -291,6 +291,7 @@ cryptobam: dma-controller@704000 {
 			#dma-cells = <1>;
 			qcom,ee = <1>;
 			qcom,controlled-remotely;
+			qcom,bam_pipe_lock;
 			status = "disabled";
 		};
 
-- 
2.34.1

[PATCH v2 16/16] arm64: dts: qcom: ipq6018: enable bam pipe locking/unlocking

[Md Sadre Alam]

enable bam pipe locking/unlocking for ipq6018 SoC.

Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
---

Change in [v2]

* enabled locking/unlocking support for ipq6018

Change in [v1]

* This patch was not included in [v1]

 arch/arm64/boot/dts/qcom/ipq6018.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/ipq6018.dtsi b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
index e1e45da7f787..652c2bbf5e99 100644
--- a/arch/arm64/boot/dts/qcom/ipq6018.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq6018.dtsi
@@ -354,6 +354,7 @@ cryptobam: dma-controller@704000 {
 			#dma-cells = <1>;
 			qcom,ee = <1>;
 			qcom,controlled-remotely;
+			qcom,bam_pipe_lock;
 		};
 
 		crypto: crypto@73a000 {
-- 
2.34.1

Re: [PATCH v2 00/16] Add cmd descriptor support

[Caleb Connolly]

Hi,

A note for future patches, please scope your cover letter subject:

"dmaengine: qcom: bam_dma: add cmd descriptor support"

On 15/08/2024 10:57, Md Sadre Alam wrote:
> This series of patches will add command descriptor
> support to read/write crypto engine register via
> BAM/DMA
> 
> We need this support because if there is multiple EE's
> (Execution Environment) accessing the same CE then there
> will be race condition. To avoid this race condition
> BAM HW hsving LOC/UNLOCK feature on BAM pipes and this
> LOCK/UNLOCK will be set via command descriptor only.
> 
> Since each EE's having their dedicated BAM pipe, BAM allows
> Locking and Unlocking on BAM pipe. So if one EE's requesting
> for CE5 access then that EE's first has to LOCK the BAM pipe
> while setting LOCK bit on command descriptor and then access
> it. After finishing the request EE's has to UNLOCK the BAM pipe
> so in this way we race condition will not happen.
> 
> tested with "tcrypt.ko" and "kcapi" tool.
> 
> Need help to test these all the patches on msm platform

DT changes here are only for a few IPQ platforms, please explain in the 
cover letter if this is some IPQ specific feature which doesn't exist on 
other platforms, or if you're only enabling it on IPQ.

Some broad strokes testing instructions (at the very least) and 
requirements (testing on what hardware?) aren't made obvious at all here.

Kind regards,
> 
> v2:
>   * Addressed all the comments from v1
>   * Added the dt-binding
>   * Added locking/unlocking mechanism in bam driver
> 
> v1:
>   * https://lore.kernel.org/lkml/20231214114239.2635325-1-quic_mdalam@quicinc.com/
>   * Initial set of patches for cmd descriptor support
> 
> Md Sadre Alam (16):
>    dt-bindings: dma: qcom,bam: Add bam pipe lock
>    dmaengine: qcom: bam_dma: add bam_pipe_lock dt property
>    dmaengine: qcom: bam_dma: add LOCK & UNLOCK flag support
>    crypto: qce - Add support for crypto address read
>    crypto: qce - Add bam dma support for crypto register r/w
>    crypto: qce - Convert register r/w for skcipher via BAM/DMA
>    crypto: qce - Convert register r/w for sha via BAM/DMA
>    crypto: qce - Convert register r/w for aead via BAM/DMA
>    crypto: qce - Add LOCK and UNLOCK flag support
>    crypto: qce - Add support for lock aquire,lock release api.
>    crypto: qce - Add support for lock/unlock in skcipher
>    crypto: qce - Add support for lock/unlock in sha
>    crypto: qce - Add support for lock/unlock in aead
>    arm64: dts: qcom: ipq9574: enable bam pipe locking/unlocking
>    arm64: dts: qcom: ipq8074: enable bam pipe locking/unlocking
>    arm64: dts: qcom: ipq6018: enable bam pipe locking/unlocking
> 
>   .../devicetree/bindings/dma/qcom,bam-dma.yaml |   8 +
>   arch/arm64/boot/dts/qcom/ipq6018.dtsi         |   1 +
>   arch/arm64/boot/dts/qcom/ipq8074.dtsi         |   1 +
>   arch/arm64/boot/dts/qcom/ipq9574.dtsi         |   1 +
>   drivers/crypto/qce/aead.c                     |   4 +
>   drivers/crypto/qce/common.c                   | 142 +++++++----
>   drivers/crypto/qce/core.c                     |  13 +-
>   drivers/crypto/qce/core.h                     |  12 +
>   drivers/crypto/qce/dma.c                      | 232 ++++++++++++++++++
>   drivers/crypto/qce/dma.h                      |  26 +-
>   drivers/crypto/qce/sha.c                      |   4 +
>   drivers/crypto/qce/skcipher.c                 |   4 +
>   drivers/dma/qcom/bam_dma.c                    |  14 +-
>   include/linux/dmaengine.h                     |   6 +
>   14 files changed, 424 insertions(+), 44 deletions(-)
> 

-- 
// Caleb (they/them)

Re: [PATCH v2 00/16] Add cmd descriptor support

[Md Sadre Alam]

On 8/15/2024 6:38 PM, Caleb Connolly wrote:
> Hi,
> 
> A note for future patches, please scope your cover letter subject:
> 
> "dmaengine: qcom: bam_dma: add cmd descriptor support"

   Sure will add this in next patch.
> 
> On 15/08/2024 10:57, Md Sadre Alam wrote:
>> This series of patches will add command descriptor
>> support to read/write crypto engine register via
>> BAM/DMA
>>
>> We need this support because if there is multiple EE's
>> (Execution Environment) accessing the same CE then there
>> will be race condition. To avoid this race condition
>> BAM HW hsving LOC/UNLOCK feature on BAM pipes and this
>> LOCK/UNLOCK will be set via command descriptor only.
>>
>> Since each EE's having their dedicated BAM pipe, BAM allows
>> Locking and Unlocking on BAM pipe. So if one EE's requesting
>> for CE5 access then that EE's first has to LOCK the BAM pipe
>> while setting LOCK bit on command descriptor and then access
>> it. After finishing the request EE's has to UNLOCK the BAM pipe
>> so in this way we race condition will not happen.
>>
>> tested with "tcrypt.ko" and "kcapi" tool.
>>
>> Need help to test these all the patches on msm platform
> 
> DT changes here are only for a few IPQ platforms, please explain in the cover letter if this is some IPQ specific feature which doesn't exist on other platforms, or if you're only enabling it on IPQ.

    This feature is BAM hardware feature so its applicable for all the QCOM Soc where bam is there. Its not IPQ specific. Will add all the explanation in cover letter in next patch
> 
> Some broad strokes testing instructions (at the very least) and requirements (testing on what hardware?) aren't made obvious at all here.

    Sure will update in cover letter in next patch.
> 
> Kind regards,
>>
>> v2:
>>   * Addressed all the comments from v1
>>   * Added the dt-binding
>>   * Added locking/unlocking mechanism in bam driver
>>
>> v1:
>>   * https://lore.kernel.org/lkml/20231214114239.2635325-1-quic_mdalam@quicinc.com/
>>   * Initial set of patches for cmd descriptor support
>>
>> Md Sadre Alam (16):
>>    dt-bindings: dma: qcom,bam: Add bam pipe lock
>>    dmaengine: qcom: bam_dma: add bam_pipe_lock dt property
>>    dmaengine: qcom: bam_dma: add LOCK & UNLOCK flag support
>>    crypto: qce - Add support for crypto address read
>>    crypto: qce - Add bam dma support for crypto register r/w
>>    crypto: qce - Convert register r/w for skcipher via BAM/DMA
>>    crypto: qce - Convert register r/w for sha via BAM/DMA
>>    crypto: qce - Convert register r/w for aead via BAM/DMA
>>    crypto: qce - Add LOCK and UNLOCK flag support
>>    crypto: qce - Add support for lock aquire,lock release api.
>>    crypto: qce - Add support for lock/unlock in skcipher
>>    crypto: qce - Add support for lock/unlock in sha
>>    crypto: qce - Add support for lock/unlock in aead
>>    arm64: dts: qcom: ipq9574: enable bam pipe locking/unlocking
>>    arm64: dts: qcom: ipq8074: enable bam pipe locking/unlocking
>>    arm64: dts: qcom: ipq6018: enable bam pipe locking/unlocking
>>
>>   .../devicetree/bindings/dma/qcom,bam-dma.yaml |   8 +
>>   arch/arm64/boot/dts/qcom/ipq6018.dtsi         |   1 +
>>   arch/arm64/boot/dts/qcom/ipq8074.dtsi         |   1 +
>>   arch/arm64/boot/dts/qcom/ipq9574.dtsi         |   1 +
>>   drivers/crypto/qce/aead.c                     |   4 +
>>   drivers/crypto/qce/common.c                   | 142 +++++++----
>>   drivers/crypto/qce/core.c                     |  13 +-
>>   drivers/crypto/qce/core.h                     |  12 +
>>   drivers/crypto/qce/dma.c                      | 232 ++++++++++++++++++
>>   drivers/crypto/qce/dma.h                      |  26 +-
>>   drivers/crypto/qce/sha.c                      |   4 +
>>   drivers/crypto/qce/skcipher.c                 |   4 +
>>   drivers/dma/qcom/bam_dma.c                    |  14 +-
>>   include/linux/dmaengine.h                     |   6 +
>>   14 files changed, 424 insertions(+), 44 deletions(-)
>>
> 

Re: [PATCH v2 00/16] Add cmd descriptor support

[Bjorn Andersson]

On Fri, Aug 16, 2024 at 05:33:43PM GMT, Md Sadre Alam wrote:
> On 8/15/2024 6:38 PM, Caleb Connolly wrote:
[..]
> > On 15/08/2024 10:57, Md Sadre Alam wrote:
[..]
> > > 
> > > tested with "tcrypt.ko" and "kcapi" tool.
> > > 
> > > Need help to test these all the patches on msm platform
> > 
> > DT changes here are only for a few IPQ platforms, please explain in the cover letter if this is some IPQ specific feature which doesn't exist on other platforms, or if you're only enabling it on IPQ.
> 
>    This feature is BAM hardware feature so its applicable for all the QCOM Soc where bam is there. Its not IPQ specific. Will add all the explanation in cover letter in next patch

Please configure your email client such that your replies follow the
typical style of mail list discussions. I believe go/upstream has
instructions for this.

> > 
> > Some broad strokes testing instructions (at the very least) and requirements (testing on what hardware?) aren't made obvious at all here.
> 
>    Sure will update in cover letter in next patch.

I'm interested in these instructions as well, but no need to wait for
another version to provide these instructions. Please just reply here
(and then include them if there are future versions)

Regards,
Bjorn

Re: [PATCH v2 01/16] dt-bindings: dma: qcom,bam: Add bam pipe lock

[Bjorn Andersson]

On Thu, Aug 15, 2024 at 02:27:10PM GMT, Md Sadre Alam wrote:
> BAM having pipe locking mechanism. The Lock and Un-Lock bit
> should be set on CMD descriptor only. Upon encountering a
> descriptor with Lock bit set, the BAM will lock all other
> pipes not related to the current pipe group, and keep
> handling the current pipe only until it sees the Un-Lock
> set.

This describes the mechanism for mutual exclusion, but not really what
the patch does.

https://docs.kernel.org/process/submitting-patches.html#the-canonical-patch-format
states that you have 75 characters for your commit message, use them.

> 
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> ---
> 
> Change in [v2]
> 
> * Added initial support for dt-binding
> 
> Change in [v1]
> 
> * This patch was not included in [v1]
> 
>  Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
> index 3ad0d9b1fbc5..91cc2942aa62 100644
> --- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
> +++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
> @@ -77,6 +77,12 @@ properties:
>        Indicates that the bam is powered up by a remote processor but must be
>        initialized by the local processor.
>  
> +  qcom,bam_pipe_lock:

'_' is not a valid character in node names or properties.

> +    type: boolean
> +    description:
> +      Indicates that the bam pipe needs locking or not based on client driver
> +      sending the LOCK or UNLOK bit set on command descriptor.

Missing 'C'?

Regards,
Bjorn

> +
>    reg:
>      maxItems: 1
>  
> @@ -92,6 +98,8 @@ anyOf:
>        - qcom,powered-remotely
>    - required:
>        - qcom,controlled-remotely
> +  - required:
> +      - qcom,bam_pipe_lock
>    - required:
>        - clocks
>        - clock-names
> -- 
> 2.34.1
> 

Re: [PATCH v2 00/16] Add cmd descriptor support

[Bjorn Andersson]

On Thu, Aug 15, 2024 at 02:27:09PM GMT, Md Sadre Alam wrote:
> This series of patches will add command descriptor
> support to read/write crypto engine register via
> BAM/DMA
> 
> We need this support because if there is multiple EE's
> (Execution Environment) accessing the same CE then there
> will be race condition. To avoid this race condition
> BAM HW hsving LOC/UNLOCK feature on BAM pipes and this
> LOCK/UNLOCK will be set via command descriptor only.
> 
> Since each EE's having their dedicated BAM pipe, BAM allows
> Locking and Unlocking on BAM pipe. So if one EE's requesting
> for CE5 access then that EE's first has to LOCK the BAM pipe
> while setting LOCK bit on command descriptor and then access
> it. After finishing the request EE's has to UNLOCK the BAM pipe
> so in this way we race condition will not happen.
> 
> tested with "tcrypt.ko" and "kcapi" tool.
> 
> Need help to test these all the patches on msm platform
> 
> v2:
>  * Addressed all the comments from v1

Please describe the actual changes you're making between your versions.

>  * Added the dt-binding
>  * Added locking/unlocking mechanism in bam driver

Seems to me that this was already part of v1, as patch 6/11?

Regards,
Bjorn

> 
> v1:
>  * https://lore.kernel.org/lkml/20231214114239.2635325-1-quic_mdalam@quicinc.com/
>  * Initial set of patches for cmd descriptor support
> 
> Md Sadre Alam (16):
>   dt-bindings: dma: qcom,bam: Add bam pipe lock
>   dmaengine: qcom: bam_dma: add bam_pipe_lock dt property
>   dmaengine: qcom: bam_dma: add LOCK & UNLOCK flag support
>   crypto: qce - Add support for crypto address read
>   crypto: qce - Add bam dma support for crypto register r/w
>   crypto: qce - Convert register r/w for skcipher via BAM/DMA
>   crypto: qce - Convert register r/w for sha via BAM/DMA
>   crypto: qce - Convert register r/w for aead via BAM/DMA
>   crypto: qce - Add LOCK and UNLOCK flag support
>   crypto: qce - Add support for lock aquire,lock release api.
>   crypto: qce - Add support for lock/unlock in skcipher
>   crypto: qce - Add support for lock/unlock in sha
>   crypto: qce - Add support for lock/unlock in aead
>   arm64: dts: qcom: ipq9574: enable bam pipe locking/unlocking
>   arm64: dts: qcom: ipq8074: enable bam pipe locking/unlocking
>   arm64: dts: qcom: ipq6018: enable bam pipe locking/unlocking
> 
>  .../devicetree/bindings/dma/qcom,bam-dma.yaml |   8 +
>  arch/arm64/boot/dts/qcom/ipq6018.dtsi         |   1 +
>  arch/arm64/boot/dts/qcom/ipq8074.dtsi         |   1 +
>  arch/arm64/boot/dts/qcom/ipq9574.dtsi         |   1 +
>  drivers/crypto/qce/aead.c                     |   4 +
>  drivers/crypto/qce/common.c                   | 142 +++++++----
>  drivers/crypto/qce/core.c                     |  13 +-
>  drivers/crypto/qce/core.h                     |  12 +
>  drivers/crypto/qce/dma.c                      | 232 ++++++++++++++++++
>  drivers/crypto/qce/dma.h                      |  26 +-
>  drivers/crypto/qce/sha.c                      |   4 +
>  drivers/crypto/qce/skcipher.c                 |   4 +
>  drivers/dma/qcom/bam_dma.c                    |  14 +-
>  include/linux/dmaengine.h                     |   6 +
>  14 files changed, 424 insertions(+), 44 deletions(-)
> 
> -- 
> 2.34.1
> 

Re: [PATCH v2 03/16] dmaengine: qcom: bam_dma: add LOCK & UNLOCK flag support

[Bjorn Andersson]

On Thu, Aug 15, 2024 at 02:27:12PM GMT, Md Sadre Alam wrote:
> Add lock and unlock flag support on command descriptor.
> Once lock set in requester pipe, then the bam controller
> will lock all others pipe and process the request only
> from requester pipe. Unlocking only can be performed from
> the same pipe.
> 

Is the lock per channel, or for the whole BAM instance?

> If DMA_PREP_LOCK flag passed in command descriptor then requester
> of this transaction wanted to lock the BAM controller for this
> transaction so BAM driver should set LOCK bit for the HW descriptor.

You use the expression "this transaction" here, but if I understand the
calling code the lock is going to be held over multiple DMA operations
and even across asynchronous operations in the crypto driver.

DMA_PREP_LOCK indicates that this is the beginning of a transaction,
consisting of multiple operations that needs to happen while other EEs
are being locked out, and DMA_PREP_UNLOCK marks the end of the
transaction.

That said, I'm not entirely fond of the fact that these flags are not
just used on first and last operation in one sequence, but spread out.

Locking is hard, when you spread the responsibility of locking and
unlocking across different entities it's made harder...

> 
> If DMA_PREP_UNLOCK flag passed in command descriptor then requester
> of this transaction wanted to unlock the BAM controller.so BAM driver
> should set UNLOCK bit for the HW descriptor.
> 
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> ---
> 
> Change in [v2]
> 
> * Added LOCK and UNLOCK flag in bam driver
> 
> Change in [v1]
> 
> * This patch was not included in [v1]

v1 can be found here:
https://lore.kernel.org/all/20231214114239.2635325-7-quic_mdalam@quicinc.com/

And it was also posted once before that:
https://lore.kernel.org/all/1608215842-15381-1-git-send-email-mdalam@codeaurora.org/

In particular during the latter (i.e. first post) we had a rather long
discussion about this feature, so that's certainly worth linking to.

Looks like this series provides some answers to the questions we had
back then.

Regards,
Bjorn

> 
>  drivers/dma/qcom/bam_dma.c | 10 +++++++++-
>  include/linux/dmaengine.h  |  6 ++++++
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index 1ac7e250bdaa..ab3b5319aa68 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
> @@ -58,6 +58,8 @@ struct bam_desc_hw {
>  #define DESC_FLAG_EOB BIT(13)
>  #define DESC_FLAG_NWD BIT(12)
>  #define DESC_FLAG_CMD BIT(11)
> +#define DESC_FLAG_LOCK BIT(10)
> +#define DESC_FLAG_UNLOCK BIT(9)
>  
>  struct bam_async_desc {
>  	struct virt_dma_desc vd;
> @@ -692,9 +694,15 @@ static struct dma_async_tx_descriptor *bam_prep_slave_sg(struct dma_chan *chan,
>  		unsigned int curr_offset = 0;
>  
>  		do {
> -			if (flags & DMA_PREP_CMD)
> +			if (flags & DMA_PREP_CMD) {
>  				desc->flags |= cpu_to_le16(DESC_FLAG_CMD);
>  
> +				if (bdev->bam_pipe_lock && flags & DMA_PREP_LOCK)
> +					desc->flags |= cpu_to_le16(DESC_FLAG_LOCK);
> +				else if (bdev->bam_pipe_lock && flags & DMA_PREP_UNLOCK)
> +					desc->flags |= cpu_to_le16(DESC_FLAG_UNLOCK);
> +			}
> +
>  			desc->addr = cpu_to_le32(sg_dma_address(sg) +
>  						 curr_offset);
>  
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index b137fdb56093..70f23068bfdc 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -200,6 +200,10 @@ struct dma_vec {
>   *  transaction is marked with DMA_PREP_REPEAT will cause the new transaction
>   *  to never be processed and stay in the issued queue forever. The flag is
>   *  ignored if the previous transaction is not a repeated transaction.
> + *  @DMA_PREP_LOCK: tell the driver that there is a lock bit set on command
> + *  descriptor.
> + *  @DMA_PREP_UNLOCK: tell the driver that there is a un-lock bit set on command
> + *  descriptor.
>   */
>  enum dma_ctrl_flags {
>  	DMA_PREP_INTERRUPT = (1 << 0),
> @@ -212,6 +216,8 @@ enum dma_ctrl_flags {
>  	DMA_PREP_CMD = (1 << 7),
>  	DMA_PREP_REPEAT = (1 << 8),
>  	DMA_PREP_LOAD_EOT = (1 << 9),
> +	DMA_PREP_LOCK = (1 << 10),
> +	DMA_PREP_UNLOCK = (1 << 11),
>  };
>  
>  /**
> -- 
> 2.34.1
> 

Re: [PATCH v2 10/16] crypto: qce - Add support for lock aquire,lock release api.

[Bjorn Andersson]

On Thu, Aug 15, 2024 at 02:27:19PM GMT, Md Sadre Alam wrote:
> Add support for lock acquire and lock release api.
> When multiple EE's(Execution Environment) want to access
> CE5 then there will be race condition b/w multiple EE's.
> 
> Since each EE's having their dedicated BAM pipe, BAM allows
> Locking and Unlocking on BAM pipe. So if one EE's requesting
> for CE5 access then that EE's first has to LOCK the BAM pipe
> while setting LOCK bit on command descriptor and then access
> it. After finishing the request EE's has to UNLOCK the BAM pipe
> so in this way we race condition will not happen.

Does the lock/unlock need to happen on a dummy access before and after
the actual sequence? Is it not sufficient to lock/unlock on the first
and last operation?

Please squash this with the previous commit, if kept as explicit
operations, please squash it with the previous patch that introduces the
flags.

> 
> Added these two API qce_bam_acquire_lock() and qce_bam_release_lock()
> for the same.
> 
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> ---
> 
> Change in [v2]
> 
> * No chnage
> 
> Change in [v1]
> 
> * Added initial support for lock_acquire and lock_release
>   api.
> 
>  drivers/crypto/qce/common.c | 36 ++++++++++++++++++++++++++++++++++++
>  drivers/crypto/qce/core.h   |  2 ++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
> index ff96f6ba1fc5..a8eaffe41101 100644
> --- a/drivers/crypto/qce/common.c
> +++ b/drivers/crypto/qce/common.c
> @@ -617,3 +617,39 @@ void qce_get_version(struct qce_device *qce, u32 *major, u32 *minor, u32 *step)
>  	*minor = (val & CORE_MINOR_REV_MASK) >> CORE_MINOR_REV_SHIFT;
>  	*step = (val & CORE_STEP_REV_MASK) >> CORE_STEP_REV_SHIFT;
>  }
> +
> +int qce_bam_acquire_lock(struct qce_device *qce)
> +{
> +	int ret;
> +
> +	qce_clear_bam_transaction(qce);

It's not entirely obvious that a "lock" operation will invalidate any
pending operations.

> +
> +	/* This is just a dummy write to acquire lock on bam pipe */
> +	qce_write_reg_dma(qce, REG_AUTH_SEG_CFG, 0, 1);
> +
> +	ret = qce_submit_cmd_desc(qce, QCE_DMA_DESC_FLAG_LOCK);
> +	if (ret) {
> +		dev_err(qce->dev, "Error in Locking cmd descriptor\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +int qce_bam_release_lock(struct qce_device *qce)

What would be a reasonable response from the caller if this release
operation returns a failure? How do you expect it to recover?

> +{
> +	int ret;
> +
> +	qce_clear_bam_transaction(qce);
> +

In particularly not on "unlock".

Regards,
Bjorn

> +	/* This just dummy write to release lock on bam pipe*/
> +	qce_write_reg_dma(qce, REG_AUTH_SEG_CFG, 0, 1);
> +
> +	ret = qce_submit_cmd_desc(qce, QCE_DMA_DESC_FLAG_UNLOCK);
> +	if (ret) {
> +		dev_err(qce->dev, "Error in Un-Locking cmd descriptor\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> diff --git a/drivers/crypto/qce/core.h b/drivers/crypto/qce/core.h
> index bf28dedd1509..d01d810b60ad 100644
> --- a/drivers/crypto/qce/core.h
> +++ b/drivers/crypto/qce/core.h
> @@ -68,4 +68,6 @@ int qce_read_reg_dma(struct qce_device *qce, unsigned int offset, void *buff,
>  void qce_clear_bam_transaction(struct qce_device *qce);
>  int qce_submit_cmd_desc(struct qce_device *qce, unsigned long flags);
>  struct qce_bam_transaction *qce_alloc_bam_txn(struct qce_dma_data *dma);
> +int qce_bam_acquire_lock(struct qce_device *qce);
> +int qce_bam_release_lock(struct qce_device *qce);
>  #endif /* _CORE_H_ */
> -- 
> 2.34.1
> 

Re: [PATCH v2 14/16] arm64: dts: qcom: ipq9574: enable bam pipe locking/unlocking

[Bjorn Andersson]

On Thu, Aug 15, 2024 at 02:27:23PM GMT, Md Sadre Alam wrote:
> enable bam pipe locking/unlocking for ipq9507 SoC.

Note that the commit messages for the other non-dts commits will not
show up in the git history for this file. So, please follow
https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
and give some indication of "problem description", to give future
readers an idea why this is here.

> 
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> ---
> 
> Change in [v2]
> 
> * enabled locking/unlocking support for ipq9574
> 
> Change in [v1]
> 
> * This patch was not included in [v1]
> 
>  arch/arm64/boot/dts/qcom/ipq9574.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> index 48dfafea46a7..dacaec62ec39 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> @@ -262,6 +262,7 @@ cryptobam: dma-controller@704000 {
>  			#dma-cells = <1>;
>  			qcom,ee = <1>;
>  			qcom,controlled-remotely;
> +			qcom,bam_pipe_lock;

Per the question before about what does this actually lock. Is this a
property of the BAM controller, or the crypto channel?

Regards,
Bjorn

>  		};
>  
>  		crypto: crypto@73a000 {
> -- 
> 2.34.1
> 

Re: [PATCH v2 01/16] dt-bindings: dma: qcom,bam: Add bam pipe lock

[Krzysztof Kozlowski]

On 15/08/2024 10:57, Md Sadre Alam wrote:
> BAM having pipe locking mechanism. The Lock and Un-Lock bit
> should be set on CMD descriptor only. Upon encountering a
> descriptor with Lock bit set, the BAM will lock all other
> pipes not related to the current pipe group, and keep
> handling the current pipe only until it sees the Un-Lock
> set.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

> 
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> ---
> 
> Change in [v2]
> 
> * Added initial support for dt-binding
> 
> Change in [v1]
> 
> * This patch was not included in [v1]
> 
>  Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
> index 3ad0d9b1fbc5..91cc2942aa62 100644
> --- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
> +++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
> @@ -77,6 +77,12 @@ properties:
>        Indicates that the bam is powered up by a remote processor but must be
>        initialized by the local processor.
>  
> +  qcom,bam_pipe_lock:

Please follow DTS coding style.

> +    type: boolean
> +    description:
> +      Indicates that the bam pipe needs locking or not based on client driver
> +      sending the LOCK or UNLOK bit set on command descriptor.

You described the desired Linux feature or behavior, not the actual
hardware. The bindings are about the latter, so instead you need to
rephrase the property and its description to match actual hardware
capabilities/features/configuration etc.

> +
>    reg:
>      maxItems: 1
>  
> @@ -92,6 +98,8 @@ anyOf:
>        - qcom,powered-remotely
>    - required:
>        - qcom,controlled-remotely
> +  - required:
> +      - qcom,bam_pipe_lock

Why is it here? What do you want to achieve?

>    - required:
>        - clocks
>        - clock-names

Best regards,
Krzysztof

Re: [PATCH v2 02/16] dmaengine: qcom: bam_dma: add bam_pipe_lock dt property

[Krzysztof Kozlowski]

On 15/08/2024 10:57, Md Sadre Alam wrote:
> bam having locking and unlocking mechanism of bam pipes.
> Upon encountering a descriptor with Lock bit set, the
> BAM will lock all other pipes not related to the current
> pipe group, and keep handling the current pipe only until
> it sees the Un-Lock set , then it will release all locked
> pipes. The actual locking is done on the new descriptor
> fetching for publishing, i.e. locked pipe will not fetch
> new descriptors even if it got event/events adding more
> descriptors for this pipe.
> 
> Adding the bam_pipe_lock flag in bam driver to handle
> Lock and Un-Lock bit set on command descriptor.
> 
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> ---
> 
> Change in [v2]
> 
> * Added bam_pipe_lock dt property
> 
> Change in [v1]
> 
> * This patch was not included in [v1]
> 
>  drivers/dma/qcom/bam_dma.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index 5e7d332731e0..1ac7e250bdaa 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
> @@ -389,6 +389,7 @@ struct bam_device {
>  	u32 ee;
>  	bool controlled_remotely;
>  	bool powered_remotely;
> +	bool bam_pipe_lock;

There is no user of this property. It's just no-op. Split your code into
logical chunks, but logical chunk is not "I add field to structure which
is not used".


Best regards,
Krzysztof

Re: [PATCH v2 04/16] crypto: qce - Add support for crypto address read

[Krzysztof Kozlowski]

On 15/08/2024 10:57, Md Sadre Alam wrote:
> Get crypto base address from DT. This will use for
> command descriptor support for crypto register r/w
> via BAM/DMA

All your commit messages are oddly wrapped. This does not make reading
it easy...

> 
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> ---
> Change in [v2]
> 
> * Addressed all comments from v1
> 
> Change in [v1]
> 
> * Added support to read crypto base address from dt
> 
>  drivers/crypto/qce/core.c | 13 ++++++++++++-
>  drivers/crypto/qce/core.h |  1 +
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
> index 28b5fd823827..9b23a948078a 100644
> --- a/drivers/crypto/qce/core.c
> +++ b/drivers/crypto/qce/core.c
> @@ -192,6 +192,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct qce_device *qce;
> +	struct resource *res;
>  	int ret;
>  
>  	qce = devm_kzalloc(dev, sizeof(*qce), GFP_KERNEL);
> @@ -201,10 +202,16 @@ static int qce_crypto_probe(struct platform_device *pdev)
>  	qce->dev = dev;
>  	platform_set_drvdata(pdev, qce);
>  
> -	qce->base = devm_platform_ioremap_resource(pdev, 0);
> +	qce->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
>  	if (IS_ERR(qce->base))
>  		return PTR_ERR(qce->base);
>  
> +	qce->base_dma = dma_map_resource(dev, res->start,
> +					 resource_size(res),
> +					 DMA_BIDIRECTIONAL, 0);
> +	if (dma_mapping_error(dev, qce->base_dma))
> +		return -ENXIO;
> +
>  	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>  	if (ret < 0)
>  		return ret;

And how do you handle error paths?


> @@ -280,6 +287,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
>  static void qce_crypto_remove(struct platform_device *pdev)
>  {
>  	struct qce_device *qce = platform_get_drvdata(pdev);
> +	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  
>  	tasklet_kill(&qce->done_tasklet);
>  	qce_unregister_algs(qce);
> @@ -287,6 +295,9 @@ static void qce_crypto_remove(struct platform_device *pdev)
>  	clk_disable_unprepare(qce->bus);
>  	clk_disable_unprepare(qce->iface);
>  	clk_disable_unprepare(qce->core);
> +
> +	dma_unmap_resource(&pdev->dev, qce->base_dma, resource_size(res),
> +			   DMA_BIDIRECTIONAL, 0);

If you add code to the remove callback, not adding it to error paths is
suspicious by itself...

Best regards,
Krzysztof

Re: [PATCH v2 01/16] dt-bindings: dma: qcom,bam: Add bam pipe lock

[Md Sadre Alam]

On 8/16/2024 9:23 PM, Bjorn Andersson wrote:
> On Thu, Aug 15, 2024 at 02:27:10PM GMT, Md Sadre Alam wrote:
>> BAM having pipe locking mechanism. The Lock and Un-Lock bit
>> should be set on CMD descriptor only. Upon encountering a
>> descriptor with Lock bit set, the BAM will lock all other
>> pipes not related to the current pipe group, and keep
>> handling the current pipe only until it sees the Un-Lock
>> set.
> 
> This describes the mechanism for mutual exclusion, but not really what
> the patch does.
   Ok, will update in next patch.
> 
> https://docs.kernel.org/process/submitting-patches.html#the-canonical-patch-format
> states that you have 75 characters for your commit message, use them.
   ok, will update in next patch.
> 
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> ---
>>
>> Change in [v2]
>>
>> * Added initial support for dt-binding
>>
>> Change in [v1]
>>
>> * This patch was not included in [v1]
>>
>>   Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>> index 3ad0d9b1fbc5..91cc2942aa62 100644
>> --- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>> +++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>> @@ -77,6 +77,12 @@ properties:
>>         Indicates that the bam is powered up by a remote processor but must be
>>         initialized by the local processor.
>>   
>> +  qcom,bam_pipe_lock:
> 
> '_' is not a valid character in node names or properties.
    Ok, will update in next patch.
> 
>> +    type: boolean
>> +    description:
>> +      Indicates that the bam pipe needs locking or not based on client driver
>> +      sending the LOCK or UNLOK bit set on command descriptor.
> 
> Missing 'C'?
   Ok, will fix in next patch.
> 
> Regards,
> Bjorn
> 
>> +
>>     reg:
>>       maxItems: 1
>>   
>> @@ -92,6 +98,8 @@ anyOf:
>>         - qcom,powered-remotely
>>     - required:
>>         - qcom,controlled-remotely
>> +  - required:
>> +      - qcom,bam_pipe_lock
>>     - required:
>>         - clocks
>>         - clock-names
>> -- 
>> 2.34.1
>>

Re: [PATCH v2 00/16] Add cmd descriptor support

[Md Sadre Alam]

On 8/16/2024 9:31 PM, Bjorn Andersson wrote:
> On Thu, Aug 15, 2024 at 02:27:09PM GMT, Md Sadre Alam wrote:
>> This series of patches will add command descriptor
>> support to read/write crypto engine register via
>> BAM/DMA
>>
>> We need this support because if there is multiple EE's
>> (Execution Environment) accessing the same CE then there
>> will be race condition. To avoid this race condition
>> BAM HW hsving LOC/UNLOCK feature on BAM pipes and this
>> LOCK/UNLOCK will be set via command descriptor only.
>>
>> Since each EE's having their dedicated BAM pipe, BAM allows
>> Locking and Unlocking on BAM pipe. So if one EE's requesting
>> for CE5 access then that EE's first has to LOCK the BAM pipe
>> while setting LOCK bit on command descriptor and then access
>> it. After finishing the request EE's has to UNLOCK the BAM pipe
>> so in this way we race condition will not happen.
>>
>> tested with "tcrypt.ko" and "kcapi" tool.
>>
>> Need help to test these all the patches on msm platform
>>
>> v2:
>>   * Addressed all the comments from v1
> 
> Please describe the actual changes you're making between your versions.
   Sure , will update in next patch.
> 
>>   * Added the dt-binding
>>   * Added locking/unlocking mechanism in bam driver
> 
> Seems to me that this was already part of v1, as patch 6/11?
   Sorry, by mistake I have added this line in v2.
> 
> Regards,
> Bjorn
> 
>>
>> v1:
>>   * https://lore.kernel.org/lkml/20231214114239.2635325-1-quic_mdalam@quicinc.com/
>>   * Initial set of patches for cmd descriptor support
>>
>> Md Sadre Alam (16):
>>    dt-bindings: dma: qcom,bam: Add bam pipe lock
>>    dmaengine: qcom: bam_dma: add bam_pipe_lock dt property
>>    dmaengine: qcom: bam_dma: add LOCK & UNLOCK flag support
>>    crypto: qce - Add support for crypto address read
>>    crypto: qce - Add bam dma support for crypto register r/w
>>    crypto: qce - Convert register r/w for skcipher via BAM/DMA
>>    crypto: qce - Convert register r/w for sha via BAM/DMA
>>    crypto: qce - Convert register r/w for aead via BAM/DMA
>>    crypto: qce - Add LOCK and UNLOCK flag support
>>    crypto: qce - Add support for lock aquire,lock release api.
>>    crypto: qce - Add support for lock/unlock in skcipher
>>    crypto: qce - Add support for lock/unlock in sha
>>    crypto: qce - Add support for lock/unlock in aead
>>    arm64: dts: qcom: ipq9574: enable bam pipe locking/unlocking
>>    arm64: dts: qcom: ipq8074: enable bam pipe locking/unlocking
>>    arm64: dts: qcom: ipq6018: enable bam pipe locking/unlocking
>>
>>   .../devicetree/bindings/dma/qcom,bam-dma.yaml |   8 +
>>   arch/arm64/boot/dts/qcom/ipq6018.dtsi         |   1 +
>>   arch/arm64/boot/dts/qcom/ipq8074.dtsi         |   1 +
>>   arch/arm64/boot/dts/qcom/ipq9574.dtsi         |   1 +
>>   drivers/crypto/qce/aead.c                     |   4 +
>>   drivers/crypto/qce/common.c                   | 142 +++++++----
>>   drivers/crypto/qce/core.c                     |  13 +-
>>   drivers/crypto/qce/core.h                     |  12 +
>>   drivers/crypto/qce/dma.c                      | 232 ++++++++++++++++++
>>   drivers/crypto/qce/dma.h                      |  26 +-
>>   drivers/crypto/qce/sha.c                      |   4 +
>>   drivers/crypto/qce/skcipher.c                 |   4 +
>>   drivers/dma/qcom/bam_dma.c                    |  14 +-
>>   include/linux/dmaengine.h                     |   6 +
>>   14 files changed, 424 insertions(+), 44 deletions(-)
>>
>> -- 
>> 2.34.1
>>

Re: [PATCH v2 10/16] crypto: qce - Add support for lock aquire,lock release api.

[Md Sadre Alam]

On 8/16/2024 10:08 PM, Bjorn Andersson wrote:
> On Thu, Aug 15, 2024 at 02:27:19PM GMT, Md Sadre Alam wrote:
>> Add support for lock acquire and lock release api.
>> When multiple EE's(Execution Environment) want to access
>> CE5 then there will be race condition b/w multiple EE's.
>>
>> Since each EE's having their dedicated BAM pipe, BAM allows
>> Locking and Unlocking on BAM pipe. So if one EE's requesting
>> for CE5 access then that EE's first has to LOCK the BAM pipe
>> while setting LOCK bit on command descriptor and then access
>> it. After finishing the request EE's has to UNLOCK the BAM pipe
>> so in this way we race condition will not happen.
> 
> Does the lock/unlock need to happen on a dummy access before and after
> the actual sequence? Is it not sufficient to lock/unlock on the first
> and last operation?
   The locking/unlocking has to happen on command descriptor only, If we
   need locking/unlocking on data descriptor then we have to use dummy
   command descriptor only as per Hardware Programming Guide.

   Hardware Programming Guide state as below:
   Pipe Locking and Unlocking should appear ONLY in Command-Descriptor.
   In case a Lock is required on a Data Descriptor this can be implemented
   by a dummy Command-Descriptor with Lock/Unlock bit asserted preceding/
   following the Data Descriptor.
> 
> Please squash this with the previous commit, if kept as explicit
> operations, please squash it with the previous patch that introduces the
> flags.
   Ok
> 
>>
>> Added these two API qce_bam_acquire_lock() and qce_bam_release_lock()
>> for the same.
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> ---
>>
>> Change in [v2]
>>
>> * No chnage
>>
>> Change in [v1]
>>
>> * Added initial support for lock_acquire and lock_release
>>    api.
>>
>>   drivers/crypto/qce/common.c | 36 ++++++++++++++++++++++++++++++++++++
>>   drivers/crypto/qce/core.h   |  2 ++
>>   2 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/crypto/qce/common.c b/drivers/crypto/qce/common.c
>> index ff96f6ba1fc5..a8eaffe41101 100644
>> --- a/drivers/crypto/qce/common.c
>> +++ b/drivers/crypto/qce/common.c
>> @@ -617,3 +617,39 @@ void qce_get_version(struct qce_device *qce, u32 *major, u32 *minor, u32 *step)
>>   	*minor = (val & CORE_MINOR_REV_MASK) >> CORE_MINOR_REV_SHIFT;
>>   	*step = (val & CORE_STEP_REV_MASK) >> CORE_STEP_REV_SHIFT;
>>   }
>> +
>> +int qce_bam_acquire_lock(struct qce_device *qce)
>> +{
>> +	int ret;
>> +
>> +	qce_clear_bam_transaction(qce);
> 
> It's not entirely obvious that a "lock" operation will invalidate any
> pending operations.
   qce_clear_bam_transaction() api is not going to invalidate any pending
   thransaction. This is just an internal api which will set bam_transaction
   structure to 0 before starting new bam transaction.
> 
>> +
>> +	/* This is just a dummy write to acquire lock on bam pipe */
>> +	qce_write_reg_dma(qce, REG_AUTH_SEG_CFG, 0, 1);
>> +
>> +	ret = qce_submit_cmd_desc(qce, QCE_DMA_DESC_FLAG_LOCK);
>> +	if (ret) {
>> +		dev_err(qce->dev, "Error in Locking cmd descriptor\n");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +int qce_bam_release_lock(struct qce_device *qce)
> 
> What would be a reasonable response from the caller if this release
> operation returns a failure? How do you expect it to recover?
   If unlocking bam pipe failed means its a bam failure and we can abort
   the current transaction.
> 
>> +{
>> +	int ret;
>> +
>> +	qce_clear_bam_transaction(qce);
>> +
> 
> In particularly not on "unlock".
   qce_clear_bam_transaction() this is just to initialize with 0
   for bam transaction structure before any new transaction start.
> 
> Regards,
> Bjorn
> 
>> +	/* This just dummy write to release lock on bam pipe*/
>> +	qce_write_reg_dma(qce, REG_AUTH_SEG_CFG, 0, 1);
>> +
>> +	ret = qce_submit_cmd_desc(qce, QCE_DMA_DESC_FLAG_UNLOCK);
>> +	if (ret) {
>> +		dev_err(qce->dev, "Error in Un-Locking cmd descriptor\n");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> diff --git a/drivers/crypto/qce/core.h b/drivers/crypto/qce/core.h
>> index bf28dedd1509..d01d810b60ad 100644
>> --- a/drivers/crypto/qce/core.h
>> +++ b/drivers/crypto/qce/core.h
>> @@ -68,4 +68,6 @@ int qce_read_reg_dma(struct qce_device *qce, unsigned int offset, void *buff,
>>   void qce_clear_bam_transaction(struct qce_device *qce);
>>   int qce_submit_cmd_desc(struct qce_device *qce, unsigned long flags);
>>   struct qce_bam_transaction *qce_alloc_bam_txn(struct qce_dma_data *dma);
>> +int qce_bam_acquire_lock(struct qce_device *qce);
>> +int qce_bam_release_lock(struct qce_device *qce);
>>   #endif /* _CORE_H_ */
>> -- 
>> 2.34.1
>>

Re: [PATCH v2 14/16] arm64: dts: qcom: ipq9574: enable bam pipe locking/unlocking

[Md Sadre Alam]

On 8/16/2024 10:10 PM, Bjorn Andersson wrote:
> On Thu, Aug 15, 2024 at 02:27:23PM GMT, Md Sadre Alam wrote:
>> enable bam pipe locking/unlocking for ipq9507 SoC.
> 
> Note that the commit messages for the other non-dts commits will not
> show up in the git history for this file. So, please follow
> https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> and give some indication of "problem description", to give future
> readers an idea why this is here.
   Ok
> 
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> ---
>>
>> Change in [v2]
>>
>> * enabled locking/unlocking support for ipq9574
>>
>> Change in [v1]
>>
>> * This patch was not included in [v1]
>>
>>   arch/arm64/boot/dts/qcom/ipq9574.dtsi | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> index 48dfafea46a7..dacaec62ec39 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> @@ -262,6 +262,7 @@ cryptobam: dma-controller@704000 {
>>   			#dma-cells = <1>;
>>   			qcom,ee = <1>;
>>   			qcom,controlled-remotely;
>> +			qcom,bam_pipe_lock;
> 
> Per the question before about what does this actually lock. Is this a
> property of the BAM controller, or the crypto channel?
   This is the property of BAM controller.
> 
> Regards,
> Bjorn
> 
>>   		};
>>   
>>   		crypto: crypto@73a000 {
>> -- 
>> 2.34.1
>>

Re: [PATCH v2 03/16] dmaengine: qcom: bam_dma: add LOCK & UNLOCK flag support

[Md Sadre Alam]

On 8/16/2024 9:52 PM, Bjorn Andersson wrote:
> On Thu, Aug 15, 2024 at 02:27:12PM GMT, Md Sadre Alam wrote:
>> Add lock and unlock flag support on command descriptor.
>> Once lock set in requester pipe, then the bam controller
>> will lock all others pipe and process the request only
>> from requester pipe. Unlocking only can be performed from
>> the same pipe.
>>
> 
> Is the lock per channel, or for the whole BAM instance?
   This lock is for whole BAM instance. Once LOCK bit will
   set on initiator CMD descriptor BAM will lock all pipes
   which belongs to other EE's and Pipes not in the current
   group.
> 
>> If DMA_PREP_LOCK flag passed in command descriptor then requester
>> of this transaction wanted to lock the BAM controller for this
>> transaction so BAM driver should set LOCK bit for the HW descriptor.
> 
> You use the expression "this transaction" here, but if I understand the
> calling code the lock is going to be held over multiple DMA operations
> and even across asynchronous operations in the crypto driver.
   Yes its correct.
> 
> DMA_PREP_LOCK indicates that this is the beginning of a transaction,
> consisting of multiple operations that needs to happen while other EEs
> are being locked out, and DMA_PREP_UNLOCK marks the end of the
> transaction.
   Yes its correct.
> 
> That said, I'm not entirely fond of the fact that these flags are not
> just used on first and last operation in one sequence, but spread out.
   Yes its correct.
> 
> Locking is hard, when you spread the responsibility of locking and
> unlocking across different entities it's made harder...
   The locking/unlocking always synchronous because unlocking happening
   in the dma callback.
> 
>>
>> If DMA_PREP_UNLOCK flag passed in command descriptor then requester
>> of this transaction wanted to unlock the BAM controller.so BAM driver
>> should set UNLOCK bit for the HW descriptor.
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> ---
>>
>> Change in [v2]
>>
>> * Added LOCK and UNLOCK flag in bam driver
>>
>> Change in [v1]
>>
>> * This patch was not included in [v1]
> 
> v1 can be found here:
> https://lore.kernel.org/all/20231214114239.2635325-7-quic_mdalam@quicinc.com/
> 
> And it was also posted once before that:
> https://lore.kernel.org/all/1608215842-15381-1-git-send-email-mdalam@codeaurora.org/
> 
> In particular during the latter (i.e. first post) we had a rather long
> discussion about this feature, so that's certainly worth linking to.
> 
> Looks like this series provides some answers to the questions we had
> back then.
   Will add the link in next post
> 
> Regards,
> Bjorn
> 
>>
>>   drivers/dma/qcom/bam_dma.c | 10 +++++++++-
>>   include/linux/dmaengine.h  |  6 ++++++
>>   2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
>> index 1ac7e250bdaa..ab3b5319aa68 100644
>> --- a/drivers/dma/qcom/bam_dma.c
>> +++ b/drivers/dma/qcom/bam_dma.c
>> @@ -58,6 +58,8 @@ struct bam_desc_hw {
>>   #define DESC_FLAG_EOB BIT(13)
>>   #define DESC_FLAG_NWD BIT(12)
>>   #define DESC_FLAG_CMD BIT(11)
>> +#define DESC_FLAG_LOCK BIT(10)
>> +#define DESC_FLAG_UNLOCK BIT(9)
>>   
>>   struct bam_async_desc {
>>   	struct virt_dma_desc vd;
>> @@ -692,9 +694,15 @@ static struct dma_async_tx_descriptor *bam_prep_slave_sg(struct dma_chan *chan,
>>   		unsigned int curr_offset = 0;
>>   
>>   		do {
>> -			if (flags & DMA_PREP_CMD)
>> +			if (flags & DMA_PREP_CMD) {
>>   				desc->flags |= cpu_to_le16(DESC_FLAG_CMD);
>>   
>> +				if (bdev->bam_pipe_lock && flags & DMA_PREP_LOCK)
>> +					desc->flags |= cpu_to_le16(DESC_FLAG_LOCK);
>> +				else if (bdev->bam_pipe_lock && flags & DMA_PREP_UNLOCK)
>> +					desc->flags |= cpu_to_le16(DESC_FLAG_UNLOCK);
>> +			}
>> +
>>   			desc->addr = cpu_to_le32(sg_dma_address(sg) +
>>   						 curr_offset);
>>   
>> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
>> index b137fdb56093..70f23068bfdc 100644
>> --- a/include/linux/dmaengine.h
>> +++ b/include/linux/dmaengine.h
>> @@ -200,6 +200,10 @@ struct dma_vec {
>>    *  transaction is marked with DMA_PREP_REPEAT will cause the new transaction
>>    *  to never be processed and stay in the issued queue forever. The flag is
>>    *  ignored if the previous transaction is not a repeated transaction.
>> + *  @DMA_PREP_LOCK: tell the driver that there is a lock bit set on command
>> + *  descriptor.
>> + *  @DMA_PREP_UNLOCK: tell the driver that there is a un-lock bit set on command
>> + *  descriptor.
>>    */
>>   enum dma_ctrl_flags {
>>   	DMA_PREP_INTERRUPT = (1 << 0),
>> @@ -212,6 +216,8 @@ enum dma_ctrl_flags {
>>   	DMA_PREP_CMD = (1 << 7),
>>   	DMA_PREP_REPEAT = (1 << 8),
>>   	DMA_PREP_LOAD_EOT = (1 << 9),
>> +	DMA_PREP_LOCK = (1 << 10),
>> +	DMA_PREP_UNLOCK = (1 << 11),
>>   };
>>   
>>   /**
>> -- 
>> 2.34.1
>>

Re: [PATCH v2 01/16] dt-bindings: dma: qcom,bam: Add bam pipe lock

[Md Sadre Alam]

On 8/17/2024 2:38 PM, Krzysztof Kozlowski wrote:
> On 15/08/2024 10:57, Md Sadre Alam wrote:
>> BAM having pipe locking mechanism. The Lock and Un-Lock bit
>> should be set on CMD descriptor only. Upon encountering a
>> descriptor with Lock bit set, the BAM will lock all other
>> pipes not related to the current pipe group, and keep
>> handling the current pipe only until it sees the Un-Lock
>> set.
> 
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
   Ok , will update in next patch.
> 
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> ---
>>
>> Change in [v2]
>>
>> * Added initial support for dt-binding
>>
>> Change in [v1]
>>
>> * This patch was not included in [v1]
>>
>>   Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>> index 3ad0d9b1fbc5..91cc2942aa62 100644
>> --- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>> +++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>> @@ -77,6 +77,12 @@ properties:
>>         Indicates that the bam is powered up by a remote processor but must be
>>         initialized by the local processor.
>>   
>> +  qcom,bam_pipe_lock:
> 
> Please follow DTS coding style.
   Ok
> 
>> +    type: boolean
>> +    description:
>> +      Indicates that the bam pipe needs locking or not based on client driver
>> +      sending the LOCK or UNLOK bit set on command descriptor.
> 
> You described the desired Linux feature or behavior, not the actual
> hardware. The bindings are about the latter, so instead you need to
> rephrase the property and its description to match actual hardware
> capabilities/features/configuration etc.
   Ok, will update in next patch.
> 
>> +
>>     reg:
>>       maxItems: 1
>>   
>> @@ -92,6 +98,8 @@ anyOf:
>>         - qcom,powered-remotely
>>     - required:
>>         - qcom,controlled-remotely
>> +  - required:
>> +      - qcom,bam_pipe_lock
> 
> Why is it here? What do you want to achieve?
   This property added to achieve locking/unlocking
   of BAM pipe groups for mutual exclusion of resources
   that can be used across multiple EE's
> 
>>     - required:
>>         - clocks
>>         - clock-names
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 02/16] dmaengine: qcom: bam_dma: add bam_pipe_lock dt property

[Md Sadre Alam]

On 8/17/2024 2:38 PM, Krzysztof Kozlowski wrote:
> On 15/08/2024 10:57, Md Sadre Alam wrote:
>> bam having locking and unlocking mechanism of bam pipes.
>> Upon encountering a descriptor with Lock bit set, the
>> BAM will lock all other pipes not related to the current
>> pipe group, and keep handling the current pipe only until
>> it sees the Un-Lock set , then it will release all locked
>> pipes. The actual locking is done on the new descriptor
>> fetching for publishing, i.e. locked pipe will not fetch
>> new descriptors even if it got event/events adding more
>> descriptors for this pipe.
>>
>> Adding the bam_pipe_lock flag in bam driver to handle
>> Lock and Un-Lock bit set on command descriptor.
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> ---
>>
>> Change in [v2]
>>
>> * Added bam_pipe_lock dt property
>>
>> Change in [v1]
>>
>> * This patch was not included in [v1]
>>
>>   drivers/dma/qcom/bam_dma.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
>> index 5e7d332731e0..1ac7e250bdaa 100644
>> --- a/drivers/dma/qcom/bam_dma.c
>> +++ b/drivers/dma/qcom/bam_dma.c
>> @@ -389,6 +389,7 @@ struct bam_device {
>>   	u32 ee;
>>   	bool controlled_remotely;
>>   	bool powered_remotely;
>> +	bool bam_pipe_lock;
> 
> There is no user of this property. It's just no-op. Split your code into
> logical chunks, but logical chunk is not "I add field to structure which
> is not used".
   Ok ,will squash this patch accordingly.
> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 04/16] crypto: qce - Add support for crypto address read

[Md Sadre Alam]

On 8/17/2024 2:40 PM, Krzysztof Kozlowski wrote:
> On 15/08/2024 10:57, Md Sadre Alam wrote:
>> Get crypto base address from DT. This will use for
>> command descriptor support for crypto register r/w
>> via BAM/DMA
> 
> All your commit messages are oddly wrapped. This does not make reading
> it easy...
> 
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> ---
>> Change in [v2]
>>
>> * Addressed all comments from v1
>>
>> Change in [v1]
>>
>> * Added support to read crypto base address from dt
>>
>>   drivers/crypto/qce/core.c | 13 ++++++++++++-
>>   drivers/crypto/qce/core.h |  1 +
>>   2 files changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/crypto/qce/core.c b/drivers/crypto/qce/core.c
>> index 28b5fd823827..9b23a948078a 100644
>> --- a/drivers/crypto/qce/core.c
>> +++ b/drivers/crypto/qce/core.c
>> @@ -192,6 +192,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
>>   {
>>   	struct device *dev = &pdev->dev;
>>   	struct qce_device *qce;
>> +	struct resource *res;
>>   	int ret;
>>   
>>   	qce = devm_kzalloc(dev, sizeof(*qce), GFP_KERNEL);
>> @@ -201,10 +202,16 @@ static int qce_crypto_probe(struct platform_device *pdev)
>>   	qce->dev = dev;
>>   	platform_set_drvdata(pdev, qce);
>>   
>> -	qce->base = devm_platform_ioremap_resource(pdev, 0);
>> +	qce->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
>>   	if (IS_ERR(qce->base))
>>   		return PTR_ERR(qce->base);
>>   
>> +	qce->base_dma = dma_map_resource(dev, res->start,
>> +					 resource_size(res),
>> +					 DMA_BIDIRECTIONAL, 0);
>> +	if (dma_mapping_error(dev, qce->base_dma))
>> +		return -ENXIO;
>> +
>>   	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>>   	if (ret < 0)
>>   		return ret;
> 
> And how do you handle error paths?
   Ok , will fix the error path to cleanup the resources.
> 
> 
>> @@ -280,6 +287,7 @@ static int qce_crypto_probe(struct platform_device *pdev)
>>   static void qce_crypto_remove(struct platform_device *pdev)
>>   {
>>   	struct qce_device *qce = platform_get_drvdata(pdev);
>> +	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>   
>>   	tasklet_kill(&qce->done_tasklet);
>>   	qce_unregister_algs(qce);
>> @@ -287,6 +295,9 @@ static void qce_crypto_remove(struct platform_device *pdev)
>>   	clk_disable_unprepare(qce->bus);
>>   	clk_disable_unprepare(qce->iface);
>>   	clk_disable_unprepare(qce->core);
>> +
>> +	dma_unmap_resource(&pdev->dev, qce->base_dma, resource_size(res),
>> +			   DMA_BIDIRECTIONAL, 0);
> 
> If you add code to the remove callback, not adding it to error paths is
> suspicious by itself...
   Ok , will fix the error path to cleanup the resources.
> 
> Best regards,
> Krzysztof
> 
> 

Re: [PATCH v2 01/16] dt-bindings: dma: qcom,bam: Add bam pipe lock

[Krzysztof Kozlowski]

On 21/08/2024 18:34, Md Sadre Alam wrote:
> 
> 
> On 8/17/2024 2:38 PM, Krzysztof Kozlowski wrote:
>> On 15/08/2024 10:57, Md Sadre Alam wrote:
>>> BAM having pipe locking mechanism. The Lock and Un-Lock bit
>>> should be set on CMD descriptor only. Upon encountering a
>>> descriptor with Lock bit set, the BAM will lock all other
>>> pipes not related to the current pipe group, and keep
>>> handling the current pipe only until it sees the Un-Lock
>>> set.
>>
>> Please wrap commit message according to Linux coding style / submission
>> process (neither too early nor over the limit):
>> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>    Ok , will update in next patch.
>>
>>>
>>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>> ---
>>>
>>> Change in [v2]
>>>
>>> * Added initial support for dt-binding
>>>
>>> Change in [v1]
>>>
>>> * This patch was not included in [v1]
>>>
>>>   Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>>> index 3ad0d9b1fbc5..91cc2942aa62 100644
>>> --- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>>> +++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>>> @@ -77,6 +77,12 @@ properties:
>>>         Indicates that the bam is powered up by a remote processor but must be
>>>         initialized by the local processor.
>>>   
>>> +  qcom,bam_pipe_lock:
>>
>> Please follow DTS coding style.
>    Ok
>>
>>> +    type: boolean
>>> +    description:
>>> +      Indicates that the bam pipe needs locking or not based on client driver
>>> +      sending the LOCK or UNLOK bit set on command descriptor.
>>
>> You described the desired Linux feature or behavior, not the actual
>> hardware. The bindings are about the latter, so instead you need to
>> rephrase the property and its description to match actual hardware
>> capabilities/features/configuration etc.
>    Ok, will update in next patch.
>>
>>> +
>>>     reg:
>>>       maxItems: 1
>>>   
>>> @@ -92,6 +98,8 @@ anyOf:
>>>         - qcom,powered-remotely
>>>     - required:
>>>         - qcom,controlled-remotely
>>> +  - required:
>>> +      - qcom,bam_pipe_lock
>>
>> Why is it here? What do you want to achieve?
>    This property added to achieve locking/unlocking
>    of BAM pipe groups for mutual exclusion of resources
>    that can be used across multiple EE's

This explains me nothing. I am questioning the anyOf block. Why this is
the fourth method of controlling BAM? Anyway, if it is, then explain
this in commit msg.

Best regards,
Krzysztof

Re: [PATCH v2 01/16] dt-bindings: dma: qcom,bam: Add bam pipe lock

[Md Sadre Alam]

On 8/22/2024 11:57 AM, Krzysztof Kozlowski wrote:
> On 21/08/2024 18:34, Md Sadre Alam wrote:
>>
>>
>> On 8/17/2024 2:38 PM, Krzysztof Kozlowski wrote:
>>> On 15/08/2024 10:57, Md Sadre Alam wrote:
>>>> BAM having pipe locking mechanism. The Lock and Un-Lock bit
>>>> should be set on CMD descriptor only. Upon encountering a
>>>> descriptor with Lock bit set, the BAM will lock all other
>>>> pipes not related to the current pipe group, and keep
>>>> handling the current pipe only until it sees the Un-Lock
>>>> set.
>>>
>>> Please wrap commit message according to Linux coding style / submission
>>> process (neither too early nor over the limit):
>>> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>>     Ok , will update in next patch.
>>>
>>>>
>>>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>>> ---
>>>>
>>>> Change in [v2]
>>>>
>>>> * Added initial support for dt-binding
>>>>
>>>> Change in [v1]
>>>>
>>>> * This patch was not included in [v1]
>>>>
>>>>    Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml | 8 ++++++++
>>>>    1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>>>> index 3ad0d9b1fbc5..91cc2942aa62 100644
>>>> --- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>>>> +++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>>>> @@ -77,6 +77,12 @@ properties:
>>>>          Indicates that the bam is powered up by a remote processor but must be
>>>>          initialized by the local processor.
>>>>    
>>>> +  qcom,bam_pipe_lock:
>>>
>>> Please follow DTS coding style.
>>     Ok
>>>
>>>> +    type: boolean
>>>> +    description:
>>>> +      Indicates that the bam pipe needs locking or not based on client driver
>>>> +      sending the LOCK or UNLOK bit set on command descriptor.
>>>
>>> You described the desired Linux feature or behavior, not the actual
>>> hardware. The bindings are about the latter, so instead you need to
>>> rephrase the property and its description to match actual hardware
>>> capabilities/features/configuration etc.
>>     Ok, will update in next patch.
>>>
>>>> +
>>>>      reg:
>>>>        maxItems: 1
>>>>    
>>>> @@ -92,6 +98,8 @@ anyOf:
>>>>          - qcom,powered-remotely
>>>>      - required:
>>>>          - qcom,controlled-remotely
>>>> +  - required:
>>>> +      - qcom,bam_pipe_lock
>>>
>>> Why is it here? What do you want to achieve?
>>     This property added to achieve locking/unlocking
>>     of BAM pipe groups for mutual exclusion of resources
>>     that can be used across multiple EE's
> 
> This explains me nothing. I am questioning the anyOf block. Why this is
> the fourth method of controlling BAM? Anyway, if it is, then explain
> this in commit msg.
   This is the BAM property for locking/unlocking the BAM pipes.That's
   why I kept in anyOf block.
   Will explain in commit message in next patch.
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 01/16] dt-bindings: dma: qcom,bam: Add bam pipe lock

[Krzysztof Kozlowski]

On 22/08/2024 13:45, Md Sadre Alam wrote:
> 
> 
> On 8/22/2024 11:57 AM, Krzysztof Kozlowski wrote:
>> On 21/08/2024 18:34, Md Sadre Alam wrote:
>>>
>>>
>>> On 8/17/2024 2:38 PM, Krzysztof Kozlowski wrote:
>>>> On 15/08/2024 10:57, Md Sadre Alam wrote:
>>>>> BAM having pipe locking mechanism. The Lock and Un-Lock bit
>>>>> should be set on CMD descriptor only. Upon encountering a
>>>>> descriptor with Lock bit set, the BAM will lock all other
>>>>> pipes not related to the current pipe group, and keep
>>>>> handling the current pipe only until it sees the Un-Lock
>>>>> set.
>>>>
>>>> Please wrap commit message according to Linux coding style / submission
>>>> process (neither too early nor over the limit):
>>>> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>>>     Ok , will update in next patch.
>>>>
>>>>>
>>>>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>>>> ---
>>>>>
>>>>> Change in [v2]
>>>>>
>>>>> * Added initial support for dt-binding
>>>>>
>>>>> Change in [v1]
>>>>>
>>>>> * This patch was not included in [v1]
>>>>>
>>>>>    Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml | 8 ++++++++
>>>>>    1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>>>>> index 3ad0d9b1fbc5..91cc2942aa62 100644
>>>>> --- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>>>>> +++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>>>>> @@ -77,6 +77,12 @@ properties:
>>>>>          Indicates that the bam is powered up by a remote processor but must be
>>>>>          initialized by the local processor.
>>>>>    
>>>>> +  qcom,bam_pipe_lock:
>>>>
>>>> Please follow DTS coding style.
>>>     Ok
>>>>
>>>>> +    type: boolean
>>>>> +    description:
>>>>> +      Indicates that the bam pipe needs locking or not based on client driver
>>>>> +      sending the LOCK or UNLOK bit set on command descriptor.
>>>>
>>>> You described the desired Linux feature or behavior, not the actual
>>>> hardware. The bindings are about the latter, so instead you need to
>>>> rephrase the property and its description to match actual hardware
>>>> capabilities/features/configuration etc.
>>>     Ok, will update in next patch.
>>>>
>>>>> +
>>>>>      reg:
>>>>>        maxItems: 1
>>>>>    
>>>>> @@ -92,6 +98,8 @@ anyOf:
>>>>>          - qcom,powered-remotely
>>>>>      - required:
>>>>>          - qcom,controlled-remotely
>>>>> +  - required:
>>>>> +      - qcom,bam_pipe_lock
>>>>
>>>> Why is it here? What do you want to achieve?
>>>     This property added to achieve locking/unlocking
>>>     of BAM pipe groups for mutual exclusion of resources
>>>     that can be used across multiple EE's
>>
>> This explains me nothing. I am questioning the anyOf block. Why this is
>> the fourth method of controlling BAM? Anyway, if it is, then explain
>> this in commit msg.
>    This is the BAM property for locking/unlocking the BAM pipes.That's
>    why I kept in anyOf block.

You keep repeating the same. It's like poking me with the same comment
till I agree. I am done with this.

NAK. Provide proper rationale.

Best regards,
Krzysztof

Re: [PATCH v2 01/16] dt-bindings: dma: qcom,bam: Add bam pipe lock

[Manivannan Sadhasivam]

On Thu, Aug 15, 2024 at 02:27:10PM +0530, Md Sadre Alam wrote:
> BAM having pipe locking mechanism. The Lock and Un-Lock bit
> should be set on CMD descriptor only. Upon encountering a
> descriptor with Lock bit set, the BAM will lock all other
> pipes not related to the current pipe group, and keep
> handling the current pipe only until it sees the Un-Lock
> set.
> 
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> ---
> 
> Change in [v2]
> 
> * Added initial support for dt-binding
> 
> Change in [v1]
> 
> * This patch was not included in [v1]
> 
>  Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
> index 3ad0d9b1fbc5..91cc2942aa62 100644
> --- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
> +++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
> @@ -77,6 +77,12 @@ properties:
>        Indicates that the bam is powered up by a remote processor but must be
>        initialized by the local processor.
>  
> +  qcom,bam_pipe_lock:
> +    type: boolean
> +    description:
> +      Indicates that the bam pipe needs locking or not based on client driver
> +      sending the LOCK or UNLOK bit set on command descriptor.
> +

This looks like a pure driver implementation and doesn't belong to the DT at
all. Why can't you add a logic in the driver to use the lock based on some
detection mechanism?

- Mani

>    reg:
>      maxItems: 1
>  
> @@ -92,6 +98,8 @@ anyOf:
>        - qcom,powered-remotely
>    - required:
>        - qcom,controlled-remotely
> +  - required:
> +      - qcom,bam_pipe_lock
>    - required:
>        - clocks
>        - clock-names
> -- 
> 2.34.1
> 
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 01/16] dt-bindings: dma: qcom,bam: Add bam pipe lock

[Md Sadre Alam]

On 8/23/2024 9:09 PM, Manivannan Sadhasivam wrote:
> On Thu, Aug 15, 2024 at 02:27:10PM +0530, Md Sadre Alam wrote:
>> BAM having pipe locking mechanism. The Lock and Un-Lock bit
>> should be set on CMD descriptor only. Upon encountering a
>> descriptor with Lock bit set, the BAM will lock all other
>> pipes not related to the current pipe group, and keep
>> handling the current pipe only until it sees the Un-Lock
>> set.
>>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> ---
>>
>> Change in [v2]
>>
>> * Added initial support for dt-binding
>>
>> Change in [v1]
>>
>> * This patch was not included in [v1]
>>
>>   Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>> index 3ad0d9b1fbc5..91cc2942aa62 100644
>> --- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>> +++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>> @@ -77,6 +77,12 @@ properties:
>>         Indicates that the bam is powered up by a remote processor but must be
>>         initialized by the local processor.
>>   
>> +  qcom,bam_pipe_lock:
>> +    type: boolean
>> +    description:
>> +      Indicates that the bam pipe needs locking or not based on client driver
>> +      sending the LOCK or UNLOK bit set on command descriptor.
>> +
> 
> This looks like a pure driver implementation and doesn't belong to the DT at
> all. Why can't you add a logic in the driver to use the lock based on some
> detection mechanism?
   Sure , will use BAM_SW_VERSION register for detection mechanism, since this
   support only for bam version above 1.4.0.
> 
> - Mani
> 
>>     reg:
>>       maxItems: 1
>>   
>> @@ -92,6 +98,8 @@ anyOf:
>>         - qcom,powered-remotely
>>     - required:
>>         - qcom,controlled-remotely
>> +  - required:
>> +      - qcom,bam_pipe_lock
>>     - required:
>>         - clocks
>>         - clock-names
>> -- 
>> 2.34.1
>>
>>
> 

Re: [PATCH v2 01/16] dt-bindings: dma: qcom,bam: Add bam pipe lock

[Md Sadre Alam]

On 8/23/2024 2:37 PM, Krzysztof Kozlowski wrote:
> On 22/08/2024 13:45, Md Sadre Alam wrote:
>>
>>
>> On 8/22/2024 11:57 AM, Krzysztof Kozlowski wrote:
>>> On 21/08/2024 18:34, Md Sadre Alam wrote:
>>>>
>>>>
>>>> On 8/17/2024 2:38 PM, Krzysztof Kozlowski wrote:
>>>>> On 15/08/2024 10:57, Md Sadre Alam wrote:
>>>>>> BAM having pipe locking mechanism. The Lock and Un-Lock bit
>>>>>> should be set on CMD descriptor only. Upon encountering a
>>>>>> descriptor with Lock bit set, the BAM will lock all other
>>>>>> pipes not related to the current pipe group, and keep
>>>>>> handling the current pipe only until it sees the Un-Lock
>>>>>> set.
>>>>>
>>>>> Please wrap commit message according to Linux coding style / submission
>>>>> process (neither too early nor over the limit):
>>>>> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
>>>>      Ok , will update in next patch.
>>>>>
>>>>>>
>>>>>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>>>>>> ---
>>>>>>
>>>>>> Change in [v2]
>>>>>>
>>>>>> * Added initial support for dt-binding
>>>>>>
>>>>>> Change in [v1]
>>>>>>
>>>>>> * This patch was not included in [v1]
>>>>>>
>>>>>>     Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml | 8 ++++++++
>>>>>>     1 file changed, 8 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>>>>>> index 3ad0d9b1fbc5..91cc2942aa62 100644
>>>>>> --- a/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/dma/qcom,bam-dma.yaml
>>>>>> @@ -77,6 +77,12 @@ properties:
>>>>>>           Indicates that the bam is powered up by a remote processor but must be
>>>>>>           initialized by the local processor.
>>>>>>     
>>>>>> +  qcom,bam_pipe_lock:
>>>>>
>>>>> Please follow DTS coding style.
>>>>      Ok
>>>>>
>>>>>> +    type: boolean
>>>>>> +    description:
>>>>>> +      Indicates that the bam pipe needs locking or not based on client driver
>>>>>> +      sending the LOCK or UNLOK bit set on command descriptor.
>>>>>
>>>>> You described the desired Linux feature or behavior, not the actual
>>>>> hardware. The bindings are about the latter, so instead you need to
>>>>> rephrase the property and its description to match actual hardware
>>>>> capabilities/features/configuration etc.
>>>>      Ok, will update in next patch.
>>>>>
>>>>>> +
>>>>>>       reg:
>>>>>>         maxItems: 1
>>>>>>     
>>>>>> @@ -92,6 +98,8 @@ anyOf:
>>>>>>           - qcom,powered-remotely
>>>>>>       - required:
>>>>>>           - qcom,controlled-remotely
>>>>>> +  - required:
>>>>>> +      - qcom,bam_pipe_lock
>>>>>
>>>>> Why is it here? What do you want to achieve?
>>>>      This property added to achieve locking/unlocking
>>>>      of BAM pipe groups for mutual exclusion of resources
>>>>      that can be used across multiple EE's
>>>
>>> This explains me nothing. I am questioning the anyOf block. Why this is
>>> the fourth method of controlling BAM? Anyway, if it is, then explain
>>> this in commit msg.
>>     This is the BAM property for locking/unlocking the BAM pipes.That's
>>     why I kept in anyOf block.
> 
> You keep repeating the same. It's like poking me with the same comment
> till I agree. I am done with this.
> 
> NAK. Provide proper rationale.
   Sorry, I misunderstood your review comment. Now as Mani suggested will
   keep this implementation in driver itself. Will drop the binding patch
   in next revision.
> 
> Best regards,
> Krzysztof
> 
> 

Re: [PATCH v2 00/16] Add cmd descriptor support

[Md Sadre Alam]

On 8/16/2024 9:15 PM, Bjorn Andersson wrote:
> On Fri, Aug 16, 2024 at 05:33:43PM GMT, Md Sadre Alam wrote:
>> On 8/15/2024 6:38 PM, Caleb Connolly wrote:
> [..]
>>> On 15/08/2024 10:57, Md Sadre Alam wrote:
> [..]
>>>>
>>>> tested with "tcrypt.ko" and "kcapi" tool.
>>>>
>>>> Need help to test these all the patches on msm platform
>>>
>>> DT changes here are only for a few IPQ platforms, please explain in the cover letter if this is some IPQ specific feature which doesn't exist on other platforms, or if you're only enabling it on IPQ.
>>
>>     This feature is BAM hardware feature so its applicable for all the QCOM Soc where bam is there. Its not IPQ specific. Will add all the explanation in cover letter in next patch
> 
> Please configure your email client such that your replies follow the
> typical style of mail list discussions. I believe go/upstream has
> instructions for this.
   Ok
> 
>>>
>>> Some broad strokes testing instructions (at the very least) and requirements (testing on what hardware?) aren't made obvious at all here.
>>
>>     Sure will update in cover letter in next patch.
> 
> I'm interested in these instructions as well, but no need to wait for
> another version to provide these instructions. Please just reply here
> (and then include them if there are future versions)

   Requirements:
   In QCE crypto driver we are accessing the crypto engine registers directly via CPU read/write so its possible
   if other subsystem possibly Trust Zone also tries to perform some crypto operations simultaneously, a race
   condition will be created and this could result in undefined behavior.

   To avoid this behavior we need to use BAM HW LOCK/UNLOCK feature on BAM pipes, and this LOCK/UNLOCK will
   be set via sending a command descriptor, where the HLOS/TZ QCE crypto driver prepares a command descriptor
   with a dummy write operation on one of the QCE crypto engine register and pass the LOCK/UNLOCK flag along
   with it.

   This feature tested with tcrypt.ko and "libkcapi" with all the AES algorithm supported by QCE crypto engine.
   Tested on IPQ9574 and qcm6490.LE chipset.

   insmod tcrypt.ko mode=101
   insmod tcrypt.ko mode=102
   insmod tcrypt.ko mode=155
   insmod tcrypt.ko mode=180
   insmod tcrypt.ko mode=181
   insmod tcrypt.ko mode=182
   insmod tcrypt.ko mode=185
   insmod tcrypt.ko mode=186
   insmod tcrypt.ko mode=212
   insmod tcrypt.ko mode=216
   insmod tcrypt.ko mode=403
   insmod tcrypt.ko mode=404
   insmod tcrypt.ko mode=500
   insmod tcrypt.ko mode=501
   insmod tcrypt.ko mode=502
   insmod tcrypt.ko mode=600
   insmod tcrypt.ko mode=601
   insmod tcrypt.ko mode=602

   Encryption command line:
  ./kcapi -x 1 -e -c "cbc(aes)" -k
  8d7dd9b0170ce0b5f2f8e1aa768e01e91da8bfc67fd486d081b28254c99eb423 -i
  7fbc02ebf5b93322329df9bfccb635af -p 48981da18e4bb9ef7e2e3162d16b1910
  * 8b19050f66582cb7f7e4b6c873819b71
  *
  Decryption command line:
  * $ ./kcapi -x 1 -c "cbc(aes)" -k
  3023b2418ea59a841757dcf07881b3a8def1c97b659a4dad -i
  95aa5b68130be6fcf5cabe7d9f898a41 -q c313c6b50145b69a77b33404cb422598
  * 836de0065f9d6f6a3dd2c53cd17e33a

  * $ ./kcapi -x 3 -c sha256 -p 38f86d
  * cc42f645c5aa76ac3154b023359b665375fc3ae42f025fe961fb0f65205ad70e
  * $ ./kcapi -x 3 -c sha256 -p bbb300ac5eda9d
  * 61f7b48577a613fbdfe0d6d90b49985e07a42c99e7a439b6efb76d5ec71b3d30

  ./kcapi -x 12 -c "hmac(sha256)" -k
  0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b -i
  000102030405060708090a0b0c -p f0f1f2f3f4f5f6f7f8f9 -b 42
  *
  3cb25f25faacd57a90434f64d0362f2a2d2d0a90cf1a5a4c5db02d56ecc4c5bf3400720
  8d5b887185865

  Paraller test with two different EE's (Execution Environment)

        EE1 (Trust Zone)                                               EE2 (HLOS)

  There is a TZ application which                             "libkcapi" or "tcrypt.ko" will run in continous loop
  will do continious enc/dec with                                  to do enc/dec with different algorithm supported
  different AES algorithm supported                                QCE crypto engine.
  by QCE crypto engine.

     1) dummy write with LOCK bit set                           1) dummy write with LOCK bit set

     2) bam will lock all other pipes which not                 2) bam will lock all other pipes which not
        belongs to current EE's, i.e HLOS pipe                     belongs to current EE's, i.e tz pipe and
        and keep handling current pipe only.                       keep handling current pipe only.

     3) tz prepare data descriptor and submit to CE5            3) hlos prepare data descriptor and submit to CE5

     4) dummy write with UNLOCK bit set                         4) dummy write with UNLOCK bit set

     5) bam will release all the locked pipes                   5) bam will release all the locked pipes

  Upon encountering a descriptor with Lock bit set, the BAM will lock all other pipes not related to the current pipe group,
  and keep handling the current pipe only until it sees the Un-Lock set (then it will release all locked pipes). The actual
  locking is done on the new descriptor fetching for publishing, i.e. locked pipe will not fetch new descriptors even if it
  got event/events adding more descriptors for this pipe.

> 
> Regards,
> Bjorn