Re: [PATCH v5] virtio: introduce SUSPEND bit in device status

David Stevens
2024-06-13T05:58:17Z

On Wed, Jun 12, 2024 at 9:20 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jun 12, 2024 at 11:26:39AM +0000, Parav Pandit wrote:
> > No locks are needed. Hypervisor is accessing the admin vq on the PF.
>
> I'm not even going to try and get through a jumble of this thread
> heavily corrupted by outlook and whatnot, but from the glimpses I got
> this discussion looks like just two people talking past each other.
> Lingshan, Parav's not the only one bothered by how the device seemingly
> has to block a transaction until it completes suspend. Parav, I feel how
> to address this is up to the author to resolve really, leave it at that.

IIUC, Prava's concern here is that with the current proposal, the
device's state can't be determined unambiguously by reading the device
status field. If the SUSPEND bit is 0, it's ambiguous as to whether
the device is running normally or if it's in the process of
suspending, and there is a corresponding issue for suspended vs
resuming. The device and driver can know based on their internal
information what state the device is in. However, some hypothetical
admin tool in guest userspace wouldn't be able to tell simply by
reading the values in the PCI BAR.

Assuming that's actually a problem that we want to solve, then I think
the most straightforward way to address this would be to use another
bit in the status field as a SUSPEND_TRANSITION bit, with something
like:

"After the driver writes or clears the SUSPEND bit, the device MUST
present the SUSPEND_TRANSITION bit until it completes the requested
suspend or resume transition. If the device is unable to successfully
complete the transition, it MUST present DEVICE_NEEDS_RESET."

Doing that would give distinct values for each of the
running/suspending/suspended/resuming states. That said, although the
device status field doesn't have a defined width, both the PCI and
channel transports assume that it's a single byte, so we'd be using
the last remaining bits those transports have available.

-David