| 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: <7758a29a-b035-42b3-94d6-4ec0d8471f3b@huawei.com>
Date: Sat, 29 Nov 2025 09:37:13 +0800
From: huangchenghai <huangchenghai2@...wei.com>
To: liulongfang <liulongfang@...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
在 2025/11/28 17:03, liulongfang 写道:
> 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
Thanks for the reminder, I will change `if (qps[j]->ref_count != 1)` to
`if (atomic_read(&qps[j]->qp_status.flags) == QP_START)`.
Regards,
Chenghai
>> +
>> + 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