RE: [PATCH v4 3/3] virtio: introduce SUSPEND and RESUME feature

> From: Zhu Lingshan <[email protected]>
> Sent: 02 July 2025 02:24 PM
> 
> This commit allows the driver to suspend the device through a new device
> status bit SUSPEND and resume the device running by re-setting DRIVER_OK bit
> in device status.
> 
> Signed-off-by: Zhu Lingshan <[email protected]>
> Signed-off-by: Jason Wang <[email protected]>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/229
> ---
>  content.tex | 59
> +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 57 insertions(+), 2 deletions(-)
> 
> diff --git a/content.tex b/content.tex
> index 9213dc0..32cabfa 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -42,6 +42,9 @@ \section{\field{Device Status} Field}\label{sec:Basic
> Facilities of a Virtio Dev  \item[FEATURES_OK (8)] Indicates that the driver has
> acknowledged all the
>    features it understands, and feature negotiation is complete.
> 
> +\item[SUSPEND (16)] When VIRTIO_F_SUSPEND is negotiated, indicates that
> +the
> +  device has been suspended by the driver.
> +
>  \item[DEVICE_NEEDS_RESET (64)] Indicates that the device has experienced
>    an error from which it can't recover.
> 
> @@ -99,10 +102,10 @@ \section{Feature Bits}\label{sec:Basic Facilities of a
> Virtio Device / Feature B  \begin{description}
>  \item[0 to 23, 41, 42 and 50 to 127] Feature bits for the specific device type
> 
> -\item[24 to 40] Feature bits reserved for extensions to the queue and
> +\item[24 to 40, and 43] Feature bits reserved for extensions to the
> +queue and
>    feature negotiation mechanisms, see \ref{sec:Reserved Feature Bits}
> 
> -\item[43 to 49, and 128 and above] Feature bits reserved for future
> extensions.
> +\item[44 to 49, and 128 and above] Feature bits reserved for future
> extensions.
>  \end{description}
> 
>  \begin{note}
> @@ -629,6 +632,54 @@ \section{Device Cleanup}\label{sec:General
> Initialization And Device Operation /
> 
>  Thus a driver MUST ensure a virtqueue isn't live (by device reset) before
> removing exposed buffers.
> 
> +\section{Device Suspend}\label{sec:General Initialization And Device
> +Operation / Device Suspend}
> +
> +If VIRTIO_F_SUSPEND is negotiated, the driver is eligible to suspend the device
> by setting the SUSPEND bit in \field{device status} to 1, and the device sets the
> DRIVER_OK bit to 0 once it has been suspended.
> +
> +If the device has been suspended, the driver can resume the device running by
> setting the DRIVER_OK bit in \field{device status} to 1, and the device sets the
> SUSPEND bit to 0 once it resumes running.
> +
> +\drivernormative{\subsection}{Device Suspend}{General Initialization
> +And Device Operation / Device Suspend}
> +
> +The driver SHOULD NOT set SUSPEND bit if DRIVER_OK is not set or
> VIRTIO_F_SUSPEND is not negotiated.
> +
> +Once the driver sets SUSPEND bit in \field{device status} to 1:
> +\begin{itemize}
> +\item The driver MUST verify whether the device has been suspended by re-
> reading \field{device status}, examining whether the SUSPEND bit is set to 1
> and the DRIVER_OK bit is set to 0.
> +\item The driver MUST NOT make any more buffers available to the device.
> +\item The driver MUST NOT send notifications for any virtqueues.
> +\item The driver MUST NOT make any changes to Device Configuration Space
> except for \field{device status} if it is part of the Configuration Space.
> +\end{itemize}
> +
I missed to suggest this before or suggested, don't recall exactly.

You should add the similar normative here to indicate what are the requirements on:

Once the driver sets DRIVER_OK to 1 when suspended, the driver must wait for SUSPEND bit to turn 0 and DRIVER_OK to 1.

(you have described this in the description, just the normative is missing).
Though it is not a blocker to me.

> +\devicenormative{\subsection}{Device Suspend}{General Initialization
> +And Device Operation / Device Suspend}
> +
> +The device MUST ignore any operations on the SUSPEND bit from the
> +driver if the device has not been completely initialized by the
> +procedures in \ref{sec:General Initialization And Device Operation /
> +Device Initialization}
> +
> +The device SHOULD ignore any write access to its Configuration Space
> +while suspended, except for \field{device status} if it is part of the
> Configuration Space.
> +
> +A device MUST NOT send any notifications for any virtqueues, access any
> +virtqueues, or modify any fields in its Configuration Space while
> +suspended.
> +
> +If changes occur in the Configuration Space during suspended period,
> +the device MUST NOT send any configuration change notifications.
> +Instead, the device MUST send the notification when it resumes running.
> +
> +If the driver sets the SUSPEND bit in \field{device status} to 1, the
> +device MUST either suspend itself or set the DEVICE_NEEDS_RESET bit in
> \field{device status} to 1 when it fails to suspend.
> +
> +If the device has been suspended and the driver resumes the device
> +running by setting the DRIVER_OK bit in \field{device status} to 1, the device
> MUST either resume normal operation or set the DEVICE_NEEDS_RESET bit in
> \field{device status} to 1 when it fails to resume.
> +
> +When the driver sets the SUSPEND bit to 1, the device SHOULD perform
> +the following actions before presenting the SUSPEND bit as 1 and DRIVER_OK
> bit as 0 in the \field{device status}:
> +
> +\begin{itemize}
> +\item Stop consuming more buffers of any virtqueues.
> +\item Wait until all buffers that are being processed have been used.
> +\item Send used buffer notifications to the driver.
> +\end{itemize}
> +
>  \chapter{Virtio Transport Options}\label{sec:Virtio Transport Options}
> 
>  Virtio can use various different buses, thus the standard is split @@ -872,6
> +923,10 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>  	\ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits}
> for
>  	handling features reserved for future use.
> 
> +  \item[VIRTIO_F_SUSPEND(43)] This feature indicates that the driver can
> +   suspend the device by set the SUSPEND bit to 1.
> +   See \ref{sec:Basic Facilities of a Virtio Device / Device Status Field}.
> +
>  \end{description}
> 
>  \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
> --
> 2.49.0
> 
If you happen to send v5, please add above suggested requirement, if not it can be added later as well right after this as additional patch for same github issue 229.

Reviewed-by: Parav Pandit <[email protected]>