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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 23/09/2013 08:26, Michael S. Tsirkin ha scritto:
> 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?

It's done by add_disk (in block/genhd.c).

Paolo

> That only seems to poke at config space, not VQs.
> 
>> ) before finalizing features.
>>
>> 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?
> 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.
> 
> 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?
>