[PATCH v3 0/3] hisi_acc_vfio_pci: fix three driver issues

[PATCH v3 0/3] hisi_acc_vfio_pci: fix three driver issues

[Longfang Liu]

This series of patches fixes three issues discovered in specific scenarios
within the driver: one caused by PF device passthrough, one sticky-latch
defect in the reset path, and one triggered by a mismatch between hardware
version and memory page size (QM_HW_V3 + 64KB pages).

Longfang Liu (3):
  hisi_acc_vfio_pci: fix live migration enable conditions for PF
    passthrough
  hisi_acc_vfio_pci: clear set_reset_flag after reset completed
  hisi_acc_vfio_pci: reject live migration on 64KB page with QM_HW_V3
    hardware

 .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c    | 43 ++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 6 deletions(-)

--
2.43.0

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

[Longfang Liu]

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@huawei.com>
---
 .../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");
+	}
 
 	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;
+	}
+
 	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");
-- 
2.43.0

[PATCH v3 2/3] hisi_acc_vfio_pci: clear set_reset_flag after reset completed

[Longfang Liu]

set_reset_flag is a sticky latch set in reset_prepare() when this driver
acquires the QM_RESETTING lock, but it is never cleared. If a later reset
attempt times out in reset_prepare() because another agent already holds
the lock, aer_reset_done() sees the stale true flag and calls clear_bit()
on QM_RESETTING, prematurely releasing a lock owned by the other agent.

Clear set_reset_flag right after clearing the QM_RESETTING bit in
aer_reset_done(), so every reset cycle is self-contained and a timed-out
reset cannot release a foreign lock.

Fixes: a22099ed7936f ("hisi_acc_vfio_pci: fix VF reset timeout issue")
Signed-off-by: Longfang Liu <liulongfang@huawei.com>
---
 drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
index e95d0ab0f11a..4abed2e49986 100644
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
+++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
@@ -1180,10 +1180,12 @@ static void hisi_acc_vf_pci_aer_reset_done(struct pci_dev *pdev)
 	struct hisi_qm *qm = hisi_acc_vdev->pf_qm;
 
 	if (hisi_acc_vdev->set_reset_flag) {
-		if (qm && qm->io_base)
+		if (qm && qm->io_base) {
 			clear_bit(QM_RESETTING, &qm->misc_ctl);
-		else
+			hisi_acc_vdev->set_reset_flag = false;
+		} else {
 			dev_err(&pdev->dev, "PF QM not available for reset done\n");
+		}
 	}
 
 	if (!hisi_acc_vdev->core_device.vdev.mig_ops)
-- 
2.43.0

[PATCH v3 3/3] hisi_acc_vfio_pci: reject live migration on 64KB page with QM_HW_V3 hardware

[Longfang Liu]

In the scenario combining QM_HW_V3 hardware with 64KB large pages, the
device functional region and migration registers share the BAR2 physical
page, resulting in a lack of isolation that triggers security issues.
More critically, since KVM does not support the specific 16-byte read/write
instructions used by the guest driver, operating under the KVM emulated
device scheme will lead to Guest kernel calltrace issues.
To prevent Guest exceptions and avoid security vulnerabilities, the
driver directly disables live migration functionality in this scenario.
This ensures the system fails cleanly with an error early during startup,
while other configuration modes remain unaffected and can still utilize
passthrough functionality normally.

Fixes: b0eed085903e ("hisi_acc_vfio_pci: Add support for VFIO live migration")
Signed-off-by: Longfang Liu <liulongfang@huawei.com>
---
 drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
index 4abed2e49986..4f12cab96545 100644
--- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
+++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
@@ -1210,6 +1210,23 @@ static int hisi_acc_vf_qm_init(struct hisi_acc_vf_core_device *hisi_acc_vdev)
 	else
 		hisi_acc_vdev->drv_mode = HW_ACC_MIG_VF_CTRL;
 
