[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH v1] virtio-mmio: Specify wait needed in driver during reset
On Mon, Jul 26, 2021 at 01:03:02PM +0200, Cornelia Huck wrote: > On Fri, Jul 23 2021, Srivatsa Vaddagiri <svaddagi@qti.qualcomm.com> wrote: > > > Changes since v0: > > > > Introduce a new feature bit, VIRTIO_F_MMIO_RESET_WAIT, which controls whether a driver polls > > for reset completion or not. > > Nit: changelog should go into a "---" section, next to the diffstat (git > format-patch should give you the correct base to edit.) > > > > > === > > > > Reset of a virtio-mmio device is accomplished by writing 0 to its Status register. > > 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 the case for virtio-mmio devices implemented in hardware > > directly. A write could complete before the reset operation inside device is > > completed. Introduce a new feature, VIRTIO_F_MMIO_RESET_WAIT, that such devices > > will offer as means to indicate to driver that they need to poll for reset > > completion, indicated by a read of Status register returning 0. > > > > Signed-off-by: Srivatsa Vaddagiri <svaddagi@qti.qualcomm.com> > > > > diff --git a/content.tex b/content.tex > > index 5c70a3c..1990a5c 100644 > > --- a/content.tex > > +++ b/content.tex > > @@ -1924,7 +1924,10 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi > > do not represent events which took place MUST be zero. > > > > 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. > > +\field{QueueReady} register for all queues in the device. The device MUST also offer > > +VIRTIO_F_MMIO_RESET_WAIT if it requires driver to poll for reset completion > > s/driver/the driver/ > > > +indicated by a read of \field{Status} returning 0. Such a device MUST also fail to accept > > +FEATURES_OK bit if driver does not negotiate VIRTIO_F_MMIO_RESET_WAIT. > > So, this basically means that an older driver that does not know the new > feature bit will not work with devices offering this bit, even if it did > poll? This seems acceptable, I just wanted to spell it out. Maybe should or even may is enough here. For example it is reasonable to have hypervisor block guest until reset is complete. > > > > The device MUST change value returned in \field{ConfigGeneration} if there is any risk of a > > driver seeing an inconsistent configuration state. > > @@ -1944,6 +1947,10 @@ \subsection{MMIO Device Register Layout}\label{sec:Virtio Transport Options / Vi > > 8 bit wide fields, 16 bit wide and aligned accesses for 16 bit wide fields and 32 bit wide and > > aligned accesses for 32 and 64 bit wide fields. > > > > +The driver MUST accept VIRTIO_F_MMIO_RESET_WAIT if offered by device. During > > s/device/the device/ > > > +reset of such a device that offers VIRTIO_F_MMIO_RESET_WAIT, driver MUST poll for > > s/driver/the driver/ > > But maybe use "if VIRTIO_F_MMIO_RESET_WAIT has been negotiated" instead? > This is implied by the statements above, but the deciding factor here is > that the feature has been negotiated, and not that it has been offered. > > > > +reset completion indicated by a read of \field{Status} returning 0. > > + > > The driver MUST ignore a device with \field{MagicValue} which is not 0x74726976, > > although it MAY report an error. > > > > @@ -6672,6 +6679,10 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits} > > transport specific. > > For more details about driver notifications over PCI see \ref{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications}. > > > > + \item[VIRTIO_F_MMIO_RESET_WAIT(40)] This feature could be offered by a device > > + using MMIO transport and indicates that the driver needs to poll for reset > > "This feature is specific to the MMIO transport and indicates that the > device requires the driver to poll..." > > ? > > > + completion indicated by a read of \field{Status} returning 0. > > + > > \end{description} > > > > \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits} > > @@ -6708,6 +6719,8 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits} > > > > A driver SHOULD accept VIRTIO_F_NOTIF_CONFIG_DATA if it is offered. > > > > +A driver SHOULD accept VIRTIO_F_MMIO_RESET_WAIT if it is offered. > > + > > But this was specified as MUST above, wasn't it? > > > \devicenormative{\section}{Reserved Feature Bits}{Reserved Feature Bits} > > > > A device MUST offer VIRTIO_F_VERSION_1. A device MAY fail to operate further
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]