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

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

On Mon, Dec 19, 2011 at 09:38:42PM +1030, Rusty Russell wrote:
> On Mon, 19 Dec 2011 11:13:26 +0200, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Mon, Dec 19, 2011 at 04:36:38PM +1030, Rusty Russell wrote:
> > > On Sun, 18 Dec 2011 12:18:32 +0200, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > On Fri, Dec 16, 2011 at 12:20:08PM +1030, Rusty Russell wrote:
> > > > > Perhaps a new feature VIRTIO_F_UNSTABLE?  Which (unlike other features)
> > > > > appears and vanishes around config writes by either side?  Kind of a
> > > > > hack though...
> > > > 
> > > > Not sure how this can work in such a setup: when would guest
> > > > check this bit to avoid races?
> > > > A separate registers also seems nicer than a flag.
> > > > 
> > > > Some other possible design choices:
> > > > - a flag to signal config accesses in progress by guest
> > > >   host would need to buffer changes and apply them in one go
> > > >   when flag is cleared
> > > > - a register to make host get/set config in guest memory
> > > > - use a control vq for all devices
> > > 
> > > - seqlock-style generation count register(s)?
> > >   Has the advantage of
> > >   being a noop if things never change.
> 
> Actually, the host doesn't need anything, since it can always lock out
> the guest while it updates the area.
> It's the guest which can't do atomic updates.

There are 2 cases
- updates by guest accesses by host
- accesses by guest updates by host

Both are problematic because the guest accesses are split.
Consider the example I gave at the beginning was with capacity read
by guest. Host can not solve it without guest changes, right?

> > The counter can be in guest memory, right? So we don't pay extra
> > exits.
> 
> Could be, but I'm not delighted about the design.

OK, so this is an argument for always using a control vq, right?

> What does the host do
> if the guest screws things up?  How long do you wait for them to
> complete the seqlock?  Or does it save the old version for use in the
> duration?

Yes, it will have to only apply the change when seqlock is dropped.

> And we don't have any atomic guest write problems that I know of.  We
> can solve it in future (by specifying a config queue).

Don't have == not reported as observed in the field?
It seems clear from code that we do have a race, correct?

> > > - continue to ignore it ;)
> > 
> > Since you decided on a config layout redesign it seems a good time to
> > fix architectural problems ...
> 
> Yes, indeed.
> 
> Cheers,
> Rusty.