| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ffb14448-a127-317e-40a0-fec431c4cc04@huawei.com>
Date: Fri, 28 Nov 2025 17:03:35 +0800
From: liulongfang <liulongfang@...wei.com>
To: Chenghai Huang <huangchenghai2@...wei.com>, <herbert@...dor.apana.org.au>,
<davem@...emloft.net>
CC: <linux-kernel@...r.kernel.org>, <linux-crypto@...r.kernel.org>,
<fanghao11@...wei.com>, <qianweili@...wei.com>, <linwenkai6@...ilicon.com>,
<wangzhou1@...ilicon.com>, <lizhi206@...wei.com>, <taoqi10@...wei.com>
Subject: Re: [PATCH v3 07/11] crypto: hisilicon/qm - add reference counting to
queues for tfm kernel reuse
On 2025/11/22 15:49, Chenghai Huang wrote:
> Add reference counting to queues. When all queues are occupied, tfm
> will reuse queues with the same algorithm type that have already
> been allocated in the kernel. The corresponding queue will be
> released when the reference count reaches 1.
>
> Signed-off-by: Chenghai Huang <huangchenghai2@...wei.com>
> Signed-off-by: Weili Qian <qianweili@...wei.com>
> ---
> drivers/crypto/hisilicon/qm.c | 81 +++++++++++++++++++++++++++--------
> include/linux/hisi_acc_qm.h | 1 +
> 2 files changed, 65 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/crypto/hisilicon/qm.c b/drivers/crypto/hisilicon/qm.c
> index 28256f64aa3c..6ff189941300 100644
> --- a/drivers/crypto/hisilicon/qm.c
> +++ b/drivers/crypto/hisilicon/qm.c
> @@ -2002,7 +2002,38 @@ static void hisi_qm_unset_hw_reset(struct hisi_qp *qp)
> *addr = 0;
> }
>
> -static struct hisi_qp *qm_create_qp_nolock(struct hisi_qm *qm, u8 alg_type)
> +static struct hisi_qp *find_shareable_qp(struct hisi_qm *qm, u8 alg_type, bool is_in_kernel)
> +{
> + struct device *dev = &qm->pdev->dev;
> + struct hisi_qp *share_qp = NULL;
> + struct hisi_qp *qp;
> + u32 ref_count = ~0;
> + int i;
> +
> + if (!is_in_kernel)
> + goto queues_busy;
> +
> + for (i = 0; i < qm->qp_num; i++) {
> + qp = &qm->qp_array[i];
> + if (qp->is_in_kernel && qp->alg_type == alg_type && qp->ref_count < ref_count) {
> + ref_count = qp->ref_count;
> + share_qp = qp;
> + }
> + }
> +
> + if (share_qp) {
> + share_qp->ref_count++;
> + return share_qp;
> + }
> +
> +queues_busy:
> + dev_info_ratelimited(dev, "All %u queues of QM are busy and no shareable queue\n",
> + qm->qp_num);
> + atomic64_inc(&qm->debug.dfx.create_qp_err_cnt);
> + return ERR_PTR(-EBUSY);
> +}
> +
> +static struct hisi_qp *qm_create_qp_nolock(struct hisi_qm *qm, u8 alg_type, bool is_in_kernel)
> {
> struct device *dev = &qm->pdev->dev;
> struct hisi_qp *qp;
> @@ -2013,17 +2044,14 @@ static struct hisi_qp *qm_create_qp_nolock(struct hisi_qm *qm, u8 alg_type)
> return ERR_PTR(-EPERM);
> }
>
> - if (qm->qp_in_used == qm->qp_num) {
> - dev_info_ratelimited(dev, "All %u queues of QM are busy!\n",
> - qm->qp_num);
> - atomic64_inc(&qm->debug.dfx.create_qp_err_cnt);
> - return ERR_PTR(-EBUSY);
> - }
> + /* Try to find a shareable queue when all queues are busy */
> + if (qm->qp_in_used == qm->qp_num)
> + return find_shareable_qp(qm, alg_type, is_in_kernel);
>
> qp_id = idr_alloc_cyclic(&qm->qp_idr, NULL, 0, qm->qp_num, GFP_ATOMIC);
> if (qp_id < 0) {
> - dev_info_ratelimited(dev, "All %u queues of QM are busy!\n",
> - qm->qp_num);
> + dev_info_ratelimited(dev, "All %u queues of QM are busy, in_used = %u!\n",
> + qm->qp_num, qm->qp_in_used);
> atomic64_inc(&qm->debug.dfx.create_qp_err_cnt);
> return ERR_PTR(-EBUSY);
> }
> @@ -2034,10 +2062,10 @@ static struct hisi_qp *qm_create_qp_nolock(struct hisi_qm *qm, u8 alg_type)
>
> qp->event_cb = NULL;
> qp->req_cb = NULL;
> - qp->qp_id = qp_id;
> qp->alg_type = alg_type;
> - qp->is_in_kernel = true;
> + qp->is_in_kernel = is_in_kernel;
> qm->qp_in_used++;
> + qp->ref_count = 1;
>
> return qp;
> }
> @@ -2059,7 +2087,7 @@ static struct hisi_qp *hisi_qm_create_qp(struct hisi_qm *qm, u8 alg_type)
> return ERR_PTR(ret);
>
> down_write(&qm->qps_lock);
> - qp = qm_create_qp_nolock(qm, alg_type);
> + qp = qm_create_qp_nolock(qm, alg_type, false);
> up_write(&qm->qps_lock);
>
> if (IS_ERR(qp))
> @@ -2458,7 +2486,6 @@ static int hisi_qm_uacce_get_queue(struct uacce_device *uacce,
> qp->uacce_q = q;
> qp->event_cb = qm_qp_event_notifier;
> qp->pasid = arg;
> - qp->is_in_kernel = false;
>
> return 0;
> }
> @@ -3532,6 +3559,9 @@ static void qm_release_qp_nolock(struct hisi_qp *qp)
> {
> struct hisi_qm *qm = qp->qm;
>
> + if (--qp->ref_count)
> + return;
> +
> qm->qp_in_used--;
> idr_remove(&qm->qp_idr, qp->qp_id);
> }
> @@ -3551,7 +3581,10 @@ void hisi_qm_free_qps(struct hisi_qp **qps, int qp_num)
> down_write(&qps[0]->qm->qps_lock);
>
> for (i = qp_num - 1; i >= 0; i--) {
> - qm_stop_qp_nolock(qps[i]);
> + if (qps[i]->ref_count == 1) {
> + qm_stop_qp_nolock(qps[i]);
> + qm_pm_put_sync(qps[i]->qm);
> + }
> qm_release_qp_nolock(qps[i]);
> }
>
> @@ -3576,16 +3609,27 @@ static int qm_get_and_start_qp(struct hisi_qm *qm, int qp_num, struct hisi_qp **
>
> down_write(&qm->qps_lock);
> for (i = 0; i < qp_num; i++) {
> - qps[i] = qm_create_qp_nolock(qm, alg_type[i]);
> + qps[i] = qm_create_qp_nolock(qm, alg_type[i], true);
> if (IS_ERR(qps[i])) {
> goto free_qp;
> }
> }
>
> for (j = 0; j < qp_num; j++) {
> + if (qps[j]->ref_count != 1)
> + continue;
You will encounter an issue here: if a queue is reused multiple times, the reference count
in the subsequent find_shareable_qp will automatically increase to a value greater than 1.
In this case, the queue actually needs to be accessed using qm_start_qp_nolock,
but it is skipped here, leading to an abnormal behavior.
Thanks.
Longfang
> +
> + ret = qm_pm_get_sync(qm);
> + if (ret) {
> + ret = -EINVAL;
> + goto stop_qp;
> + }
> +
> ret = qm_start_qp_nolock(qps[j], 0);
> - if (ret)
> + if (ret) {
> + qm_pm_put_sync(qm);
> goto stop_qp;
> + }
> }
> up_write(&qm->qps_lock);
>
> @@ -3593,7 +3637,10 @@ static int qm_get_and_start_qp(struct hisi_qm *qm, int qp_num, struct hisi_qp **
>
> stop_qp:
> for (j--; j >= 0; j--)
> - qm_stop_qp_nolock(qps[j]);
> + if (qps[j]->ref_count == 1) {
> + qm_stop_qp_nolock(qps[j]);
> + qm_pm_put_sync(qm);
> + }
> free_qp:
> for (i--; i >= 0; i--)
> qm_release_qp_nolock(qps[i]);
> diff --git a/include/linux/hisi_acc_qm.h b/include/linux/hisi_acc_qm.h
> index 4cf418a41fe4..26032d98e9bd 100644
> --- a/include/linux/hisi_acc_qm.h
> +++ b/include/linux/hisi_acc_qm.h
> @@ -472,6 +472,7 @@ struct hisi_qp {
> u16 pasid;
> struct uacce_queue *uacce_q;
>
> + u32 ref_count;
> spinlock_t qp_lock;
> struct instance_backlog backlog;
> const void **msg;
>
Powered by blists - more mailing lists