[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] comment on pci section
On Fri, 31 Jan 2014 15:16:09 +1030 Rusty Russell <rusty@au1.ibm.com> wrote: > Rusty Russell <rusty@au1.ibm.com> writes: > > "Michael S. Tsirkin" <mst@redhat.com> writes: > >> On Wed, Jan 29, 2014 at 04:38:58PM +1030, Rusty Russell wrote: > >>> "Michael S. Tsirkin" <mst@redhat.com> writes: > >>> > Two comments received from a co-worker: > >>> > > >>> > 3.1 "The device MUST NOT consume buffers before DRIVER_OK ..." - I > >>> > personally > >>> > would like to see also something like: "device must not generate > >>> > interrupts if > >>> > DRIVER_OK is not set” to better handle power management transitions. > >>> > >>> That addition seems to make sense, but I don't understand the reference > >>> to power management transitions? > >> > >> Well - ATM we do things like suspend by device reset and re-initialization, > >> right? I'm guessing during e.g. resume it's important not to get > >> interrupts too early. I'll check. > >> > >> Hmm actually we have a chapter about initialization but not cleanup. > >> Might be a good addition. > > > > Maybe, but it would only need to say to reset the device and you can > > remove buffers safely. > > How's this? Actually does two things > > 1) Explicitly allow drivers to access config space during feature > negotiation. > > 2) Add the concept of a "live" virtqueue, and explicitly disallow > moving it backwards or changing descriptors. Make it clear that > taking the net device out of mq mode makes the extra vq's not-live. > > diff --git a/content.tex b/content.tex > index 48698fc..00d5fb3 100644 > --- a/content.tex > +++ b/content.tex > @@ -520,7 +520,10 @@ The driver MUST follow this sequence to initialize a device: > \item Set the DRIVER status bit: the guest OS knows how to drive the device. > > \item Read device feature bits, and write the subset of feature bits > - understood by the OS and driver to the device. > + understood by the OS and driver to the device. During this step the > + driver MAY access the device-specific configuration fields, even if > + those fields are conditional on feature bits, as long as those > + feature buts are offered by the device. s/buts/bits/ > > \item\label{itm:General Initialization And Device Operation / Device Initialization / Set FEATURES-OK} Set the FEATURES_OK status bit. The driver MUST not accept > new feature bits after this step. > @@ -540,9 +543,9 @@ The driver MUST follow this sequence to initialize a device: > If any of these steps go irrecoverably wrong, the driver SHOULD > set the FAILED status bit to indicate that it has given up on the > device (it can reset the device later to restart if desired). The > -driver MUST not continue initialization in that case. > +driver MUST NOT continue initialization in that case. > > -The device MUST NOT consume buffers before DRIVER_OK, and the driver > +The device MUST NOT consume buffers or send interrupts before DRIVER_OK, and the driver I'd rather call that 'host->guest notifications' or something like that. Else, it would be confusing to somebody trying to implement a virtio-ccw device (the channel commands always conclude with an I/O interrupt; the host->guest notifiction is 'interrupt + indicator bit'). > MUST NOT notify the device before it sets DRIVER_OK. > > Devices SHOULD support all valid combinations of features, but we know > @@ -765,7 +768,19 @@ suppressed by the device: > For devices where the configuration information can be changed, an > interrupt is delivered when a configuration change occurs. > > +\section{Device Cleanup}\label{sec:General Initialization And Device Operation / Device Cleanup} > > +Once the driver has set the DRIVER_OK status bit, the device is and > +all configured virtqueue are considered live. "the device and all of its configured virtqueues are..." ? > A device and all > +virtqueues are no longer live once the device has been reset. "A device and any of its virtqueues..." ? > + > +A driver MUST NOT alter descriptor table entries which have been > +exposed in the available ring (and not marked consumed by the device > +in the used ring) of a live virtqueue. > + > +A driver MUST NOT decrement the available index to "unexpose" buffers. > + > +Thus a driver will normally reset a device to remove exposed buffers. "If a driver wants to remove exposed buffers, it SHOULD reset the device." ? > > \chapter{Virtio Transport Options}\label{sec:Virtio Transport Options} > > @@ -2880,7 +2895,8 @@ device MUST NOT steer new packets to virtqueues > receveq1..receiveqN (i.e. other than receiveq0) and MUST NOT read from > transmitq1..transmitqN (i.e. other than transmitq0); accordingly, > the driver MUST NOT transmit new packets on virtqueues other than > -transmitq0. > +transmitq0. These virtqueues are no longer considered live, and thus > +the driver may be safely remove any buffers from them. > > \subparagraph{Legacy Interface: Automatic receive steering in multiqueue mode}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Legacy Interface: Automatic receive steering in multiqueue mode} > For legacy devices, the virtqueue_paris field is in the >
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]