Re: [PATCH v3 1/3] hisi_acc_vfio_pci: fix live migration enable conditions for PF passthrough

[Alex Williamson]

On Mon, 31 Aug 2026 17:09:49 +0800
Longfang Liu <liulongfang@xxxxxxxxxx> wrote:

> When a PF device is bound to the live migration driver in passthrough mode,
> it cannot support live migration functionality, and key pointers will
> remain uninitialized. Although most migration functions within the driver
> are unreachable, low-level error handling callbacks may be triggered
> directly, causing a crash due to null pointer dereference.
> The fix involves adding validity checks at three entry points: device
> probe, error handling, and migration initialization. If the pointer is
> invalid, the operation is exited or rejected directly to avoid crashes,
> while redundant internal checks are removed.
> 
> Fixes: b0eed085903e ("hisi_acc_vfio_pci: Add support for VFIO live migration")
> Signed-off-by: Longfang Liu <liulongfang@xxxxxxxxxx>
> ---
>  .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c    | 24 ++++++++++++++-----
>  1 file changed, 18 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 86362ec424a5..e95d0ab0f11a 100644
> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> @@ -1154,9 +1154,14 @@ static void hisi_acc_vf_pci_reset_prepare(struct pci_dev *pdev)
>  {
>  	struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_drvdata(pdev);
>  	struct hisi_qm *qm = hisi_acc_vdev->pf_qm;
> -	struct device *dev = &qm->pdev->dev;
> +	struct device *dev = &pdev->dev;
>  	u32 delay = 0;
>  
> +	if (!qm || !qm->io_base) {
> +		dev_err(dev, "PF QM not available for reset\n");
> +		return;
> +	}
> +
>  	/* All reset requests need to be queued for processing */
>  	while (test_and_set_bit(QM_RESETTING, &qm->misc_ctl)) {
>  		msleep(1);
> @@ -1174,8 +1179,12 @@ static void hisi_acc_vf_pci_aer_reset_done(struct pci_dev *pdev)
>  	struct hisi_acc_vf_core_device *hisi_acc_vdev = hisi_acc_drvdata(pdev);
>  	struct hisi_qm *qm = hisi_acc_vdev->pf_qm;
>  
> -	if (hisi_acc_vdev->set_reset_flag)
> -		clear_bit(QM_RESETTING, &qm->misc_ctl);
> +	if (hisi_acc_vdev->set_reset_flag) {
> +		if (qm && qm->io_base)
> +			clear_bit(QM_RESETTING, &qm->misc_ctl);
> +		else
> +			dev_err(&pdev->dev, "PF QM not available for reset done\n");
> +	}

set_reset_flag is only set by reset_prepare, which per the previous
chunk can only occur if qm && qm->io_base, so this chunk is redundant.
Why not just promote the mig_ops tests in both?

>  
>  	if (!hisi_acc_vdev->core_device.vdev.mig_ops)
>  		return;
> @@ -1565,6 +1574,11 @@ static int hisi_acc_vfio_pci_migrn_init_dev(struct vfio_device *core_vdev)
>  	struct pci_dev *pdev = to_pci_dev(core_vdev->dev);
>  	struct hisi_qm *pf_qm = hisi_acc_get_pf_qm(pdev);
>  
> +	if (!pf_qm) {
> +		dev_err(&pdev->dev, "PF driver not loaded, cannot enable migration\n");
> +		return -ENODEV;
> +	}

This function is only reached via hisi_acc_vfio_pci_migrn_ops, which is
already validated in probe to have a pf_qm with version >= QM_HW_V3.

> +
>  	hisi_acc_vdev->vf_id = pci_iov_vf_id(pdev) + 1;
>  	hisi_acc_vdev->pf_qm = pf_qm;
>  	hisi_acc_vdev->vf_dev = pdev;
> @@ -1670,13 +1684,11 @@ static int hisi_acc_vfio_pci_probe(struct pci_dev *pdev, const struct pci_device
>  	struct hisi_acc_vf_core_device *hisi_acc_vdev;
>  	const struct vfio_device_ops *ops = &hisi_acc_vfio_pci_ops;
>  	struct hisi_qm *pf_qm;
> -	int vf_id;
>  	int ret;
>  
>  	pf_qm = hisi_acc_get_pf_qm(pdev);
>  	if (pf_qm && pf_qm->ver >= QM_HW_V3) {
> -		vf_id = pci_iov_vf_id(pdev);
> -		if (vf_id >= 0)
> +		if (pdev->is_virtfn)
>  			ops = &hisi_acc_vfio_pci_migrn_ops;
>  		else
>  			pci_warn(pdev, "migration support failed, continue with generic interface\n");

This is not reachable as a VF:

static struct hisi_qm *hisi_acc_get_pf_qm(struct pci_dev *pdev)
{
        struct hisi_qm  *pf_qm;
        struct pci_driver *pf_driver;

        if (!pdev->is_virtfn)
                return NULL;


pf_qm is NULL, the branch is never taken for a PF.  Also:

int pci_iov_vf_id(struct pci_dev *dev)
{
        struct pci_dev *pf;

        if (!dev->is_virtfn)
                return -EINVAL;

So even the redundant test is already here.  What are you trying to
accomplish in this chunk?  Thanks,

Alex