On Mon, Aug 16, 2021 at 1:31 PM Michael S. Tsirkin <[email protected]> wrote:
>
> On Fri, Aug 13, 2021 at 08:23:51PM +0530, Srivatsa Vaddagiri wrote:
> > Reset of a virtio-mmio device is initiated by writing 0 to its Status
> > register. The reset operation itself may or may not be completed by the time
> > write instruction completes. For devices that are emulated in software,
> > writes
> > to Status register are trapped and resumed only after the reset operation is
> > complete. Thus a driver can be assured of reset completion as soon as its
> > write
> > completes. That may not be however true in other cases, such as when
> > virtio-mmio
> > devices implemented directly in hardware. Driver's write to Status register
> > in
> > such case could complete before the device reset operation is completed.
> >
> > Update the specification to indicate when a driver may need to poll for
> > reset
> > completion before going ahead with the remaining initialization steps.
> >
> > Suggested-by: Jason Wang <[email protected]>
> > Signed-off-by: Srivatsa Vaddagiri <[email protected]>
> >
> > ---
> > V2->V1:
> > - Use version 0x3 as an indication for drivers to poll for reset completion
> > (rather than base it on a new feature bit)
> >
> > Previous version can be found at:
> >
> > https://lists.oasis-open.org/archives/virtio-dev/202107/msg00168.html
> >
> > content.tex | 11 +++++++----
> > 1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/content.tex b/content.tex
> > index 7cec1c3..b6b30a0 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -1730,9 +1730,9 @@ \subsection{MMIO Device Register
> > Layout}\label{sec:Virtio Transport Options / Vi
> > }
> > \hline
> > \mmioreg{Version}{Device version number}{0x004}{R}{%
> > - 0x2.
> > + 0x3.
> > \begin{note}
> > - Legacy devices (see \ref{sec:Virtio Transport Options / Virtio Over
> > MMIO / Legacy interface}~\nameref{sec:Virtio Transport Options / Virtio
> > Over MMIO / Legacy interface}) used 0x1.
> > + Legacy devices (see \ref{sec:Virtio Transport Options / Virtio Over
> > MMIO / Legacy interface}~\nameref{sec:Virtio Transport Options / Virtio
> > Over MMIO / Legacy interface}) used 0x1. Devices that do not require
> > drivers to poll for reset completion can use 0x2. See
> > \ref{devicenormative:Virtio Transport Options / Virtio Over MMIO / MMIO
> > Device Register Layout} for more details.
> > \end{note}
> > }
> > \hline
> > @@ -1916,7 +1916,7 @@ \subsection{MMIO Device Register
> > Layout}\label{sec:Virtio Transport Options / Vi
> >
> > The device MUST return 0x74726976 in \field{MagicValue}.
> >
> > -The device MUST return value 0x2 in \field{Version}.
> > +The device MUST return either value 0x3 or 0x2 in \field{Version} based on
> > its reset behavior. Drivers trigger reset of a device by writing 0 to
> > \field{Status}. The reset operation itself may or may not be completed by
> > the time write operation is complete. Devices whose reset operation
> > completes synchronously with the write operation are allowed to return
> > value of 0x2 for \field{Version}. Other devices, whose reset operation can
> > be incomplete by the time write operation completes MUST return value 0x3
> > as an indication for drivers to poll for reset completion.
> >
> > The device MUST present each event by setting the corresponding bit in
> > \field{InterruptStatus} from the
> > moment it takes place, until the driver acknowledges the interrupt
> > @@ -1947,12 +1947,15 @@ \subsection{MMIO Device Register
> > Layout}\label{sec:Virtio Transport Options / Vi
> > The driver MUST ignore a device with \field{MagicValue} which is not
> > 0x74726976,
> > although it MAY report an error.
> >
> > -The driver MUST ignore a device with \field{Version} which is not 0x2,
> > +The driver MUST ignore a device with \field{Version} which is neither 0x2
> > nor 0x3,
> > although it MAY report an error.
> >
> > The driver MUST ignore a device with \field{DeviceID} 0x0,
> > but MUST NOT report any error.
> >
> > +After writing 0 to \field{Status}, which triggers a reset of device, the
> > driver MUST wait for a read of
> > +\field{Status} to return 0 before proceeding with the remaining steps of
> > initializing the device.
>
> I know you copied this from pci but looking at PCI now, I think
> this is not great there either.
>
> 1. This is IMHO too opaque in that we did not say driver should read.
>
> E.g. after writing 0 to Status and before reading any fields, the driver
> MUST read Status and wait for a read etc.
>
> also would not just a read be sufficient? Is there a case for device to
> return any value
> other than 0 to signal "reset in progress"?
I'm not sure what case it can help. What is doing here is no worse
than the way PCI deal with it.
>
> 2. what if device encounters an error and sets
> NEED_RESET meanwhile? waiting for a reset will then deadlock ...
> I know, it's problematic like this in PCI too.
Yes, it's kind of a recursion that is not something that can be
handled at the virtio level.
We probably need a transport reset (FLR) here. But it's another topic I think.
>
> Further what will device return after reset was written but
> before it completed? I think it is better to specify that
> rather than rely on any non-0 value meaning "wait for reset".
> For pci maybe make it a SHOULD ...
>
I agree.
Thanks
>
> > +
> > Before reading from \field{DeviceFeatures}, the driver MUST write a value
> > to \field{DeviceFeaturesSel}.
> >
> > Before writing to the \field{DriverFeatures} register, the driver MUST
> > write a value to the \field{DriverFeaturesSel} register.
> >
> >
> > ---
> >
> > Qualcomm Innovation Center, Inc. is submitting the attached "feedback" as a
> > non-member to the virtio-dev mailing list for consideration and inclusion.
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