+	/*
+	 * On VF_CTRL hardware, BAR2 holds 32KB functional + 32KB
+	 * migration registers. When the host page exceeds 32KB, the
+	 * two share one physical page and cannot be isolated by mmap.
+	 * Reject the open so QEMU fails cleanly at startup.
+	 */
+	if (hisi_acc_vdev->drv_mode == HW_ACC_MIG_VF_CTRL) {
+		resource_size_t func_len =
+			pci_resource_len(vf_dev, VFIO_PCI_BAR2_REGION_INDEX) >> 1;
+
+		if (func_len < PAGE_SIZE) {
+			dev_err(&vf_dev->dev,
+				"migration not supported on 64KB pages with QM_HW_V3\n");
+			return -EINVAL;
+		}
+	}
+
 	if (hisi_acc_vdev->drv_mode == HW_ACC_MIG_PF_CTRL) {
 		/*
 		 * On hardware platforms greater than QM_HW_V3, the migration function
-- 
2.43.0

Re: [PATCH v3 3/3] hisi_acc_vfio_pci: reject live migration on 64KB page with QM_HW_V3 hardware

[Alex Williamson]

On Mon, 31 Aug 2026 17:09:51 +0800
Longfang Liu <liulongfang@huawei.com> wrote:

> In the scenario combining QM_HW_V3 hardware with 64KB large pages, the
> device functional region and migration registers share the BAR2 physical
> page, resulting in a lack of isolation that triggers security issues.
> More critically, since KVM does not support the specific 16-byte read/write
> instructions used by the guest driver, operating under the KVM emulated
> device scheme will lead to Guest kernel calltrace issues.
> To prevent Guest exceptions and avoid security vulnerabilities, the
> driver directly disables live migration functionality in this scenario.
> This ensures the system fails cleanly with an error early during startup,
> while other configuration modes remain unaffected and can still utilize
> passthrough functionality normally.
> 
> Fixes: b0eed085903e ("hisi_acc_vfio_pci: Add support for VFIO live migration")
> Signed-off-by: Longfang Liu <liulongfang@huawei.com>
> ---
>  drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> index 4abed2e49986..4f12cab96545 100644
> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> @@ -1210,6 +1210,23 @@ static int hisi_acc_vf_qm_init(struct hisi_acc_vf_core_device *hisi_acc_vdev)
>  	else
>  		hisi_acc_vdev->drv_mode = HW_ACC_MIG_VF_CTRL;
>  
> +	/*
> +	 * On VF_CTRL hardware, BAR2 holds 32KB functional + 32KB
> +	 * migration registers. When the host page exceeds 32KB, the
> +	 * two share one physical page and cannot be isolated by mmap.
> +	 * Reject the open so QEMU fails cleanly at startup.
> +	 */
> +	if (hisi_acc_vdev->drv_mode == HW_ACC_MIG_VF_CTRL) {
> +		resource_size_t func_len =
> +			pci_resource_len(vf_dev, VFIO_PCI_BAR2_REGION_INDEX) >> 1;
> +
> +		if (func_len < PAGE_SIZE) {
> +			dev_err(&vf_dev->dev,
> +				"migration not supported on 64KB pages with QM_HW_V3\n");
> +			return -EINVAL;
> +		}
> +	}
> +

Sashiko correctly identifies that this breaks any assignment on 64K
pages, which is contradictory to the comment that indicates this is
only intended to disable migration support.  This should instead be
invoking the non-migration path in the probe function.  Thanks,

Alex

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@huawei.com> 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@huawei.com>
> ---
>  .../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

Re: [PATCH v3 3/3] hisi_acc_vfio_pci: reject live migration on 64KB page with QM_HW_V3 hardware

[liulongfang]

On 2026/9/12 1:37, Alex Williamson wrote:
> On Mon, 31 Aug 2026 17:09:51 +0800
> Longfang Liu <liulongfang@huawei.com> wrote:
> 
>> In the scenario combining QM_HW_V3 hardware with 64KB large pages, the
>> device functional region and migration registers share the BAR2 physical
>> page, resulting in a lack of isolation that triggers security issues.
>> More critically, since KVM does not support the specific 16-byte read/write
>> instructions used by the guest driver, operating under the KVM emulated
>> device scheme will lead to Guest kernel calltrace issues.
>> To prevent Guest exceptions and avoid security vulnerabilities, the
>> driver directly disables live migration functionality in this scenario.
>> This ensures the system fails cleanly with an error early during startup,
>> while other configuration modes remain unaffected and can still utilize
>> passthrough functionality normally.
>>
>> Fixes: b0eed085903e ("hisi_acc_vfio_pci: Add support for VFIO live migration")
>> Signed-off-by: Longfang Liu <liulongfang@huawei.com>
>> ---
>>  drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
>> index 4abed2e49986..4f12cab96545 100644
>> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
>> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
>> @@ -1210,6 +1210,23 @@ static int hisi_acc_vf_qm_init(struct hisi_acc_vf_core_device *hisi_acc_vdev)
>>  	else
>>  		hisi_acc_vdev->drv_mode = HW_ACC_MIG_VF_CTRL;
>>  
>> +	/*
>> +	 * On VF_CTRL hardware, BAR2 holds 32KB functional + 32KB
>> +	 * migration registers. When the host page exceeds 32KB, the
>> +	 * two share one physical page and cannot be isolated by mmap.
>> +	 * Reject the open so QEMU fails cleanly at startup.
>> +	 */
>> +	if (hisi_acc_vdev->drv_mode == HW_ACC_MIG_VF_CTRL) {
>> +		resource_size_t func_len =
>> +			pci_resource_len(vf_dev, VFIO_PCI_BAR2_REGION_INDEX) >> 1;
>> +
>> +		if (func_len < PAGE_SIZE) {
>> +			dev_err(&vf_dev->dev,
>> +				"migration not supported on 64KB pages with QM_HW_V3\n");
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
> 
> Sashiko correctly identifies that this breaks any assignment on 64K
> pages, which is contradictory to the comment that indicates this is
> only intended to disable migration support.  This should instead be
> invoking the non-migration path in the probe function.  Thanks,
>

It is a reasonable approach to directly select the non-migration ops in the probe
function based on the logic check.
I plan to implement this in the next revision and complete testing and verification.

Thanks.
Longfang.

> Alex
> 
> .
> 

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

[liulongfang]

On 2026/9/12 1:37, Alex Williamson wrote:
> On Mon, 31 Aug 2026 17:09:49 +0800
> Longfang Liu <liulongfang@huawei.com> 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@huawei.com>
>> ---
>>  .../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,
>

Thank you for the review. Regarding the first two checks in `aer_reset_done` and `migrn_init_dev`,
you are correct that they are redundant; we will remove them in the next revision.

However, the PF passthrough scenario still crashes. The calltrace we actually tested occurs on
the following path:


hisi_acc_vfio_pci_open_device
  → vfio_pci_core_enable
    → pci_try_reset_function
      → hisi_acc_vf_pci_reset_prepare   ← NULL pointer dereference


`reset_prepare` is a PCI error handler callback invoked by `pci_try_reset_function` inside
`vfio_pci_core_enable`, which happens before the `mig_ops` check in `open_device`.
At this point, the PF's `pf_qm` is NULL, and `reset_prepare` dereferences `&qm->pdev->dev`,
causing an oops:


Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
pc : hisi_acc_vf_pci_reset_prepare+0x34/0xc0
Call trace:
 hisi_acc_vf_pci_reset_prepare
 pci_dev_save_and_disable
 pci_try_reset_function
 vfio_pci_core_enable
 hisi_acc_vfio_pci_open_device
 vfio_df_open


This shows that the `vf_id`-based check in `probe` does not actually prevent the PF
from reaching `reset_prepare`;
the PF still triggers the calltrace through this path. The fix here will follow your
suggestion: elevate the `mig_ops` check at the entry of `reset_prepare`, and also
move the existing `mig_ops` check in `aer_reset_done` ahead of the `set_reset_flag`
handling.
The `is_virtfn` check in `probe` will be retained to maintain consistent semantics with
other vendor drivers.

Thanks.
Longfang.


> Alex
> .
> 

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

[Alex Williamson]

On Thu, 17 Sep 2026 14:13:31 +0800
liulongfang <liulongfang@huawei.com> wrote:

> On 2026/9/12 1:37, Alex Williamson wrote:
> > On Mon, 31 Aug 2026 17:09:49 +0800
> > Longfang Liu <liulongfang@huawei.com> 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@huawei.com>
> >> ---
> >>  .../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,
> >  
> 
> Thank you for the review. Regarding the first two checks in `aer_reset_done` and `migrn_init_dev`,
> you are correct that they are redundant; we will remove them in the next revision.
> 
> However, the PF passthrough scenario still crashes. The calltrace we actually tested occurs on
> the following path:

I didn't argue this fact, I only noted the .reset_done fix is redundant
and suggested a better ordering.

> 
> hisi_acc_vfio_pci_open_device
>   → vfio_pci_core_enable
>     → pci_try_reset_function
>       → hisi_acc_vf_pci_reset_prepare   ← NULL pointer dereference
> 
> 
> `reset_prepare` is a PCI error handler callback invoked by `pci_try_reset_function` inside
> `vfio_pci_core_enable`, which happens before the `mig_ops` check in `open_device`.
> At this point, the PF's `pf_qm` is NULL, and `reset_prepare` dereferences `&qm->pdev->dev`,
> causing an oops:
> 
> 
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
> pc : hisi_acc_vf_pci_reset_prepare+0x34/0xc0
> Call trace:
>  hisi_acc_vf_pci_reset_prepare
>  pci_dev_save_and_disable
>  pci_try_reset_function
>  vfio_pci_core_enable
>  hisi_acc_vfio_pci_open_device
>  vfio_df_open
> 
> 
> This shows that the `vf_id`-based check in `probe` does not actually prevent the PF
> from reaching `reset_prepare`;
> the PF still triggers the calltrace through this path. The fix here will follow your
> suggestion: elevate the `mig_ops` check at the entry of `reset_prepare`, and also
> move the existing `mig_ops` check in `aer_reset_done` ahead of the `set_reset_flag`
> handling.
> The `is_virtfn` check in `probe` will be retained to maintain consistent semantics with
> other vendor drivers.

I don't see this provides any consistency with other drivers, only the
virtio driver has a is_virtfn test and it is the fundamental test that
driver uses to select the migration ops.  It's not comparable to this
use case.  Thanks,

Alex

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

[liulongfang]

On 2026/9/18 11:10, Alex Williamson wrote:
> On Thu, 17 Sep 2026 14:13:31 +0800
> liulongfang <liulongfang@huawei.com> wrote:
> 
>> On 2026/9/12 1:37, Alex Williamson wrote:
>>> On Mon, 31 Aug 2026 17:09:49 +0800
>>> Longfang Liu <liulongfang@huawei.com> 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@huawei.com>
>>>> ---
>>>>  .../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,
>>>  
>>
>> Thank you for the review. Regarding the first two checks in `aer_reset_done` and `migrn_init_dev`,
>> you are correct that they are redundant; we will remove them in the next revision.
>>
>> However, the PF passthrough scenario still crashes. The calltrace we actually tested occurs on
>> the following path:
> 
> I didn't argue this fact, I only noted the .reset_done fix is redundant
> and suggested a better ordering.
>

Yes, in the reset_done function, the existing mig_ops check can intercept abnormal PF devices.

>>
>> hisi_acc_vfio_pci_open_device
>>   → vfio_pci_core_enable
>>     → pci_try_reset_function
>>       → hisi_acc_vf_pci_reset_prepare   ← NULL pointer dereference
>>
>>
>> `reset_prepare` is a PCI error handler callback invoked by `pci_try_reset_function` inside
>> `vfio_pci_core_enable`, which happens before the `mig_ops` check in `open_device`.
>> At this point, the PF's `pf_qm` is NULL, and `reset_prepare` dereferences `&qm->pdev->dev`,
>> causing an oops:
>>
>>
>> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
>> pc : hisi_acc_vf_pci_reset_prepare+0x34/0xc0
>> Call trace:
>>  hisi_acc_vf_pci_reset_prepare
>>  pci_dev_save_and_disable
>>  pci_try_reset_function
>>  vfio_pci_core_enable
>>  hisi_acc_vfio_pci_open_device
>>  vfio_df_open
>>
>>
>> This shows that the `vf_id`-based check in `probe` does not actually prevent the PF
>> from reaching `reset_prepare`;
>> the PF still triggers the calltrace through this path. The fix here will follow your
>> suggestion: elevate the `mig_ops` check at the entry of `reset_prepare`, and also
>> move the existing `mig_ops` check in `aer_reset_done` ahead of the `set_reset_flag`
>> handling.
>> The `is_virtfn` check in `probe` will be retained to maintain consistent semantics with
>> other vendor drivers.
> 
> I don't see this provides any consistency with other drivers, only the
> virtio driver has a is_virtfn test and it is the fundamental test that
> driver uses to select the migration ops.  It's not comparable to this
> use case.  Thanks,
> 

After intercepting live migration support for 64K page systems in probe, using the
`is_virtfn` check allows us to eliminate some redundant code and simplify the handling.
I've posted the new revision; please take a look.

Thanks.
Longfang.

> Alex
> .
>