OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

# virtio-comment message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]

Subject: Re: [virtio-comment] [PATCH RFC v2] clarify device reset

• From: Halil Pasic <pasic@linux.ibm.com>
• To: Cornelia Huck <cohuck@redhat.com>
• Date: Tue, 19 Jan 2021 19:52:52 +0100

On Tue, 19 Jan 2021 18:45:06 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> On Tue, 19 Jan 2021 03:40:08 +0100
> Halil Pasic <pasic@linux.ibm.com> wrote:
>
> > On Mon, 18 Jan 2021 16:41:32 +0000
> > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> >
> > > * 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 -> RFC v2:
> > > >   - moved reset spec to basic facilities
> > > >
> > > > ---
> > > >  conformance.tex |  1 +
> > > >  content.tex     | 13 +++++++++++++
> > > >  2 files changed, 14 insertions(+)
> > > >
> > > > diff --git a/conformance.tex b/conformance.tex
> > > > index eb3324053080..3be499ae3c5e 100644
> > > > --- a/conformance.tex
> > > > +++ b/conformance.tex
> > > > @@ -271,6 +271,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..782ddf3ed78d 100644
> > > > --- a/content.tex
> > > > +++ b/content.tex
> > > > @@ -193,6 +193,19 @@ \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 receiving a reset.
> > > > +
> >
> > s/after receiving a reset/after presenting a 0 status, that indicates
> > the reset is done/
>
> "A device MUST NOT send notifications after indicating completion of
> the reset by reinitializing the device status to 0."
>
> ?

Works with me. I tried to align my wording with the pci wording.

>
> > >
> > > This feels like a bit of a race in the description;  a Device may have
> > > just sent a notification at the point that it receives a reset.
> > > When a driver initiates a reset, how does the driver know that the
> > > device has received it?
> >
> > I agree, but with the proposed modification not any more.
> >
> > To answer your question: PCI has the following driver normative (which I
> > believe needs to be generalized so we have something similar for each
> > transport, and thus the same semantics):
> > "After writing 0 to device_status, the driver MUST wait for a read of
> > device_status to return 0 before reinitializing the device."
> > (4.1.4.3.2 Driver Requirements: Common configuration structure layout,
> > https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-1090004)
> >
> > In general, after asking for a reset, the driver should/must ensure that
> > the reset was performed by the device by reading a 0 status. If the
> > status is non-zero, the reset at the device may still be in progress.
> > IMHO we need another driver normative for that.
>
> "After the driver has initiated a reset of the device, it MUST NOT
> consider the reset to be completed if the device status is not 0."
>
> ?

", before it reads status 0."

My point is, that usually when I do an assignment to a memory location
with a single instruction, and the instruction completes successfully,
for me (on my CPU), that memory location is 0.

PCI is however not like this: the device can delay or reject the write,
apparently. Jason taught me that. So I think we should insist on the

>
> Maybe without the double negation.
>
> (We could consider the reset for ccw devices done once we get final
> status for the reset ccw. Would save the round trip for a read status
> ccw, but would also be different from the other transports.)

We could work around that by making a positive statement. Not telling,
when the driver MUST NOT consider the reset completed, but tell when the
driver SHOULD consider the reset completed.

The MUST NOT does not buy much to the driver. It knows, what is
certainly wrong, but it still does not know what is right. What the
driver needs is a criterion when the reset is certainly completed (so
it can free up resources for example).

Regards,
Halil


[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]