On Wed, 20 Jan 2021 11:13:37 +0800
Jason Wang <
jasowang@redhat.com> wrote:
> On 2021/1/20 ??2:52, Halil Pasic wrote:
> > 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
> > read.
>
>
> Yes. For PCI the status is implemented via registers, there's no
> guarantee a read is 0 after write 0 to that.
I thought it was clear that the driver cannot know the device status
without doing a read, but we certainly can make that explicit.
>
>
> >
> >> 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.)
>
>
> I think it's probably not a problem since we don't care about the
> performance of reset.
I was less concerned about the performance, more about the complexity
of the implementation. Sending an extra command is not that bad, though.
>
> Thanks
>
>
> > 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.
That could be
"The driver SHOULD consider a driver-initiated reset complete when it
reads the device status as 0."
> >
> > 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).
Any further comments before I put together a v3 (without the RFC)?
I also need to open a github issue for this.