* [virtio-comment] [PATCH v3] clarify device reset @ 2021-01-25 11:08 Cornelia Huck 2021-01-26 14:00 ` Stefan Hajnoczi ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Cornelia Huck @ 2021-01-25 11:08 UTC (permalink / raw) To: virtio-comment Cc: Jason Wang, Dr. David Alan Gilbert, Halil Pasic, Cornelia Huck 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. + +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. + +A device MUST NOT send notifications after indicating completion of +the reset by reinitializing the device status to 0. + +\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 -- 2.26.2 This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC. In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting. Subscribe: virtio-comment-subscribe@lists.oasis-open.org Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org List help: virtio-comment-help@lists.oasis-open.org List archive: https://lists.oasis-open.org/archives/virtio-comment/ Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists Committee: https://www.oasis-open.org/committees/virtio/ Join OASIS: https://www.oasis-open.org/join/ ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [virtio-comment] [PATCH v3] clarify device reset 2021-01-25 11:08 [virtio-comment] [PATCH v3] clarify device reset Cornelia Huck @ 2021-01-26 14:00 ` Stefan Hajnoczi 2021-01-27 3:07 ` Jason Wang 2021-01-27 9:19 ` [virtio-comment] " Halil Pasic 2 siblings, 0 replies; 11+ messages in thread From: Stefan Hajnoczi @ 2021-01-26 14:00 UTC (permalink / raw) To: Cornelia Huck Cc: virtio-comment, Jason Wang, Dr. David Alan Gilbert, Halil Pasic [-- Attachment #1: Type: text/plain, Size: 711 bytes --] On Mon, Jan 25, 2021 at 12:08:23PM +0100, Cornelia Huck 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(+) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [virtio-comment] [PATCH v3] clarify device reset 2021-01-25 11:08 [virtio-comment] [PATCH v3] clarify device reset Cornelia Huck 2021-01-26 14:00 ` Stefan Hajnoczi @ 2021-01-27 3:07 ` Jason Wang 2021-01-27 11:11 ` Cornelia Huck 2021-01-27 9:19 ` [virtio-comment] " Halil Pasic 2 siblings, 1 reply; 11+ messages in thread From: Jason Wang @ 2021-01-27 3:07 UTC (permalink / raw) To: Cornelia Huck, virtio-comment; +Cc: Dr. David Alan Gilbert, Halil Pasic On 2021/1/25 下午7:08, Cornelia Huck 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. > + > +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. > + We had similar description in "2.1.2 Device Requirements: Device Status Field": "The device MUST initialize device status to 0 upon reset." Consider we had a dedicated part for reset, maybe we can remove that one. Thanks > +A device MUST NOT send notifications after indicating completion of > +the reset by reinitializing the device status to 0. > + > +\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 This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC. In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting. Subscribe: virtio-comment-subscribe@lists.oasis-open.org Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org List help: virtio-comment-help@lists.oasis-open.org List archive: https://lists.oasis-open.org/archives/virtio-comment/ Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists Committee: https://www.oasis-open.org/committees/virtio/ Join OASIS: https://www.oasis-open.org/join/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [virtio-comment] [PATCH v3] clarify device reset 2021-01-27 3:07 ` Jason Wang @ 2021-01-27 11:11 ` Cornelia Huck 2021-01-28 2:37 ` Jason Wang 0 siblings, 1 reply; 11+ messages in thread From: Cornelia Huck @ 2021-01-27 11:11 UTC (permalink / raw) To: Jason Wang; +Cc: virtio-comment, Dr. David Alan Gilbert, Halil Pasic On Wed, 27 Jan 2021 11:07:53 +0800 Jason Wang <jasowang@redhat.com> wrote: > On 2021/1/25 下午7:08, Cornelia Huck 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. > > + > > +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. > > + > > > We had similar description in "2.1.2 Device Requirements: Device Status > Field": > > "The device MUST initialize device status to 0 upon reset." > > Consider we had a dedicated part for reset, maybe we can remove that one. Maybe we can replace the normative statement with an informal description in the device status section? I.e. something like "The device status field starts out as 0, and is reinitialized to 0 by the device during reset." > > Thanks > > > > +A device MUST NOT send notifications after indicating completion of > > +the reset by reinitializing the device status to 0. > > + > > +\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 This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC. In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting. Subscribe: virtio-comment-subscribe@lists.oasis-open.org Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org List help: virtio-comment-help@lists.oasis-open.org List archive: https://lists.oasis-open.org/archives/virtio-comment/ Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists Committee: https://www.oasis-open.org/committees/virtio/ Join OASIS: https://www.oasis-open.org/join/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [virtio-comment] [PATCH v3] clarify device reset 2021-01-27 11:11 ` Cornelia Huck @ 2021-01-28 2:37 ` Jason Wang 0 siblings, 0 replies; 11+ messages in thread From: Jason Wang @ 2021-01-28 2:37 UTC (permalink / raw) To: Cornelia Huck; +Cc: virtio-comment, Dr. David Alan Gilbert, Halil Pasic On 2021/1/27 下午7:11, Cornelia Huck wrote: > On Wed, 27 Jan 2021 11:07:53 +0800 > Jason Wang <jasowang@redhat.com> wrote: > >> On 2021/1/25 下午7:08, Cornelia Huck 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. >>> + >>> +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. >>> + >> >> We had similar description in "2.1.2 Device Requirements: Device Status >> Field": >> >> "The device MUST initialize device status to 0 upon reset." >> >> Consider we had a dedicated part for reset, maybe we can remove that one. > Maybe we can replace the normative statement with an informal > description in the device status section? I.e. something like > > "The device status field starts out as 0, and is reinitialized to 0 by > the device during reset." Fine with me. Thanks > >> Thanks >> >> >>> +A device MUST NOT send notifications after indicating completion of >>> +the reset by reinitializing the device status to 0. >>> + >>> +\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 > > This publicly archived list offers a means to provide input to the > OASIS Virtual I/O Device (VIRTIO) TC. > > In order to verify user consent to the Feedback License terms and > to minimize spam in the list archive, subscription is required > before posting. > > Subscribe: virtio-comment-subscribe@lists.oasis-open.org > Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org > List help: virtio-comment-help@lists.oasis-open.org > List archive: https://lists.oasis-open.org/archives/virtio-comment/ > Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf > List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists > Committee: https://www.oasis-open.org/committees/virtio/ > Join OASIS: https://www.oasis-open.org/join/ > This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC. In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting. Subscribe: virtio-comment-subscribe@lists.oasis-open.org Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org List help: virtio-comment-help@lists.oasis-open.org List archive: https://lists.oasis-open.org/archives/virtio-comment/ Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists Committee: https://www.oasis-open.org/committees/virtio/ Join OASIS: https://www.oasis-open.org/join/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [virtio-comment] Re: [PATCH v3] clarify device reset 2021-01-25 11:08 [virtio-comment] [PATCH v3] clarify device reset Cornelia Huck 2021-01-26 14:00 ` Stefan Hajnoczi 2021-01-27 3:07 ` Jason Wang @ 2021-01-27 9:19 ` Halil Pasic 2021-01-27 11:44 ` Cornelia Huck 2 siblings, 1 reply; 11+ messages in thread From: Halil Pasic @ 2021-01-27 9:19 UTC (permalink / raw) To: Cornelia Huck; +Cc: virtio-comment, Jason Wang, Dr. David Alan Gilbert 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 This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC. In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting. Subscribe: virtio-comment-subscribe@lists.oasis-open.org Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org List help: virtio-comment-help@lists.oasis-open.org List archive: https://lists.oasis-open.org/archives/virtio-comment/ Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists Committee: https://www.oasis-open.org/committees/virtio/ Join OASIS: https://www.oasis-open.org/join/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [virtio-comment] Re: [PATCH v3] clarify device reset 2021-01-27 9:19 ` [virtio-comment] " Halil Pasic @ 2021-01-27 11:44 ` Cornelia Huck 2021-01-27 13:48 ` Halil Pasic 0 siblings, 1 reply; 11+ messages in thread From: Cornelia Huck @ 2021-01-27 11:44 UTC (permalink / raw) To: Halil Pasic; +Cc: virtio-comment, Jason Wang, Dr. David Alan Gilbert On Wed, 27 Jan 2021 10:19:04 +0100 Halil Pasic <pasic@linux.ibm.com> wrote: > 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? s/may initiate/may want to initiate/ > > 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." It was mostly supposed to be a lead-in. I'm not sure we want to spell out things like system resets. They are platform-specific, and virtio devices are basically just a device there. Of course, the actual implementation will share backend code to reinitialize to a clean state for driver-initiated resets, system resets, and whatever else there might be, but I consider that an implementation detail and not something that needs to be specified. We do want to spell out, however, what a driver can expect and a device needs to do for a driver-initiated reset, and that's what this update aims to do. > > > + > > +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). Yes, I don't think it makes sense to reject a reset. > > 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". That's also true, but I think it is already covered? > > > > +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. Append "until the driver re-initializes the device."? > > 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. What about "A device MUST NOT send notifications or process queue buffers after indication completion of the reset by reinitializing the device status to 0, until the driver re-initializes the device." ? > > > + > > +\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 > This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC. In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting. Subscribe: virtio-comment-subscribe@lists.oasis-open.org Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org List help: virtio-comment-help@lists.oasis-open.org List archive: https://lists.oasis-open.org/archives/virtio-comment/ Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists Committee: https://www.oasis-open.org/committees/virtio/ Join OASIS: https://www.oasis-open.org/join/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [virtio-comment] Re: [PATCH v3] clarify device reset 2021-01-27 11:44 ` Cornelia Huck @ 2021-01-27 13:48 ` Halil Pasic 2021-01-27 17:14 ` Cornelia Huck 0 siblings, 1 reply; 11+ messages in thread From: Halil Pasic @ 2021-01-27 13:48 UTC (permalink / raw) To: Cornelia Huck; +Cc: virtio-comment, Jason Wang, Dr. David Alan Gilbert On Wed, 27 Jan 2021 12:44:03 +0100 Cornelia Huck <cohuck@redhat.com> wrote: > On Wed, 27 Jan 2021 10:19:04 +0100 > Halil Pasic <pasic@linux.ibm.com> wrote: > > > 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? > > s/may initiate/may want to initiate/ Definitively better. I would still prefer avoiding the discussion on when the driver may want to reset the device. But I can live with this. > > > > > If we are just looking for an introductionary sentence, I would > > probably go with something like "A device reset be either initiated > > by the s/be/can be/ > > driver, or by a system reset, or other external controls." > > It was mostly supposed to be a lead-in. > > I'm not sure we want to spell out things like system resets. They are > platform-specific, and virtio devices are basically just a device > there. Hm. AFAIR the only way to make the device non-live, i.e. illegal for it to e.g. access a queue, which is in guest memory is device reset. Now if I want to perform a reboot, I do want to stop the devices from poking such memory. But I think, in linux we don't reset the device form the driver, before reboot, and I believe we don't have to because we are guaranteed that the device will get reset. Or is my memory failing me? I think it is of value to clarify that a virtio reset can possibly be initiated by other means than explicitly specified by the virtio spec, and that when a 'proxy-device' is reset, the 'virtio-device' is to be reset as well. We do mention system reset once, in a normative section at that (2.2.2). > Of course, the actual implementation will share backend code to > reinitialize to a clean state for driver-initiated resets, system > resets, and whatever else there might be, but I consider that an > implementation detail and not something that needs to be specified. > > We do want to spell out, however, what a driver can expect and a device > needs to do for a driver-initiated reset, and that's what this update > aims to do. > > > > > > + > > > +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). > > Yes, I don't think it makes sense to reject a reset. > I don't know. And if that is what we want to say, I would certainly prefer a SHOULD over a must. E.g. what if the device looses power between receiving the reset and being able to finish it. Also if we want to express that a reset should not fail we should say that in a more straight forward way. How do we expect the device and the driver to deal with a device error that ain't recoverable even by a device reset? AFAIR all the device can do is set DEVICE_NEEDS_RESET to indicate "that the device has experienced an error from which it can’t recover". On the other hand, I think the most reasonable way for a driver to meet DEVICE_NEEDS_RESET is to reset it and make an attempt to re-initialize it... > > > > 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". > > That's also true, but I think it is already covered? > It is IMHO implicitly covered by the driver normative. > > > > > > > +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. > > Append "until the driver re-initializes the device."? > That is another solution. > > > > 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. > > What about > > "A device MUST NOT send notifications or process queue buffers after > indication completion of the reset by reinitializing the device status > to 0, until the driver re-initializes the device." > > ? Getting better, but IMHO not perfect. Process queue buffers is a bit vague IMHO. Even examining (reading) the ring(s) is taboo in my opinion, and I'm not sure that is unambiguously covered by 'process queue buffers'. In my opinion after a successful reset, the driver is entitled to even hot-unplug the ram that used to host the queues, notifier bits, etc. All this said, I believe the v3 is already a significant improvement, and the virtio spec ain't the most formal and painfully precise specification anyway. So if you don't feel like polishing this even further. Reviewed-by: Halil Pasic <pasic@linux.ibm.com> Regards, Halil This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC. In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting. Subscribe: virtio-comment-subscribe@lists.oasis-open.org Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org List help: virtio-comment-help@lists.oasis-open.org List archive: https://lists.oasis-open.org/archives/virtio-comment/ Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists Committee: https://www.oasis-open.org/committees/virtio/ Join OASIS: https://www.oasis-open.org/join/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [virtio-comment] Re: [PATCH v3] clarify device reset 2021-01-27 13:48 ` Halil Pasic @ 2021-01-27 17:14 ` Cornelia Huck 2021-01-28 8:06 ` Halil Pasic 0 siblings, 1 reply; 11+ messages in thread From: Cornelia Huck @ 2021-01-27 17:14 UTC (permalink / raw) To: Halil Pasic; +Cc: virtio-comment, Jason Wang, Dr. David Alan Gilbert On Wed, 27 Jan 2021 14:48:57 +0100 Halil Pasic <pasic@linux.ibm.com> wrote: > On Wed, 27 Jan 2021 12:44:03 +0100 > Cornelia Huck <cohuck@redhat.com> wrote: > > > On Wed, 27 Jan 2021 10:19:04 +0100 > > Halil Pasic <pasic@linux.ibm.com> wrote: > > > > > On Mon, 25 Jan 2021 12:08:23 +0100 > > > Cornelia Huck <cohuck@redhat.com> wrote: > > > > +\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? > > > > s/may initiate/may want to initiate/ > > Definitively better. I would still prefer avoiding the discussion > on when the driver may want to reset the device. But I can live > with this. The driver is free to reset the device whenever it wants to, even if it does not make sense. Device initialization and cleanup are examples for when it definitely will do so. > > > > > > > > > If we are just looking for an introductionary sentence, I would > > > probably go with something like "A device reset be either initiated > > > by the > > s/be/can be/ > > > > driver, or by a system reset, or other external controls." > > > > It was mostly supposed to be a lead-in. > > > > I'm not sure we want to spell out things like system resets. They are > > platform-specific, and virtio devices are basically just a device > > there. > > Hm. AFAIR the only way to make the device non-live, i.e. illegal for it > to e.g. access a queue, which is in guest memory is device reset. Now if > I want to perform a reboot, I do want to stop the devices from poking > such memory. But I think, in linux we don't reset the device form the > driver, before reboot, and I believe we don't have to because we are > guaranteed that the device will get reset. Or is my memory failing me? > > I think it is of value to clarify that a virtio reset can possibly be > initiated by other means than explicitly specified by the virtio spec, > and that when a 'proxy-device' is reset, the 'virtio-device' is to be > reset as well. We do mention system reset once, in a normative section > at that (2.2.2). That section looks a bit questionable (what is 'resuming from suspend'?). Platforms may have different definitions of what a system reset does (and even different types of reset). I think it is reasonable to assume that any platform/system reset that brings devices into some kind of initial state also brings virtio devices into that initial state (and especially that bringing the proxy device into an initial state also covers the virtio-specific parts.) > > > Of course, the actual implementation will share backend code to > > reinitialize to a clean state for driver-initiated resets, system > > resets, and whatever else there might be, but I consider that an > > implementation detail and not something that needs to be specified. > > > > We do want to spell out, however, what a driver can expect and a device > > needs to do for a driver-initiated reset, and that's what this update > > aims to do. > > > > > > > > > + > > > > +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). > > > > Yes, I don't think it makes sense to reject a reset. > > > > I don't know. And if that is what we want to say, I would certainly > prefer a SHOULD over a must. E.g. what if the device looses power > between receiving the reset and being able to finish it. Also if > we want to express that a reset should not fail we should say that > in a more straight forward way. If a device loses power, we cannot really mandate anything; it depends on the platform whether the device can continue with whatever has been in progress or whether it ends up in some kind of initial state. IOW, I think this is outside the scope of this standard. > > How do we expect the device and the driver to deal with a device > error that ain't recoverable even by a device reset? AFAIR all > the device can do is set DEVICE_NEEDS_RESET to indicate "that the device > has experienced an error from which it can’t recover". On the other > hand, I think the most reasonable way for a driver to meet > DEVICE_NEEDS_RESET > is to reset it and make an attempt to re-initialize it... If the device is not recoverable at all, I'd expect some platform-specific error when trying to access it in any way (write error, cc 3 on ssch, ...), as that implies to me that it is not operational anymore. > > > > > > > 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". > > > > That's also true, but I think it is already covered? > > > > It is IMHO implicitly covered by the driver normative. Yes. > > > > > > > > > > > +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. > > > > Append "until the driver re-initializes the device."? > > > > That is another solution. > > > > > > > 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. > > > > What about > > > > "A device MUST NOT send notifications or process queue buffers after > > indication completion of the reset by reinitializing the device status s/indication/indicating/ > > to 0, until the driver re-initializes the device." > > > > ? > > Getting better, but IMHO not perfect. Process queue buffers is a bit > vague IMHO. Even examining (reading) the ring(s) is taboo in my opinion, > and I'm not sure that is unambiguously covered by 'process queue > buffers'. In my opinion after a successful reset, the driver is > entitled to even hot-unplug the ram that used to host the queues, > notifier bits, etc. "interact with the queues", maybe? > > All this said, I believe the v3 is already a significant improvement, > and the virtio spec ain't the most formal and painfully precise > specification anyway. So if you don't feel like polishing this even > further. > > Reviewed-by: Halil Pasic <pasic@linux.ibm.com> Thanks! I guess I'll send a v4 containing the changes outlined above. @all: anything else I should consider? This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC. In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting. Subscribe: virtio-comment-subscribe@lists.oasis-open.org Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org List help: virtio-comment-help@lists.oasis-open.org List archive: https://lists.oasis-open.org/archives/virtio-comment/ Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists Committee: https://www.oasis-open.org/committees/virtio/ Join OASIS: https://www.oasis-open.org/join/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [virtio-comment] Re: [PATCH v3] clarify device reset 2021-01-27 17:14 ` Cornelia Huck @ 2021-01-28 8:06 ` Halil Pasic 2021-01-28 13:05 ` Cornelia Huck 0 siblings, 1 reply; 11+ messages in thread From: Halil Pasic @ 2021-01-28 8:06 UTC (permalink / raw) To: Cornelia Huck; +Cc: virtio-comment, Jason Wang, Dr. David Alan Gilbert On Wed, 27 Jan 2021 18:14:11 +0100 Cornelia Huck <cohuck@redhat.com> wrote: > On Wed, 27 Jan 2021 14:48:57 +0100 > Halil Pasic <pasic@linux.ibm.com> wrote: > > > On Wed, 27 Jan 2021 12:44:03 +0100 > > Cornelia Huck <cohuck@redhat.com> wrote: > > > > > On Wed, 27 Jan 2021 10:19:04 +0100 > > > Halil Pasic <pasic@linux.ibm.com> wrote: > > > > > > > On Mon, 25 Jan 2021 12:08:23 +0100 > > > > Cornelia Huck <cohuck@redhat.com> wrote: > > > > > > +\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? > > > > > > s/may initiate/may want to initiate/ > > > > Definitively better. I would still prefer avoiding the discussion > > on when the driver may want to reset the device. But I can live > > with this. > > The driver is free to reset the device whenever it wants to, even if it > does not make sense. Device initialization and cleanup are examples for > when it definitely will do so. > For device initialization and for cleanup is actually a MUST, and not a simple may want to. And that is properly specified in 3.1.1 and 3.3.1 respectively. I would actually advice against resetting the device when device status is still 0, and especially if we expect it to change to ACKNOWLEDGE, because we just asked the device to set that bit. Why? Because I assume we could read the old 0 and assume the reset was performed. > > > > > > > > > > > > > If we are just looking for an introductionary sentence, I would > > > > probably go with something like "A device reset be either initiated > > > > by the > > > > s/be/can be/ > > > > > > driver, or by a system reset, or other external controls." > > > > > > It was mostly supposed to be a lead-in. > > > > > > I'm not sure we want to spell out things like system resets. They are > > > platform-specific, and virtio devices are basically just a device > > > there. > > > > Hm. AFAIR the only way to make the device non-live, i.e. illegal for it > > to e.g. access a queue, which is in guest memory is device reset. Now if > > I want to perform a reboot, I do want to stop the devices from poking > > such memory. But I think, in linux we don't reset the device form the > > driver, before reboot, and I believe we don't have to because we are > > guaranteed that the device will get reset. Or is my memory failing me? > > > > I think it is of value to clarify that a virtio reset can possibly be > > initiated by other means than explicitly specified by the virtio spec, > > and that when a 'proxy-device' is reset, the 'virtio-device' is to be > > reset as well. We do mention system reset once, in a normative section > > at that (2.2.2). > > That section looks a bit questionable (what is 'resuming from suspend'?). > I have no problems with that sections, and I think it is very reasonable. You can think of 'resuming form suspend' like 'resuming from suspend-to-disk' or from 'virsh save'. I.e. a situation where the device has (probably) lost power and thus needs a (complete) re-initialization, but we are supposed to preserve some continuity (e.g. if ACCESS_PLATFORM was negotiable, it should still be negotiable). > Platforms may have different definitions of what a system reset does > (and even different types of reset). I think it is reasonable to assume > that any platform/system reset that brings devices into some kind of > initial state also brings virtio devices into that initial state (and > especially that bringing the proxy device into an initial state also > covers the virtio-specific parts.) > In an ideal world, after reading the spec, one should have to ponder if something is reasonable to assume or not. I agree, platforms (e.g. s390, ppc) may have different definitions and different flavors of resets, and the situation is similar for the transports (e.g. PCI, CCW). And the very same device may be used on different platforms. For PCIe there is something called a Function Level Reset. The relationship between the different resets ain't trivial to me. E.g. when we say 'reset the device' we actually mean 'reset the virtio aspect of the device', and not the 'entire device'. E.g. CCW_CMD_VDEV_RESET does not reset the ccw-device. [..] > > > > > > > > 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. > > > > > > What about > > > > > > "A device MUST NOT send notifications or process queue buffers after > > > indication completion of the reset by reinitializing the device status > > s/indication/indicating/ > > > > to 0, until the driver re-initializes the device." > > > > > > ? > > > > Getting better, but IMHO not perfect. Process queue buffers is a bit > > vague IMHO. Even examining (reading) the ring(s) is taboo in my opinion, > > and I'm not sure that is unambiguously covered by 'process queue > > buffers'. In my opinion after a successful reset, the driver is > > entitled to even hot-unplug the ram that used to host the queues, > > notifier bits, etc. > > "interact with the queues", maybe? > Yes even better. Another thing to consider is behavior during the reset. I think it could be beneficial to say that. The device SHOULD NOT interact with the queues while performing the reset. If I'm not wrong there was some problem with secure execution, but I don't remember what did we do about it. What I remember is that virtio-blk interacted with the queue during the reset, which happened, after the protection/encryption status of the memory backing the queues changed. > > > > All this said, I believe the v3 is already a significant improvement, > > and the virtio spec ain't the most formal and painfully precise > > specification anyway. So if you don't feel like polishing this even > > further. > > > > Reviewed-by: Halil Pasic <pasic@linux.ibm.com> > > Thanks! > > I guess I'll send a v4 containing the changes outlined above. > > @all: anything else I should consider? > This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC. In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting. Subscribe: virtio-comment-subscribe@lists.oasis-open.org Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org List help: virtio-comment-help@lists.oasis-open.org List archive: https://lists.oasis-open.org/archives/virtio-comment/ Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists Committee: https://www.oasis-open.org/committees/virtio/ Join OASIS: https://www.oasis-open.org/join/ ^ permalink raw reply [flat|nested] 11+ messages in thread
* [virtio-comment] Re: [PATCH v3] clarify device reset 2021-01-28 8:06 ` Halil Pasic @ 2021-01-28 13:05 ` Cornelia Huck 0 siblings, 0 replies; 11+ messages in thread From: Cornelia Huck @ 2021-01-28 13:05 UTC (permalink / raw) To: Halil Pasic; +Cc: virtio-comment, Jason Wang, Dr. David Alan Gilbert On Thu, 28 Jan 2021 09:06:32 +0100 Halil Pasic <pasic@linux.ibm.com> wrote: > On Wed, 27 Jan 2021 18:14:11 +0100 > Cornelia Huck <cohuck@redhat.com> wrote: > > > On Wed, 27 Jan 2021 14:48:57 +0100 > > Halil Pasic <pasic@linux.ibm.com> wrote: > > > > > On Wed, 27 Jan 2021 12:44:03 +0100 > > > Cornelia Huck <cohuck@redhat.com> wrote: > > > > > > > On Wed, 27 Jan 2021 10:19:04 +0100 > > > > Halil Pasic <pasic@linux.ibm.com> wrote: > > > > > > > > > On Mon, 25 Jan 2021 12:08:23 +0100 > > > > > Cornelia Huck <cohuck@redhat.com> wrote: > > > > > > > > +\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? > > > > > > > > s/may initiate/may want to initiate/ > > > > > > Definitively better. I would still prefer avoiding the discussion > > > on when the driver may want to reset the device. But I can live > > > with this. > > > > The driver is free to reset the device whenever it wants to, even if it > > does not make sense. Device initialization and cleanup are examples for > > when it definitely will do so. > > > > For device initialization and for cleanup is actually a MUST, > and not a simple may want to. And that is properly specified in 3.1.1 > and 3.3.1 respectively. > > I would actually advice against resetting the device when device status > is still 0, and especially if we expect it to change to ACKNOWLEDGE, > because we just asked the device to set that bit. Why? Because I assume > we could read the old 0 and assume the reset was performed. I don't think we need to point out every little questionable thing a driver might do. > > > > > > > > > > > > > > > > > > > If we are just looking for an introductionary sentence, I would > > > > > probably go with something like "A device reset be either initiated > > > > > by the > > > > > > s/be/can be/ > > > > > > > > driver, or by a system reset, or other external controls." > > > > > > > > It was mostly supposed to be a lead-in. > > > > > > > > I'm not sure we want to spell out things like system resets. They are > > > > platform-specific, and virtio devices are basically just a device > > > > there. > > > > > > Hm. AFAIR the only way to make the device non-live, i.e. illegal for it > > > to e.g. access a queue, which is in guest memory is device reset. Now if > > > I want to perform a reboot, I do want to stop the devices from poking > > > such memory. But I think, in linux we don't reset the device form the > > > driver, before reboot, and I believe we don't have to because we are > > > guaranteed that the device will get reset. Or is my memory failing me? > > > > > > I think it is of value to clarify that a virtio reset can possibly be > > > initiated by other means than explicitly specified by the virtio spec, > > > and that when a 'proxy-device' is reset, the 'virtio-device' is to be > > > reset as well. We do mention system reset once, in a normative section > > > at that (2.2.2). > > > > That section looks a bit questionable (what is 'resuming from suspend'?). > > > > I have no problems with that sections, and I think it is very > reasonable. You can think of 'resuming form suspend' like 'resuming from > suspend-to-disk' or from 'virsh save'. I.e. a situation where the device > has (probably) lost power and thus needs a (complete) re-initialization, > but we are supposed to preserve some continuity (e.g. if ACCESS_PLATFORM > was negotiable, it should still be negotiable). Maybe leave that for a different discussion? > > > Platforms may have different definitions of what a system reset does > > (and even different types of reset). I think it is reasonable to assume > > that any platform/system reset that brings devices into some kind of > > initial state also brings virtio devices into that initial state (and > > especially that bringing the proxy device into an initial state also > > covers the virtio-specific parts.) > > > > In an ideal world, after reading the spec, one should have to ponder if > something is reasonable to assume or not. > > I agree, platforms (e.g. s390, ppc) may have different definitions and > different flavors of resets, and the situation is similar for the > transports (e.g. PCI, CCW). And the very same device may be used on > different platforms. > > For PCIe there is something called a Function Level Reset. > > The relationship between the different resets ain't trivial to me. E.g. > when we say 'reset the device' we actually mean 'reset the virtio > aspect of the device', and not the 'entire device'. E.g. > CCW_CMD_VDEV_RESET does not reset the ccw-device. This spec is dealing with virtio and the proxy devices. Attempting to use a virtio mechanism to reset a generic pci or ccw entity is not something I'd extract from the spec. Resetting the virtio and proxy device parts by resetting the generic device is something that does not look any different from any other device type, so I really consider it out of scope. IOW, I'd rather not go down that rabbit hole. > > [..] > > > > > > > > > > > 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. > > > > > > > > What about > > > > > > > > "A device MUST NOT send notifications or process queue buffers after > > > > indication completion of the reset by reinitializing the device status > > > > s/indication/indicating/ > > > > > > to 0, until the driver re-initializes the device." > > > > > > > > ? > > > > > > Getting better, but IMHO not perfect. Process queue buffers is a bit > > > vague IMHO. Even examining (reading) the ring(s) is taboo in my opinion, > > > and I'm not sure that is unambiguously covered by 'process queue > > > buffers'. In my opinion after a successful reset, the driver is > > > entitled to even hot-unplug the ram that used to host the queues, > > > notifier bits, etc. > > > > "interact with the queues", maybe? > > > > Yes even better. Another thing to consider is behavior during the reset. > I think it could be beneficial to say that. > > The device SHOULD NOT interact with the queues while performing the > reset. > > If I'm not wrong there was some problem with secure execution, but I > don't remember what did we do about it. What I remember is that > virtio-blk interacted with the queue during the reset, which happened, > after the protection/encryption status of the memory backing the queues > changed. I don't think we should actually specify that: we never specified it before, so the driver cannot make a fair assumption anyway. This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC. In order to verify user consent to the Feedback License terms and to minimize spam in the list archive, subscription is required before posting. Subscribe: virtio-comment-subscribe@lists.oasis-open.org Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org List help: virtio-comment-help@lists.oasis-open.org List archive: https://lists.oasis-open.org/archives/virtio-comment/ Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists Committee: https://www.oasis-open.org/committees/virtio/ Join OASIS: https://www.oasis-open.org/join/ ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-01-28 13:06 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-25 11:08 [virtio-comment] [PATCH v3] clarify device reset Cornelia Huck 2021-01-26 14:00 ` Stefan Hajnoczi 2021-01-27 3:07 ` Jason Wang 2021-01-27 11:11 ` Cornelia Huck 2021-01-28 2:37 ` Jason Wang 2021-01-27 9:19 ` [virtio-comment] " Halil Pasic 2021-01-27 11:44 ` Cornelia Huck 2021-01-27 13:48 ` Halil Pasic 2021-01-27 17:14 ` Cornelia Huck 2021-01-28 8:06 ` Halil Pasic 2021-01-28 13:05 ` Cornelia Huck
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.