bus: mhi: host: pci_generic: Fix the physical function check

[PATCH] bus: mhi: host: pci_generic: Fix the physical function check

Posted by Manivannan Sadhasivam 1 week, 5 days ago

Commit b4d01c5b9a9d ("bus: mhi: host: pci_generic: Read SUBSYSTEM_VENDOR_ID
for VF's to check status") added the check for physical function by
checking for 'pdev->is_physfn. But 'pdev->is_physfn' is only set for the
physical function of a SR-IOV capable device. But for the non-SR-IOV device
this variable will be 0. So this check ended up breaking the health check
functionality for all non-SR-IOV devices.

Fix it by checking for '!pdev->is_virtfn' to make sure that the check is
only skipped for virtual functions.

Reported-by: Slark Xiao <slark_xiao@163.com>
Fixes: b4d01c5b9a9d ("bus: mhi: host: pci_generic: Read SUBSYSTEM_VENDOR_ID for VF's to check status")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/bus/mhi/host/pci_generic.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
index 5836ecb0ea32..0d0d9c7ffa4b 100644
--- a/drivers/bus/mhi/host/pci_generic.c
+++ b/drivers/bus/mhi/host/pci_generic.c
@@ -1261,7 +1261,7 @@ static void mhi_pci_recovery_work(struct work_struct *work)
 
 	dev_warn(&pdev->dev, "device recovery started\n");
 
-	if (pdev->is_physfn)
+	if (!pdev->is_virtfn)
 		timer_delete(&mhi_pdev->health_check_timer);
 
 	pm_runtime_forbid(&pdev->dev);
@@ -1291,7 +1291,7 @@ static void mhi_pci_recovery_work(struct work_struct *work)
 
 	set_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status);
 
-	if (pdev->is_physfn)
+	if (!pdev->is_virtfn)
 		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
 
 	return;
@@ -1382,7 +1382,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		mhi_cntrl_config = info->config;
 
 	/* Initialize health check monitor only for Physical functions */
-	if (pdev->is_physfn)
+	if (!pdev->is_virtfn)
 		timer_setup(&mhi_pdev->health_check_timer, health_check, 0);
 
 	mhi_cntrl = &mhi_pdev->mhi_cntrl;
@@ -1404,7 +1404,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	mhi_cntrl->mru = info->mru_default;
 	mhi_cntrl->name = info->name;
 
-	if (pdev->is_physfn)
+	if (!pdev->is_virtfn)
 		mhi_pdev->reset_on_remove = info->reset_on_remove;
 
 	if (info->edl_trigger)
@@ -1453,7 +1453,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	set_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status);
 
 	/* start health check */
-	if (pdev->is_physfn)
+	if (!pdev->is_virtfn)
 		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
 
 	/* Allow runtime suspend only if both PME from D3Hot and M3 are supported */
@@ -1482,7 +1482,7 @@ static void mhi_pci_remove(struct pci_dev *pdev)
 	pm_runtime_forbid(&pdev->dev);
 	pci_disable_sriov(pdev);
 
-	if (pdev->is_physfn)
+	if (!pdev->is_virtfn)
 		timer_delete_sync(&mhi_pdev->health_check_timer);
 	cancel_work_sync(&mhi_pdev->recovery_work);
 
@@ -1514,7 +1514,7 @@ static void mhi_pci_reset_prepare(struct pci_dev *pdev)
 
 	dev_info(&pdev->dev, "reset\n");
 
-	if (pdev->is_physfn)
+	if (!pdev->is_virtfn)
 		timer_delete(&mhi_pdev->health_check_timer);
 
 	/* Clean up MHI state */
@@ -1560,7 +1560,7 @@ static void mhi_pci_reset_done(struct pci_dev *pdev)
 	}
 
 	set_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status);
-	if (pdev->is_physfn)
+	if (!pdev->is_virtfn)
 		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
 }
 
@@ -1626,7 +1626,7 @@ static int  __maybe_unused mhi_pci_runtime_suspend(struct device *dev)
 	if (test_and_set_bit(MHI_PCI_DEV_SUSPENDED, &mhi_pdev->status))
 		return 0;
 
