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@xxxxxxxxxxx>
> ---
>
> 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
>
