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

[Rusty Russell <rusty@au1.ibm.com>]

"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.

> 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.