-	if (pdev->is_physfn)
+	if (!pdev->is_virtfn)
 		timer_delete(&mhi_pdev->health_check_timer);
 
 	cancel_work_sync(&mhi_pdev->recovery_work);
@@ -1679,7 +1679,7 @@ static int __maybe_unused mhi_pci_runtime_resume(struct device *dev)
 	}
 
 	/* Resume health check */
-	if (pdev->is_physfn)
+	if (!pdev->is_virtfn)
 		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
 
 	/* It can be a remote wakeup (no mhi runtime_get), update access time */
-- 
2.51.0

Re:[PATCH] bus: mhi: host: pci_generic: Fix the physical function check

Posted by Slark Xiao 1 week, 5 days ago

At 2026-05-27 16:52:20, "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com> wrote:
>Commit b4d01c5b9a9d ("bus: mhi: host: pci_generic: Read SUBSYSTEM_VENDOR_ID
>for VF's to check status") added the check for physical function by
>checking for 'pdev->is_physfn. But 'pdev->is_physfn' is only set for the
>physical function of a SR-IOV capable device. But for the non-SR-IOV device
>this variable will be 0. So this check ended up breaking the health check
>functionality for all non-SR-IOV devices.
>
>Fix it by checking for '!pdev->is_virtfn' to make sure that the check is
>only skipped for virtual functions.
>
>Reported-by: Slark Xiao <slark_xiao@163.com>
>Fixes: b4d01c5b9a9d ("bus: mhi: host: pci_generic: Read SUBSYSTEM_VENDOR_ID for VF's to check status")
>Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>---
> drivers/bus/mhi/host/pci_generic.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
>diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
>index 5836ecb0ea32..0d0d9c7ffa4b 100644
>--- a/drivers/bus/mhi/host/pci_generic.c
>+++ b/drivers/bus/mhi/host/pci_generic.c
>@@ -1261,7 +1261,7 @@ static void mhi_pci_recovery_work(struct work_struct *work)
> 
> 	dev_warn(&pdev->dev, "device recovery started\n");
> 
>-	if (pdev->is_physfn)
>+	if (!pdev->is_virtfn)
> 		timer_delete(&mhi_pdev->health_check_timer);
> 
> 	pm_runtime_forbid(&pdev->dev);
>@@ -1291,7 +1291,7 @@ static void mhi_pci_recovery_work(struct work_struct *work)
> 
> 	set_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status);
> 
>-	if (pdev->is_physfn)
>+	if (!pdev->is_virtfn)
> 		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
> 
> 	return;
>@@ -1382,7 +1382,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> 		mhi_cntrl_config = info->config;
> 
> 	/* Initialize health check monitor only for Physical functions */
>-	if (pdev->is_physfn)
>+	if (!pdev->is_virtfn)
> 		timer_setup(&mhi_pdev->health_check_timer, health_check, 0);
> 
> 	mhi_cntrl = &mhi_pdev->mhi_cntrl;
>@@ -1404,7 +1404,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> 	mhi_cntrl->mru = info->mru_default;
> 	mhi_cntrl->name = info->name;
> 
>-	if (pdev->is_physfn)
>+	if (!pdev->is_virtfn)
> 		mhi_pdev->reset_on_remove = info->reset_on_remove;
> 
> 	if (info->edl_trigger)
>@@ -1453,7 +1453,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> 	set_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status);
> 
> 	/* start health check */
>-	if (pdev->is_physfn)
>+	if (!pdev->is_virtfn)
> 		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
> 
> 	/* Allow runtime suspend only if both PME from D3Hot and M3 are supported */
>@@ -1482,7 +1482,7 @@ static void mhi_pci_remove(struct pci_dev *pdev)
> 	pm_runtime_forbid(&pdev->dev);
> 	pci_disable_sriov(pdev);
> 
>-	if (pdev->is_physfn)
>+	if (!pdev->is_virtfn)
> 		timer_delete_sync(&mhi_pdev->health_check_timer);
> 	cancel_work_sync(&mhi_pdev->recovery_work);
> 
>@@ -1514,7 +1514,7 @@ static void mhi_pci_reset_prepare(struct pci_dev *pdev)
> 
> 	dev_info(&pdev->dev, "reset\n");
> 
>-	if (pdev->is_physfn)
>+	if (!pdev->is_virtfn)
> 		timer_delete(&mhi_pdev->health_check_timer);
> 
> 	/* Clean up MHI state */
>@@ -1560,7 +1560,7 @@ static void mhi_pci_reset_done(struct pci_dev *pdev)
> 	}
> 
> 	set_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status);
>-	if (pdev->is_physfn)
>+	if (!pdev->is_virtfn)
> 		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
> }
> 
>@@ -1626,7 +1626,7 @@ static int  __maybe_unused mhi_pci_runtime_suspend(struct device *dev)
> 	if (test_and_set_bit(MHI_PCI_DEV_SUSPENDED, &mhi_pdev->status))
> 		return 0;
> 
>-	if (pdev->is_physfn)
>+	if (!pdev->is_virtfn)
> 		timer_delete(&mhi_pdev->health_check_timer);
> 
> 	cancel_work_sync(&mhi_pdev->recovery_work);
>@@ -1679,7 +1679,7 @@ static int __maybe_unused mhi_pci_runtime_resume(struct device *dev)
> 	}
> 
> 	/* Resume health check */
>-	if (pdev->is_physfn)
>+	if (!pdev->is_virtfn)
> 		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
> 
> 	/* It can be a remote wakeup (no mhi runtime_get), update access time */
>-- 
>2.51.0

