Re: [virtio-dev] [PATCH v3] pci: new configuration layout

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

On Tue, Sep 24, 2013 at 03:36:42PM +0930, Rusty Russell wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > On Mon, Sep 23, 2013 at 10:57:24AM +0930, Rusty Russell wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> > On Thu, Sep 19, 2013 at 11:53:24AM +0930, Rusty Russell wrote:
> >> >> We should also define what "locks in" feature negotiation: that was
> >> >> simple for single 32-bit field.  Should we define this in a
> >> >> transport-independent way, or leave it to the transports?
> >> >> 
> >> >> We could overload the DRIVER status bit (which Linux currently calls too
> >> >> early, though moving it would be harmless), or add a new one.
> >> >
> >> > I'm surprised.
> >> > I always read 2.2.1. Device Initialization as
> >> > an explicit requirement that DRIVER_OK locks the features,
> >> > and that's in a transport-independent section and
> >> > works for existing guests.
> >> >
> >> > And if that's not explicit enough, would the proposed
> >> > resolution for VIRTIO-30 make it explicit enough?
> >> 
> >> Linux violates this, as it makes the device live *then* sets DRIVER_OK.
> >
> > Interesting.
> > Actually, e.g. net drivers do this, too: they pre-fill the RX VQs.
> >
> >> This means we can use the device (and *do*, for virtio_blk partition
> >> scanning
> >
> > What does this refer to?  virtblk_getgeo?
> > That only seems to poke at config space, not VQs.
> >
> >>) before finalizing features.
> 
> IIRC add_disk() does partition scanning.
> 
> >> We could become compliant by setting DRIVER_OK before calling the
> >> driver, but that's be misleading: we'd not expect DRIVER_OK if the
> >> driver init failed.
> >> So I think we will need a new bit, FEATURES_OK?
> >> 
> >> Cheers,
> >> Rusty.
> >
> > But what's the meaning of DRIVER_OK then?
> 
> It's informational, telling us how far the device got before it failed.
> 
> DRIVER_OK basically indicates that as far as guest is concerned, device
> is live.
> 
> > Maybe it means "device can use VQs"?
> >
> > And if device does not use VQs, does it matter that
> > driver can add buffers there?
> > If not maybe we can let driver play with features all
> > it wants.
> 
> That doesn't really work with the Linux model.  And if a future feature
> changed ring layout, we'd *really* need to know whether the guest
> understood that before it touched the vq.

OK so I added DEVICE_OK. Could you look at the latest draft?

> > Also all this is really part of discussion for VIRTIO-30 I think?
> > It's not really related to the layout - the issue is there
> > in old layout as well.
> > Correct?
> 
> Yes, except that it was easier to work around when a single write
> updated all the features.  Still I've opened a new issue...
> 
> Thanks,
> Rusty.

Hmm you don't think VIRTIO-30 is a good place to track it all?