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

[Alex Williamson <alex@shazbot.org>]

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