Re: [RFC PATCH v2 14/16] vfio/pci: Block device access during host recovery

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] cleanup: vfio_pci_core_enable() mixes goto and scoped_guard()
--

commit 054f5c9b3c995b5b332595d9db5677b803d39ed9
Author: Shameer Kolothum <skolothumtho@nvidia.com>

vfio/pci: Block device access during host recovery

This patch extends error_detected() to block device access during host
recovery by draining existing SRCU readers. It also masks INTx, revokes
BAR mappings, and manages the host transaction state across device open
and close.

> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index d6cc34240b250..2ec8022e4a692 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -617,6 +617,15 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
>  	u16 cmd;
>  	u8 msix_pos;
>  
> +	if (vdev->pci_recovery_supported) {
> +		if (pci_dev_is_disconnected(pdev))
> +			return -ENODEV;
> +		scoped_guard(mutex, &vdev->access_lock) {

[Severity: Low]
Does this mix scope-based cleanup with goto-based error handling in the same
function?

The cleanup subsystem guidelines in include/linux/cleanup.h state that goto
and cleanup helpers like scoped_guard() should never be mixed in the same
function to prevent ownership confusion. For a given routine, the expectation
is to convert all resources that need a goto cleanup to scope-based cleanup,
or convert none of them.

Looking at vfio_pci_core_enable(), the function still relies heavily on
legacy goto cleanup labels (like out_disable_device: and out_power:) on its
error paths. 

Should this function be fully converted to scope-based cleanup first, or
should this new lock acquisition continue using the traditional lock and
unlock pattern to match the rest of the function?

> +			if (vdev->pci_recovery_host_active)
> +				return -EBUSY;
> +		}
> +	}
> +
>  	if (!vdev->disable_idle_d3) {
>  		ret = pm_runtime_resume_and_get(&pdev->dev);
>  		if (ret < 0)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929173305.204856-1-skolothumtho@nvidia.com?part=14