Re: [PATCH v4 1/2] hisi_acc_vfio_pci: fix NULL dereference in reset_prepare on PF passthrough

[Alex Williamson <alex@shazbot.org>]

On Fri, 18 Sep 2026 16:42:43 +0800
Longfang Liu <liulongfang@huawei.com> wrote:

> When a PF is bound to the driver via driver_override and passed
> through to a VM, its pf_qm stays NULL. The PCI error handler
> reset_prepare() runs during open_device through
> pci_try_reset_function(), before the mig_ops gate, and dereferences
> the NULL pf_qm for the timeout log, crashing the kernel.
> 
> Move the mig_ops check to the entry of reset_prepare() and
> aer_reset_done() so non-migration devices skip the QM_RESETTING
> coordination. Also clear set_reset_flag together with QM_RESETTING
> in aer_reset_done(); the flag was never cleared before, so a later
> timed-out reset could release a foreign lock. Replace
> pci_iov_vf_id() >= 0 with pdev->is_virtfn in probe() for clearer
> on-VF gating.
> 
> Fixes: b0eed085903e ("hisi_acc_vfio_pci: Add support for VFIO live migration")
> Fixes: a22099ed7936f ("hisi_acc_vfio_pci: fix VF reset timeout issue")
> Signed-off-by: Longfang Liu <liulongfang@huawei.com>
> ---
>  .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c    | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 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..8ff69c8d1ff7 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 (!hisi_acc_vdev->core_device.vdev.mig_ops) {
> +		dev_err(dev, "device not support migration\n");
> +		return;
> +	}

Why is not supporting migration worthy of a dev_err()?!  Resets are a
normal operation.  Generating a log to nag the lack of migration
support on every reset is unacceptable.

> +
>  	/* All reset requests need to be queued for processing */
>  	while (test_and_set_bit(QM_RESETTING, &qm->misc_ctl)) {
>  		msleep(1);
> @@ -1174,12 +1179,14 @@ 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->core_device.vdev.mig_ops)
>  		return;
>  
> +	if (hisi_acc_vdev->set_reset_flag) {
> +		clear_bit(QM_RESETTING, &qm->misc_ctl);
> +		hisi_acc_vdev->set_reset_flag = false;
> +	}
> +
>  	mutex_lock(&hisi_acc_vdev->state_mutex);
>  	hisi_acc_vf_reset(hisi_acc_vdev);
>  	mutex_unlock(&hisi_acc_vdev->state_mutex);
> @@ -1670,13 +1677,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");

I still reject the redundant is_virtfn check here.  I don't find
support for the claim that it's a convention among variant drivers, nor
does it do anything here or to the next patch.  pf_qm is
deterministically NULL for pdev->is_virtfn.  Thanks,

Alex