Re: [virtio] [PATCH v2] initialization: add extra device status handshake

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Oct 01, 2013 at 05:42:52PM +0930, Rusty Russell wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > On Tue, Oct 01, 2013 at 11:19:57AM +0930, Rusty Russell wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> > On Thu, Sep 26, 2013 at 12:05:19PM +0930, Rusty Russell wrote:
> >> >>    may be a significant (or infinite) delay before setting this
> >> >>    bit.
> >> >>  
> >> >> +  FEATURES_OK (8) Indicates that the driver has acknowledged all the
> >> >> +  features it understands, and feature negotiation is complete.
> >> >> +
> >> >>    DRIVER_OK (4) Indicates that the driver is set up and ready to
> >> >>    drive the device.
> >> >>  
> >> >> @@ -444,23 +447,46 @@ how to communicate with the specific device.
> >> >>  
> >> >>  3. The DRIVER status bit is set: we know how to drive the device.
> >> >>  
> >> >> -4. Device-specific setup, including reading the device feature
> >> >> -  bits, discovery of virtqueues for the device, optional per-bus
> >> >> -  setup, and reading and possibly writing the device's virtio
> >> >> -  configuration space.
> >> >> +4. Device feature bits are read, and the the subset of feature bits
> >> >> +   understood by the OS and driver is written to the device.
> >> >> +
> >> >> +5. The FEATURES_OK status bit is set.
> >> >>  
> >> >> -5. The subset of device feature bits understood by the driver is
> >> >> -   written to the device.
> >> >> +6. The status byte is re-read to ensure the FEATURES_OK bit is still
> >> >> +   set: otherwise, the device does not support our subset of features
> >> >> +   and the device is unusable.
> >> >>  
> >> >> -6. The DRIVER_OK status bit is set.
> >> >> +7. Device-specific setup, including discovery of virtqueues for the
> >> >> +   device, optional per-bus setup, reading and possibly writing the
> >> >> +   device's virtio configuration space, and population of virtqueues.
> >> >>  
> >> >> -7. The device can now be used (ie. buffers added to the
> >> >> -   virtqueues)[4]
> >> >> +8. The DRIVER_OK status bit is set.  At this point the device is
> >> >> +   "live".
> >> >
> >> > What exactly does this imply?
> >> > Should device consume buffers before it's live, or should
> >> > it postpone this until DRIVER_OK?
> >> 
> >> The device MUST NOT consume buffers before DRIVER_OK.
> >>     - The driver is free to rip those descriptors back out if it wants
> >>     to clean up and fail, for example.
> >>     
> >> The driver MUST NOT notify the device before DRIVER_OK.
> >>     - It's not ready yet...
> >
> > This last one will need checks on data path in driver:
> > if (vq->dev->driver_ok)
> > 	vq_kick();
> > consider that once we register a netdev it
> > can start sending packets.
> 
> That's why the *driver* will need to set DRIVER_OK manually now.

OK, but this means driver can fail after DRIVER_OK. Is that OK?

> I'm thinking Linux would add a BUG_ON(!vq->dev->driver_ok) in
> virtqueue_kick().
> 
> Cheers,
> Rusty.

We can just set vq->broken when vq is created, clear on
driver_ok.

-- 
MST