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

[liulongfang <liulongfang@huawei.com>]

On 2026/9/20 22:26, Alex Williamson wrote:
> 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.

How about changing it to a dev_info() then? Since users will notice errors when initiating
a migration, an explicit log is necessary to pinpoint the cause and facilitate subsequent
troubleshooting.

> 
>> +
>>  	/* 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,
>

Even if left unchanged here, a later patch will anyway replace this complex logic with an is_virtfn check
to handle the pci_resource_len inspection during probe.

Thanks.
Longfang.

> Alex
> .
>