Re: [PATCH v4 2/3] virtio_pci_modern: move avq cleanup from reset to del_vqs

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Surprise removal of the PF device while a VF is executing an admin command leads to an unresolvable deadlock.
--

commit 77107fdcf13b9589fc326075af29c2e63cc3478d
Author: Michael S. Tsirkin <mst@redhat.com>

virtio_pci_modern: move avq cleanup from reset to del_vqs

This commit moves the admin virtqueue cleanup from the device reset path
to the virtqueue deletion path to prevent a race condition with the interrupt
handler.

Link: https://lore.kernel.org/virtualization/20260911125745.E0A2F1F00899@smtp.kernel.org/

> diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
> index 6d8ae2a6a8caf..b4249afd7f580 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c

[ ... ]

> @@ -364,6 +364,12 @@ static void vp_modern_avq_cleanup(struct virtio_device *vdev)
>  	}
>  }
>  
> +static void vp_modern_del_vqs(struct virtio_device *vdev)
> +{
> +	vp_modern_avq_cleanup(vdev);

[Severity: High]
This isn't a bug introduced by this patch, but could this architecture still
lead to an unresolvable deadlock during a surprise removal of the PF device?

If a VF driver executes an admin command, the thread blocks uninterruptibly
in virtqueue_exec_admin_cmd() waiting for the command to complete.

If a surprise removal of the PF occurs while the VF is blocked, the PF
driver calls virtio_pci_remove(), which then calls pci_disable_sriov() to
wait for all VFs to unbind.

However, the VF unbind is blocked because the VF thread is stuck in
wait_for_completion(). Because pci_disable_sriov() never returns, the PF
removal never reaches unregister_virtio_device() and thus never calls this
vp_modern_del_vqs() function to detach the unused buffers and unblock the
VF thread.

Is there a way to abort the pending admin commands earlier in the PF surprise
removal path, before it blocks on disabling SRIOV?

> +	vp_del_vqs(vdev);
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789204858.git.mst@redhat.com?part=2