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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> 




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux