Re: [RFC PATCH 18/19] vfio/pci: Coordinate generic device access with host recovery

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Writing a reserved bit (INTX_DISABLE) to the PCI Command register on pre-PCI 2.3 devices during AER resume.
--

commit 61aa3eda44efb78a0975edb523d9adc67f239efa
Author: Shameer Kolothum <skolothumtho@nvidia.com>

vfio/pci: Coordinate generic device access with host recovery

This commit wires up error_detected() for generic vfio-pci devices that
have opted into host recovery. It safely coordinates with the host by
blocking device access, draining running requests, revoking mappings,
and stopping bus mastering.

> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index eed0430c32ee6..2d757d6a5fe1b 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c

[ ... ]

> @@ -2746,14 +2746,178 @@ pci_ers_result_t vfio_pci_core_aer_err_detected(struct pci_dev *pdev,
>  {
>  	struct vfio_pci_core_device *vdev = dev_get_drvdata(&pdev->dev);
>  	struct vfio_pci_eventfd *eventfd;

[ ... ]

> +	ret = pci_write_config_word(pdev, PCI_COMMAND,
> +				    vdev->pci_recovery_command &
> +				    ~PCI_COMMAND_MASTER);
> +	if (ret)
> +		goto out_failed;
> +
> +	vdev->pci_recovery_command_valid = true;

[Severity: Medium]
Will this correctly handle pre-PCI 2.3 devices during AER resume?

Here vdev->pci_recovery_command_valid is set to true for devices even when
the device lacks PCI 2.3 support. Later, during resume, if this flag is true,
it appears vfio_pci_intx_recovery_command() in drivers/vfio/pci/vfio_pci_intrs.c
is invoked and applies the INTX_DISABLE bit unconditionally if the logical
context is masked:

drivers/vfio/pci/vfio_pci_intrs.c:vfio_pci_intx_recovery_command() {
    ...
    if (ctx && ctx->masked)
        command |= PCI_COMMAND_INTX_DISABLE;
    ...
}

Since pre-PCI 2.3 devices do not support INTX_DISABLE, could this write a
reserved bit to the PCI Command register on legacy hardware and cause
undefined behavior?

> +	goto out_memory;
> +
> +out_failed:
> +	WRITE_ONCE(vdev->pci_recovery_flags,
> +		   (vdev->pci_recovery_flags | VFIO_PCI_RECOVERY_FAILED) &
> +		   ~VFIO_PCI_RECOVERY_IN_PROGRESS);

[ ... ]

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