Tested-by: Slark Xiao <slark_xiao@163.com>

Test result is expected now with these changes.
BTW, shall we add this changes into stable version? Since kernel 6.18 and 7.0  have been affected.

Thanks

Re: [PATCH] bus: mhi: host: pci_generic: Fix the physical function check

Posted by Manivannan Sadhasivam 1 week, 5 days ago

On Wed, May 27, 2026 at 06:08:23PM +0800, Slark Xiao wrote:
> At 2026-05-27 16:52:20, "Manivannan Sadhasivam" <manivannan.sadhasivam@oss.qualcomm.com> wrote:
> >Commit b4d01c5b9a9d ("bus: mhi: host: pci_generic: Read SUBSYSTEM_VENDOR_ID
> >for VF's to check status") added the check for physical function by
> >checking for 'pdev->is_physfn. But 'pdev->is_physfn' is only set for the
> >physical function of a SR-IOV capable device. But for the non-SR-IOV device
> >this variable will be 0. So this check ended up breaking the health check
> >functionality for all non-SR-IOV devices.
> >
> >Fix it by checking for '!pdev->is_virtfn' to make sure that the check is
> >only skipped for virtual functions.
> >
> >Reported-by: Slark Xiao <slark_xiao@163.com>
> >Fixes: b4d01c5b9a9d ("bus: mhi: host: pci_generic: Read SUBSYSTEM_VENDOR_ID for VF's to check status")
> >Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> >---
> > drivers/bus/mhi/host/pci_generic.c | 20 ++++++++++----------
> > 1 file changed, 10 insertions(+), 10 deletions(-)
> >
> >diff --git a/drivers/bus/mhi/host/pci_generic.c b/drivers/bus/mhi/host/pci_generic.c
> >index 5836ecb0ea32..0d0d9c7ffa4b 100644
> >--- a/drivers/bus/mhi/host/pci_generic.c
> >+++ b/drivers/bus/mhi/host/pci_generic.c
> >@@ -1261,7 +1261,7 @@ static void mhi_pci_recovery_work(struct work_struct *work)
> > 
> > 	dev_warn(&pdev->dev, "device recovery started\n");
> > 
> >-	if (pdev->is_physfn)
> >+	if (!pdev->is_virtfn)
> > 		timer_delete(&mhi_pdev->health_check_timer);
> > 
> > 	pm_runtime_forbid(&pdev->dev);
> >@@ -1291,7 +1291,7 @@ static void mhi_pci_recovery_work(struct work_struct *work)
> > 
> > 	set_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status);
> > 
> >-	if (pdev->is_physfn)
> >+	if (!pdev->is_virtfn)
> > 		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
> > 
> > 	return;
> >@@ -1382,7 +1382,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > 		mhi_cntrl_config = info->config;
> > 
> > 	/* Initialize health check monitor only for Physical functions */
> >-	if (pdev->is_physfn)
> >+	if (!pdev->is_virtfn)
> > 		timer_setup(&mhi_pdev->health_check_timer, health_check, 0);
> > 
> > 	mhi_cntrl = &mhi_pdev->mhi_cntrl;
> >@@ -1404,7 +1404,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > 	mhi_cntrl->mru = info->mru_default;
> > 	mhi_cntrl->name = info->name;
> > 
> >-	if (pdev->is_physfn)
> >+	if (!pdev->is_virtfn)
> > 		mhi_pdev->reset_on_remove = info->reset_on_remove;
> > 
> > 	if (info->edl_trigger)
> >@@ -1453,7 +1453,7 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> > 	set_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status);
> > 
> > 	/* start health check */
> >-	if (pdev->is_physfn)
> >+	if (!pdev->is_virtfn)
> > 		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
> > 
> > 	/* Allow runtime suspend only if both PME from D3Hot and M3 are supported */
> >@@ -1482,7 +1482,7 @@ static void mhi_pci_remove(struct pci_dev *pdev)
> > 	pm_runtime_forbid(&pdev->dev);
> > 	pci_disable_sriov(pdev);
> > 
> >-	if (pdev->is_physfn)
> >+	if (!pdev->is_virtfn)
> > 		timer_delete_sync(&mhi_pdev->health_check_timer);
> > 	cancel_work_sync(&mhi_pdev->recovery_work);
> > 
> >@@ -1514,7 +1514,7 @@ static void mhi_pci_reset_prepare(struct pci_dev *pdev)
> > 
> > 	dev_info(&pdev->dev, "reset\n");
> > 
> >-	if (pdev->is_physfn)
> >+	if (!pdev->is_virtfn)
> > 		timer_delete(&mhi_pdev->health_check_timer);
> > 
> > 	/* Clean up MHI state */
> >@@ -1560,7 +1560,7 @@ static void mhi_pci_reset_done(struct pci_dev *pdev)
> > 	}
> > 
> > 	set_bit(MHI_PCI_DEV_STARTED, &mhi_pdev->status);
> >-	if (pdev->is_physfn)
> >+	if (!pdev->is_virtfn)
> > 		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
> > }
> > 
> >@@ -1626,7 +1626,7 @@ static int  __maybe_unused mhi_pci_runtime_suspend(struct device *dev)
> > 	if (test_and_set_bit(MHI_PCI_DEV_SUSPENDED, &mhi_pdev->status))
> > 		return 0;
> > 
> >-	if (pdev->is_physfn)
> >+	if (!pdev->is_virtfn)
> > 		timer_delete(&mhi_pdev->health_check_timer);
> > 
> > 	cancel_work_sync(&mhi_pdev->recovery_work);
> >@@ -1679,7 +1679,7 @@ static int __maybe_unused mhi_pci_runtime_resume(struct device *dev)
> > 	}
> > 
> > 	/* Resume health check */
> >-	if (pdev->is_physfn)
> >+	if (!pdev->is_virtfn)
> > 		mod_timer(&mhi_pdev->health_check_timer, jiffies + HEALTH_CHECK_PERIOD);
> > 
> > 	/* It can be a remote wakeup (no mhi runtime_get), update access time */
> >-- 
> >2.51.0
> 
> Tested-by: Slark Xiao <slark_xiao@163.com>
> 
> Test result is expected now with these changes.
> BTW, shall we add this changes into stable version? Since kernel 6.18 and 7.0  have been affected.
> 

I've CCed stable list while applying. Hopefully, stable team will backport this
fix once this gets merged upstream.

- Mani

-- 
மணிவண்ணன் சதாசிவம்