Re: [RFC 7/11] virtio_pci: new, capability-aware driver.

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

On Wed, Jan 11, 2012 at 07:30:34AM -0600, Anthony Liguori wrote:
> On 01/10/2012 06:25 PM, Rusty Russell wrote:
> >On Tue, 10 Jan 2012 19:03:36 +0200, "Michael S. Tsirkin"<mst@redhat.com>  wrote:
> >>On Wed, Dec 21, 2011 at 11:03:25AM +1030, Rusty Russell wrote:
> >>>Yes.  The idea that we can alter fields in the device-specific config
> >>>area is flawed.  There may be cases where it doesn't matter, but as an
> >>>idea it was holed to begin with.
> >>>
> >>>We can reduce probability by doing a double read to check, but there are
> >>>still cases where it will fail.
> >>
> >>Okay - want me to propose an interface for that?
> >
> >Had a brief chat with BenH (CC'd).
> >
> >I think we should deprecate writing to the config space.  Only balloon
> >does it AFAICT, and I can't quite figure out *why* it has an 'active'
> >field.  This solves half the problem, of sync guest writes.  For the
> >other half, I suggest a generation counter; odd means inconsistent.  The
> >guest can poll.
> >
> >BenH also convinced me we should finally make the config space LE if
> >we're going to change things.  Since PCI is the most common transport,
> >guest-endian confuses people.  And it sucks for really weird machines
> 
> I think the more important thing to do is require accesses to
> integers in the config space to always be aligned and to use the
> appropriate accessor. Non-integer fields should be restricted to
> byte access.
> 
> That limits config space entries to 32-bit but also means that there
> is no need for a generation counter.  It's also easier to deal with
> endian conversion that way.

This is similar to what we have now. But it's still buggy: e.g. if guest
updates MAC byte by byte, we have no way to know when it's done doing
so. 


> But it means the backend code ends up being much simpler to write
> (because it behaves more like a normal PCI device).
> 
> If we're already making the change, the endianness ought to be a feature bit.
> 
> >We should also change the ring (to a single ring, I think).
> 
> Ack.
> 
> >Descriptors
> >to 24 bytes long (8 byte cookie, 8 byte addr, 4 byte len, 4 byte flags).
> >We might be able to squeeze it into 20 bytes but that means packing.  We
> >should support inline, chained or indirect.  Let the other side ack by
> >setting flag, cookie and len (if written).
> >
> >Moreover, I think we should make all these changes at once (at least, in
> >the spec).  That makes it a big change, and it'll take longer to
> >develop, but makes it easy in the long run to differentiate legacy and
> >modern virtio.
> 
> Ack.  Long live virtio2! :-)
> 
> Regards,
> 
> Anthony Liguori
> 
> >
> >Thoughts?
> >Rusty.