1. Patchew
  2. linux
  3. [PATCH 0/4] bugfix some issues under abnormal scenarios.
  4. View patch

[PATCH 4/4] hisi_acc_vfio_pci: fix the queue parameter anomaly issue

Longfang Liu posted 4 patches 1 month ago
There is a newer version of this series
[PATCH 4/4] hisi_acc_vfio_pci: fix the queue parameter anomaly issue
Posted by Longfang Liu 1 month ago 
When the number of QPs initialized by the device, as read via vft, is zero,
it indicates either an abnormal device configuration or an abnormal read
result.
Returning 0 directly in this case would allow the live migration operation
to complete successfully, leading to incorrect parameter configuration after
migration and preventing the service from recovering normal functionality.
Therefore, in such situations, an error should be returned to roll back the
live migration operation.

Signed-off-by: Longfang Liu <liulongfang@huawei.com>
---
 drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
index 394f1952a7ed..e0cc20f5f38b 100644
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
+++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
@@ -406,7 +406,7 @@ static int vf_qm_check_match(struct hisi_acc_vf_core_device *hisi_acc_vdev,
 	struct hisi_qm *pf_qm = hisi_acc_vdev->pf_qm;
 	struct device *dev = &vf_qm->pdev->dev;
 	u32 que_iso_state;
-	int ret;
+	int qp_num, ret;
 
 	if (migf->total_length < QM_MATCH_SIZE || hisi_acc_vdev->match_done)
 		return 0;
@@ -423,18 +423,18 @@ static int vf_qm_check_match(struct hisi_acc_vf_core_device *hisi_acc_vdev,
 	}
 
 	/* VF qp num check */
-	ret = qm_get_vft(vf_qm, &vf_qm->qp_base);
-	if (ret <= 0) {
+	qp_num = qm_get_vft(vf_qm, &vf_qm->qp_base);
+	if (qp_num <= 0) {
 		dev_err(dev, "failed to get vft qp nums\n");
-		return ret;
+		return -EINVAL;
 	}
 
-	if (ret != vf_data->qp_num) {
+	if (qp_num != vf_data->qp_num) {
 		dev_err(dev, "failed to match VF qp num\n");
 		return -EINVAL;
 	}
 
-	vf_qm->qp_num = ret;
+	vf_qm->qp_num = qp_num;
 
 	/* VF isolation state check */
 	ret = qm_read_regs(pf_qm, QM_QUE_ISO_CFG_V, &que_iso_state, 1);
-- 
2.24.0
Re: [PATCH 4/4] hisi_acc_vfio_pci: fix the queue parameter anomaly issue
Posted by Alex Williamson 3 weeks, 2 days ago 
On Sun, 4 Jan 2026 15:07:06 +0800
Longfang Liu <liulongfang@huawei.com> wrote:

> When the number of QPs initialized by the device, as read via vft, is zero,
> it indicates either an abnormal device configuration or an abnormal read
> result.
> Returning 0 directly in this case would allow the live migration operation
> to complete successfully, leading to incorrect parameter configuration after
> migration and preventing the service from recovering normal functionality.
> Therefore, in such situations, an error should be returned to roll back the
> live migration operation.
> 
> Signed-off-by: Longfang Liu <liulongfang@huawei.com>
> ---
>  drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> index 394f1952a7ed..e0cc20f5f38b 100644
> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> @@ -406,7 +406,7 @@ static int vf_qm_check_match(struct hisi_acc_vf_core_device *hisi_acc_vdev,
>  	struct hisi_qm *pf_qm = hisi_acc_vdev->pf_qm;
>  	struct device *dev = &vf_qm->pdev->dev;
>  	u32 que_iso_state;
> -	int ret;
> +	int qp_num, ret;
>  
>  	if (migf->total_length < QM_MATCH_SIZE || hisi_acc_vdev->match_done)
>  		return 0;
> @@ -423,18 +423,18 @@ static int vf_qm_check_match(struct hisi_acc_vf_core_device *hisi_acc_vdev,
>  	}
>  
>  	/* VF qp num check */
> -	ret = qm_get_vft(vf_qm, &vf_qm->qp_base);
> -	if (ret <= 0) {
> +	qp_num = qm_get_vft(vf_qm, &vf_qm->qp_base);
> +	if (qp_num <= 0) {
>  		dev_err(dev, "failed to get vft qp nums\n");
> -		return ret;
> +		return -EINVAL;
>  	}

Do you really want to clobber the errno or should this be something
like:

		return qp_num < 0 ? qp_num : -EINVAL;

And if you do that it might make sense to continue to use ret rather
than add the new variable.  Thanks,

Alex

>  
> -	if (ret != vf_data->qp_num) {
> +	if (qp_num != vf_data->qp_num) {
>  		dev_err(dev, "failed to match VF qp num\n");
>  		return -EINVAL;
>  	}
>  
> -	vf_qm->qp_num = ret;
> +	vf_qm->qp_num = qp_num;
>  
>  	/* VF isolation state check */
>  	ret = qm_read_regs(pf_qm, QM_QUE_ISO_CFG_V, &que_iso_state, 1);
Re: [PATCH 4/4] hisi_acc_vfio_pci: fix the queue parameter anomaly issue
Posted by liulongfang 2 weeks, 6 days ago 
On 2026/1/17 1:07, Alex Williamson wrote:
> On Sun, 4 Jan 2026 15:07:06 +0800
> Longfang Liu <liulongfang@huawei.com> wrote:
> 
>> When the number of QPs initialized by the device, as read via vft, is zero,
>> it indicates either an abnormal device configuration or an abnormal read
>> result.
>> Returning 0 directly in this case would allow the live migration operation
>> to complete successfully, leading to incorrect parameter configuration after
>> migration and preventing the service from recovering normal functionality.
>> Therefore, in such situations, an error should be returned to roll back the
>> live migration operation.
>>
>> Signed-off-by: Longfang Liu <liulongfang@huawei.com>
>> ---
>>  drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
>> index 394f1952a7ed..e0cc20f5f38b 100644
>> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
>> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
>> @@ -406,7 +406,7 @@ static int vf_qm_check_match(struct hisi_acc_vf_core_device *hisi_acc_vdev,
>>  	struct hisi_qm *pf_qm = hisi_acc_vdev->pf_qm;
>>  	struct device *dev = &vf_qm->pdev->dev;
>>  	u32 que_iso_state;
>> -	int ret;
>> +	int qp_num, ret;
>>  
>>  	if (migf->total_length < QM_MATCH_SIZE || hisi_acc_vdev->match_done)
>>  		return 0;
>> @@ -423,18 +423,18 @@ static int vf_qm_check_match(struct hisi_acc_vf_core_device *hisi_acc_vdev,
>>  	}
>>  
>>  	/* VF qp num check */
>> -	ret = qm_get_vft(vf_qm, &vf_qm->qp_base);
>> -	if (ret <= 0) {
>> +	qp_num = qm_get_vft(vf_qm, &vf_qm->qp_base);
>> +	if (qp_num <= 0) {
>>  		dev_err(dev, "failed to get vft qp nums\n");
>> -		return ret;
>> +		return -EINVAL;
>>  	}
> 
> Do you really want to clobber the errno or should this be something
> like:
> 
> 		return qp_num < 0 ? qp_num : -EINVAL;
> 
> And if you do that it might make sense to continue to use ret rather
> than add the new variable.  Thanks,
>

OK, your proposed fix doesn't require introducing a new variable.
I'll address these issues in the next version.

Thanks.
Longfang.

> Alex
> 
>>  
>> -	if (ret != vf_data->qp_num) {
>> +	if (qp_num != vf_data->qp_num) {
>>  		dev_err(dev, "failed to match VF qp num\n");
>>  		return -EINVAL;
>>  	}
>>  
>> -	vf_qm->qp_num = ret;
>> +	vf_qm->qp_num = qp_num;
>>  
>>  	/* VF isolation state check */
>>  	ret = qm_read_regs(pf_qm, QM_QUE_ISO_CFG_V, &que_iso_state, 1);
> 
> .
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.