[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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] `hisi_acc_vfio_pci_close_device()` accesses and frees migration file descriptors locklessly, racing with AER reset handling and leading to double `fput()` and Use-After-Free.
--

commit 63de12e55959c8c98f32c474a3df763dbb67aa2c
Author: Longfang Liu <liulongfang@huawei.com>
Subject: hisi_acc_vfio_pci: fix live migration enable conditions for PF passthrough

This commit fixes live migration enable conditions for PF passthrough by
adding validity checks for the PF QM pointer. It ensures operations exit
or return an error directly when the pointer is invalid to avoid crashes.

> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> index 86362ec424a50..e95d0ab0f11ab 100644
> --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
[ ... ]
> @@ -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;

[Severity: High]
This is a pre-existing issue, but I have a question about how the AER reset
interacts with device closing.

In hisi_acc_vf_pci_aer_reset_done(), the code eventually calls
hisi_acc_vf_reset() while holding state_mutex:

	mutex_lock(&hisi_acc_vdev->state_mutex);
	hisi_acc_vf_reset(hisi_acc_vdev);
	mutex_unlock(&hisi_acc_vdev->state_mutex);

Inside hisi_acc_vf_reset(), it calls hisi_acc_vf_disable_fds(), which reads
and modifies hisi_acc_vdev->resuming_migf and hisi_acc_vdev->saving_migf.

However, in hisi_acc_vfio_pci_close_device(), hisi_acc_vf_disable_fds() is
called without holding state_mutex:

static void hisi_acc_vfio_pci_close_device(struct vfio_device *core_vdev)
{
	...
	hisi_acc_vf_disable_fds(hisi_acc_vdev);
	mutex_lock(&hisi_acc_vdev->open_mutex);

Can this lead to a race condition where a user closes the device concurrently
with an AER reset?

If both threads call hisi_acc_vf_disable_fds() simultaneously, they might both
see a non-NULL migration file descriptor:

static void hisi_acc_vf_disable_fds(struct hisi_acc_vf_core_device *hisi_acc_vdev)
{
	if (hisi_acc_vdev->resuming_migf) {
		hisi_acc_debug_migf_copy(hisi_acc_vdev, hisi_acc_vdev->resuming_migf);
		hisi_acc_vf_disable_fd(hisi_acc_vdev->resuming_migf);
		fput(hisi_acc_vdev->resuming_migf->filp);
		hisi_acc_vdev->resuming_migf = NULL;
	}
        ...
}

Would this result in a double fput() and a use-after-free on the migration
file object since the read and modification are not protected by a common lock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831090951.844569-1-liulongfang@huawei.com?part=1

[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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete fix: `set_reset_flag` is not cleared on the error path when `qm` or `qm->io_base` is NULL.
--

commit 20e8953675b36be7d8bfc2a18cb44431bd40084a
Author: Longfang Liu <liulongfang@huawei.com>

hisi_acc_vfio_pci: clear set_reset_flag after reset completed

This patch attempts to make every reset cycle self-contained by clearing
set_reset_flag during aer_reset_done. It addresses an issue where a stale
latch could prematurely release a lock owned by another agent during a
reset timeout.

> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> index e95d0ab0f11ab..4abed2e49986f 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");
> +		}

[Severity: High]
Does this leave set_reset_flag persistently set if qm or qm->io_base
evaluates to false?

If agent A sets set_reset_flag to true and encounters an unavailable PF QM
during hisi_acc_vf_pci_aer_reset_done(), the flag will remain true.

Later, if agent B holds the QM_RESETTING lock and agent A attempts another
reset that times out, could agent A's aer_reset_done() see the stale true
flag and prematurely clear agent B's lock if the QM has since become
available?

Should set_reset_flag be cleared unconditionally in this function, regardless
of whether the PF QM is currently available?

>  	}
>  
>  	if (!hisi_acc_vdev->core_device.vdev.mig_ops)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831090951.844569-1-liulongfang@huawei.com?part=2

[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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch completely breaks VFIO passthrough for this device on 64KB page hosts instead of gracefully falling back to generic operations.
--

commit e0bfdb6998df9e209e0b7358693ec17f21c25bfe
Author: Longfang Liu <liulongfang@huawei.com>

hisi_acc_vfio_pci: reject live migration on 64KB page with QM_HW_V3 hardware

This commit aims to prevent security vulnerabilities and guest exceptions when
combining QM_HW_V3 hardware with 64KB pages by disabling live migration. It
returns -EINVAL during device initialization if the host page size exceeds
32KB.

> diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c
> index 4abed2e49986f..4f12cab96545e 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;

[Severity: High]
Does returning an error here completely break VFIO passthrough for this device
on 64KB page hosts?

Since hisi_acc_vf_qm_init() is called during device open, returning an error
causes the entire operation to fail. This results in users completely losing
passthrough functionality for QM_HW_V3 devices on 64KB page hosts, rather
than just having migration gracefully disabled.

Could this be handled earlier in hisi_acc_vfio_pci_probe() instead?

The driver currently assigns migration operations unconditionally:

drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:hisi_acc_vfio_pci_probe() {
    ...
    if (pf_qm && pf_qm->ver >= QM_HW_V3) {
        if (pdev->is_virtfn)
            ops = &hisi_acc_vfio_pci_migrn_ops;
    ...
}

If the 64KB page limitation was checked during probe and fell back to the
generic ops instead, normal passthrough would continue to work correctly.
The generic VFIO implementation safely maps the entire BAR2 region without
KVM trap-and-emulate issues.

> +		}
> +	}
> +
>  	if (hisi_acc_vdev->drv_mode == HW_ACC_MIG_PF_CTRL) {
>  		/*
>  		 * On hardware platforms greater than QM_HW_V3, the migration function

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831090951.844569-1-liulongfang@huawei.com?part=3