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

[liulongfang <liulongfang@huawei.com>]

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);
> 
> .
>