[PATCH] vfio/pci: Lock upstream bridge for vfio_pci_core_disable()

[PATCH] vfio/pci: Lock upstream bridge for vfio_pci_core_disable()

[Anthony Pighin (Nokia)]

Fix the following on VFIO detach:
[  242.271584] pcieport 0000:00:00.0: unlocked secondary bus reset via:
               pci_reset_bus_function+0x188/0x1b8

Commit 920f6468924f ("Warn on missing cfg_access_lock during secondary
bus reset") added a warning if the PCI configuration space was not
locked during a secondary bus reset request. That was in response to
commit 7e89efc6e9e4 ("Lock upstream bridge for pci_reset_function()")
such that remaining paths would be made more visible.

Address the vfio_pci_core_disable() path.

Signed-off-by: Anthony Pighin <anthony.pighin@nokia.com>
---
 drivers/vfio/pci/vfio_pci_core.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 3a11e6f450f7..aa2c21020ea8 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -588,6 +588,7 @@ EXPORT_SYMBOL_GPL(vfio_pci_core_enable);
 
 void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
 {
+	struct pci_dev *bridge;
 	struct pci_dev *pdev = vdev->pdev;
 	struct vfio_pci_dummy_resource *dummy_res, *tmp;
 	struct vfio_pci_ioeventfd *ioeventfd, *ioeventfd_tmp;
@@ -694,12 +695,20 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
 	 * We can not use the "try" reset interface here, which will
 	 * overwrite the previously restored configuration information.
 	 */
-	if (vdev->reset_works && pci_dev_trylock(pdev)) {
-		if (!__pci_reset_function_locked(pdev))
-			vdev->needs_reset = false;
-		pci_dev_unlock(pdev);
+	if (vdev->reset_works) {
+		bridge = pci_upstream_bridge(pdev);
+		if (bridge && !pci_dev_trylock(bridge))
+				goto out_restore_state;
+		if (pci_dev_trylock(pdev)) {
+			if (!__pci_reset_function_locked(pdev))
+				vdev->needs_reset = false;
+			pci_dev_unlock(pdev);
+		}
+		if (bridge)
+			pci_dev_unlock(bridge);
 	}
 
+out_restore_state:
 	pci_restore_state(pdev);
 out:
 	pci_disable_device(pdev);
-- 
2.43.0

Re: [PATCH] vfio/pci: Lock upstream bridge for vfio_pci_core_disable()

[Ilpo Järvinen]

On Tue, 13 Jan 2026, Anthony Pighin (Nokia) wrote:

> Fix the following on VFIO detach:

Fix the following warning that occurs during VFIO detach:

> [  242.271584] pcieport 0000:00:00.0: unlocked secondary bus reset via:
>                pci_reset_bus_function+0x188/0x1b8
> 
> Commit 920f6468924f ("Warn on missing cfg_access_lock during secondary
> bus reset") added a warning if the PCI configuration space was not
> locked during a secondary bus reset request. That was in response to
> commit 7e89efc6e9e4 ("Lock upstream bridge for pci_reset_function()")
> such that remaining paths would be made more visible.
> 
> Address the vfio_pci_core_disable() path.

Similar comments as to the other patch.

Why these are not submitted in a series (they seem to fix very similar 
cases, just for different call chains)?

> Signed-off-by: Anthony Pighin <anthony.pighin@nokia.com>
> ---
>  drivers/vfio/pci/vfio_pci_core.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 3a11e6f450f7..aa2c21020ea8 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -588,6 +588,7 @@ EXPORT_SYMBOL_GPL(vfio_pci_core_enable);
>  
>  void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
>  {
> +	struct pci_dev *bridge;
>  	struct pci_dev *pdev = vdev->pdev;
>  	struct vfio_pci_dummy_resource *dummy_res, *tmp;
>  	struct vfio_pci_ioeventfd *ioeventfd, *ioeventfd_tmp;
> @@ -694,12 +695,20 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
>  	 * We can not use the "try" reset interface here, which will
>  	 * overwrite the previously restored configuration information.
>  	 */
> -	if (vdev->reset_works && pci_dev_trylock(pdev)) {
> -		if (!__pci_reset_function_locked(pdev))
> -			vdev->needs_reset = false;
> -		pci_dev_unlock(pdev);
> +	if (vdev->reset_works) {
> +		bridge = pci_upstream_bridge(pdev);
> +		if (bridge && !pci_dev_trylock(bridge))
> +				goto out_restore_state;

Misaligned.

> +		if (pci_dev_trylock(pdev)) {
> +			if (!__pci_reset_function_locked(pdev))
> +				vdev->needs_reset = false;
> +			pci_dev_unlock(pdev);
> +		}
> +		if (bridge)
> +			pci_dev_unlock(bridge);
>  	}
>  
> +out_restore_state:
>  	pci_restore_state(pdev);
>  out:
>  	pci_disable_device(pdev);
> -- 
> 2.43.0
> 

-- 
 i.

Re: [PATCH] vfio/pci: Lock upstream bridge for vfio_pci_core_disable()

[Alex Williamson]

On Wed, 14 Jan 2026 10:36:08 +0200 (EET)
Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:

> On Tue, 13 Jan 2026, Anthony Pighin (Nokia) wrote:
> 
> > Fix the following on VFIO detach:  
> 
> Fix the following warning that occurs during VFIO detach:
> 
> > [  242.271584] pcieport 0000:00:00.0: unlocked secondary bus reset via:
> >                pci_reset_bus_function+0x188/0x1b8
> > 
> > Commit 920f6468924f ("Warn on missing cfg_access_lock during secondary
> > bus reset") added a warning if the PCI configuration space was not
> > locked during a secondary bus reset request. That was in response to
> > commit 7e89efc6e9e4 ("Lock upstream bridge for pci_reset_function()")
> > such that remaining paths would be made more visible.
> > 
> > Address the vfio_pci_core_disable() path.  
> 
> Similar comments as to the other patch.
> 
> Why these are not submitted in a series (they seem to fix very similar 
> cases, just for different call chains)?

They're related but independent fixes.  IMO it's easier for clear
ownership between maintainers and avoiding unnecessary conflicts or
shared branches to post them separately.

Let's also add the Fixes: tag on this one since it needs a respin for
the other comments.  Thanks,

Alex

> 
> > Signed-off-by: Anthony Pighin <anthony.pighin@nokia.com>
> > ---
> >  drivers/vfio/pci/vfio_pci_core.c | 17 +++++++++++++----
> >  1 file changed, 13 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> > index 3a11e6f450f7..aa2c21020ea8 100644
> > --- a/drivers/vfio/pci/vfio_pci_core.c
> > +++ b/drivers/vfio/pci/vfio_pci_core.c
> > @@ -588,6 +588,7 @@ EXPORT_SYMBOL_GPL(vfio_pci_core_enable);
> >  
> >  void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
> >  {
> > +	struct pci_dev *bridge;
> >  	struct pci_dev *pdev = vdev->pdev;
> >  	struct vfio_pci_dummy_resource *dummy_res, *tmp;
> >  	struct vfio_pci_ioeventfd *ioeventfd, *ioeventfd_tmp;
> > @@ -694,12 +695,20 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
> >  	 * We can not use the "try" reset interface here, which will
> >  	 * overwrite the previously restored configuration information.
> >  	 */
> > -	if (vdev->reset_works && pci_dev_trylock(pdev)) {
> > -		if (!__pci_reset_function_locked(pdev))
> > -			vdev->needs_reset = false;
> > -		pci_dev_unlock(pdev);
> > +	if (vdev->reset_works) {
> > +		bridge = pci_upstream_bridge(pdev);
> > +		if (bridge && !pci_dev_trylock(bridge))
> > +				goto out_restore_state;  
> 
> Misaligned.
> 
> > +		if (pci_dev_trylock(pdev)) {
> > +			if (!__pci_reset_function_locked(pdev))
> > +				vdev->needs_reset = false;
> > +			pci_dev_unlock(pdev);
> > +		}
> > +		if (bridge)
> > +			pci_dev_unlock(bridge);
> >  	}
> >  
> > +out_restore_state:
> >  	pci_restore_state(pdev);
> >  out:
> >  	pci_disable_device(pdev);
> > -- 
> > 2.43.0
> >   
>