Re: [PATCH v3 3/3] virtio: introduce SUSPEND and RESUME feature

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Jun 26, 2025 at 09:25:42PM +0800, Zhu, Lingshan wrote:
> On 6/26/2025 7:59 PM, Parav Pandit wrote:
> 
>         From: Zhu Lingshan <lingshan.zhu@amd.com>
>         Sent: 23 June 2025 02:07 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 <lingshan.zhu@amd.com>
>         Signed-off-by: Jason Wang <jasowang@redhat.com>
>         Fixes:
>         https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
>         ub.com%2Foasis-tcs%2Fvirtio-
>         spec%2Fissues%2F229&data=05%7C02%7Cparav%40nvidia.com%7C3085ab
>         378f264b5c845808ddb2313478%7C43083d15727340c1b7db39efd9ccc17
>         a%7C0%7C0%7C638862646595099882%7CUnknown%7CTWFpbGZsb3d8e
>         yJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIj
>         oiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=Pf2ub%2FwVzdmf
>         A1JjScOJ7D%2B%2BNea9XgVFYdvvDQxym40%3D&reserved=0
>         ---
>          content.tex | 59
>         +++++++++++++++++++++++++++++++++++++++++++++++++++--
>          1 file changed, 57 insertions(+), 2 deletions(-)
> 
>         diff --git a/content.tex b/content.tex
>         index 2e8da46..c987334c 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, and 50 to 127] Feature bits for the specific device type
> 
>         -\item[24 to 42] Feature bits reserved for extensions to the queue and
>         +\item[24 to 43] Feature bits reserved for extensions to the queue and
>            feature negotiation mechanisms, see \ref{sec:Reserved Feature Bits}
> 
> 
>     Ignoring this delta as the fix of patch-2 will change this.
> 
> 
>         -\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
>         SHOULD set the DRIVER_OK bit to 0 once it has been suspended.
>         +
> 
>     You ignored the inputs.
>     I do not agree to add the "SHOULD" normative wording in the general description area.
>     The reasoning is explained already.
>     Please adapt to the existing style of this spec to keep the normative in requirements section.
> 
>     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.
> 
>     Is this really that hard to write above way?
> 
> I believe you totally ignored my replies in the last thread.
> There are no rules forbid using "SHOULD" in any non-normative sections.


While true, it is confusing to the reader, and tends to make writers
sloopy about adding confirmance statements.

Our recommendation (which we do not always remembered to follow, sadly)
is to do it like this:

In the non confirmance sections, avoid these keywords, describing the
functionality in a detailed way but without putting too much
emphasis on who does what and the specific level or requirement
and without using keywords.

Additionally, repeat the requirement in a short but formal way in the
confirmance section. For details of the operation, reader has the
non-confirmance sections.



Hope that helps.