Re: [PATCH 1/1] transport-mmio: Add v3, which polls for reset completion
Matias Ezequiel Vara Larsen <[email protected]> Thu, 26 Mar 2026 19:44:01 +0100
| Newsgroups | dev.linux.lists.virtio-comment |
|---|---|
| Message-ID | <acV+cZ3jvru8BzIY@fedora> |
Hello Peter and sorry for the delay, I used Claude to review it and I added some comments below: On Wed, Feb 04, 2026 at 12:50:22PM +0100, Peter Hilber wrote: > Let devices using the MMIO transport avoid stalling the driver (virtual) > CPU during device reset, which requires introducing a new MMIO transport > version. > > Unlike the PCI transport, the MMIO transport does not require the driver > to poll for reset completion. This requires a device using the MMIO > transport to complete reset during the write of 0 to the Status > register. Device reset may take more than 100 ms if it involves > terminating ongoing device activity which accesses driver memory. When > the (virtual) CPU writing 0 to the Status register needs to be stalled > during this, this may violate real-time requirements (including those > for hypervisor trap-and-emulate). > > Address this by introducing a new MMIO transport version, v3, where the > driver must poll for reset completion, and, hence, the device reset does > not have to complete during the write to the Status register. > > For clarity, also add some related requirements for v2. These > requirements are implied by the rest of the specification and therefore > do not alter the v2 semantics. > > With MMIO transport v3, reset essentially works as with the PCI > transport, and the change is therefore not expected to cause problems. > > Existing devices with MMIO transport v2 are not required to implement > v3, which will not work with current drivers. Drivers have to support > the MMIO transport versions of the used devices. Portable MMIO transport > drivers should therefore support both v2 and v3, which is simple. > > Signed-off-by: Peter Hilber <[email protected]> > --- > transport-mmio.tex | 26 +++++++++++++++++++++++--- > 1 file changed, 23 insertions(+), 3 deletions(-) > > diff --git a/transport-mmio.tex b/transport-mmio.tex > index 94a93a1..6504a6b 100644 > --- a/transport-mmio.tex > +++ b/transport-mmio.tex > @@ -64,7 +64,8 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi > } > \hline > \mmioreg{Version}{Device version number}{0x004}{R}{% > - 0x2. > + 0x2 or 0x3. With version 0x3, the driver waits until it reads 0 from the > + \field{Status} register before considering a reset complete. Shall we add something in the Status register too?,e.g., the driver may or may not block when writing to this register. > \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. > \end{note} > @@ -262,13 +263,29 @@ \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 value 0x2 or 0x3 in \field{Version}. > > 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 > by writing a corresponding bit mask to the \field{InterruptACK} register. Bits which > do not represent events which took place MUST be zero. > > +The device MUST reset when 0 is written to \field{Status}. > + > +While a reset is in progress, the device MUST retain the previous value of > +\field{Status}. I think this may be inconsistent with the `Device Status` section: `The \field{device status} field starts out as 0, and is reinitialized to 0 by the device during reset.` I think we could change in this sentence the word `during` by `after`. This is a minor comment though because in v2 `during` and `after` is the same from driver pov due to the sync semantics. > + > +For \field{Version} 0x2, the device MUST finish a reset before the driver's > +write of 0 to \field{Status} has completed. > + > +For \field{Version} 0x3, the device MAY continue with a reset after the driver's > +write of 0 to \field{Status} has completed. > + > +For \field{Version} 0x3, when \field{Status} is 0, the device MUST ignore > +further writes of 0 to \field{Status}. > + Is this coherent with PCI? I guess the idea is to ignore if the reset is in progress but in that case status is not zero yet. > +The device MUST present 0 in \field{Status} once it has finished the reset. > + > Upon reset, the device MUST clear all bits in \field{InterruptStatus} and ready bits in the > \field{QueueReady} register for all queues in the device. > > @@ -305,7 +322,7 @@ \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 whose \field{Version} is neither 0x2 nor 0x3, > although it MAY report an error. > > The driver MUST ignore a device with \field{DeviceID} 0x0, > @@ -331,6 +348,9 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi > The driver MUST write a value with a bit mask describing events it handled into \field{InterruptACK} when > it finishes handling an interrupt and MUST NOT set any of the undefined bits in the value. > > +For \field{Version} 0x3, the driver MUST NOT consider a reset complete before > +reading back 0 in \field{Status}. > + > If VIRTIO_F_RING_RESET has been negotiated, after the driver writes 1 to > \field{QueueReset} to reset the queue, the driver MUST NOT consider queue > reset to be complete until it reads back 0 in \field{QueueReset}. The driver > -- > 2.43.0 > >