Re: [PATCH v3] clarify device reset

[Halil Pasic <pasic@linux.ibm.com>]

On Mon, 25 Jan 2021 12:08:23 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> Properly specify that the method for the driver to request a
> device reset is transport specific, and some action the device
> has to take.
> 
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
> 
> RFC v2 -> v3:
>   - re-worded the "must not send notifications" clause to avoid guessing
>   - added a driver conformance clause on how a driver should find out
>     when reset is complete
> RFC -> RFC v2:
>   - moved reset spec to basic facilities
> 
> ---
>  conformance.tex |  2 ++
>  content.tex     | 19 +++++++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/conformance.tex b/conformance.tex
> index eb3324053080..21fe89ccd937 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -60,6 +60,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \begin{itemize}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Device Status Field}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Feature Bits}
> +\item \ref{drivernormative:Basic Facilities of a Virtio Device / Device Reset}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Device Configuration Space}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Virtqueues}
>  \item \ref{drivernormative:Basic Facilities of a Virtio Device / Message Framing}
> @@ -271,6 +272,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \begin{itemize}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Status Field}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits}
> +\item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Reset}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Device Configuration Space}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Message Framing}
>  \item \ref{devicenormative:Basic Facilities of a Virtio Device / Virtqueues / The Virtqueue Descriptor Table}
> diff --git a/content.tex b/content.tex
> index 620c0e28c9a7..9cdefe16509e 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -193,6 +193,25 @@ \section{Notifications}\label{sec:Basic Facilities of a Virtio Device
>  terminology. Occasionally, the term event is used to refer to
>  a notification or a receipt of a notification.
>  
> +\section{Device Reset}\label{sec:Basic Facilities of a Virtio Device / Device Reset}
> +
> +The driver may initiate a device reset at various times; notably, during
> +device initialization and device cleanup.

Are there times the driver may not initiate a device reset? What are we
trying to tell with this sentence?

If we are just looking for an introductionary sentence, I would probably
go with something like "A device reset be either initiated by the
driver, or by a system reset, or other external controls."

> +
> +The mechanism used by the driver to initiate the reset is transport specific.
> +
> +\devicenormative{\subsection}{Device Reset}{Basic Facilities of a Virtio Device / Device Reset}
> +
> +A device MUST reinitialize device status to 0 after receiving a reset.
> +

What is the intent behind this sentence?

One way I can read this it, that the device reset is not allowed to
fail. I.e. device must reinitialize status to after receiving a reset
and status 0 indicating the reset is done implies, that a reset must
succeed eventually (provided the device did receive).

The other way i can read it is, what I would rather have worded as:
"The device MUST indicate the completion of the reset by reinitializing
the device status to 0".


> +A device MUST NOT send notifications after indicating completion of
> +the reset by reinitializing the device status to 0.

I've just realized, that 'after' is very open to the right. Of course
the device may send notifications again after the driver re-initialized
the device as per 3.1.1.

Maybe it is wiser to tie this restriction to the current status value
(without any after or before), and do it where the notifications are
described. 

Also a device that has status 0 may not poke the virtqueues. If we
are explicit about the notifications, we should be explicit about
the virtqueues as well.

> +
> +\drivernormative{\subsection}{Device Reset}{Basic Facilities of a Virtio Device / Device Reset}
> +
> +The driver SHOULD consider a driver-initiated reset complete when it
> +reads the device status as 0.
> +
>  \section{Device Configuration Space}\label{sec:Basic Facilities of a Virtio Device / Device Configuration Space}
>  
>  Device configuration space is generally used for rarely-changing or