Re: [patch 2/4] KVM: move coalesced_mmio locking to its own device

Marcelo Tosatti
2009-05-20T15:22:38Z

On Wed, May 20, 2009 at 12:13:03PM -0300, Marcelo Tosatti wrote:
> On Wed, May 20, 2009 at 05:29:23PM +0300, Avi Kivity wrote:
> > Marcelo Tosatti wrote:
> >
> >
> >
> >>> So we have a function that takes a lock and conditionally releases it?
> >>>
> >>
> >> Yes, but it is correct: it will only return with the lock held in case
> >> it returns 1, in which case its guaranteed ->write will be called (which
> >> will unlock it).
> >>
> >> It should check the range first and/or use some smarter synchronization,
> >> but one thing at a time.
> >>
> >
> > Yes it's correct but we'll get an endless stream of patches to 'fix' it
> > because it is so unorthodox.
>
> Does it have to guarantee any kind of ordering in case of parallel
> writes by distincting vcpus? This is what it does now (so if a vcpu
> arrives first, the second will wait until the first is finished
> processing).
>
> I suppose that is the responsability of the guest (if it does MMIO
> writes to a device in parallel it'll screwup in real HW too).
>
> Because in such case, you can drop the mutex and protect only the kvm
> data.

If you want it to provide ordering (as in process the MMIO writes, by
multiple vcpus, in the order they happen), you need a mutex or spinlock.

And in that case, I don't see any other way around this given the way
->in_range / ->read/->write work.

Even if you change the order in which the full mmio buffer check and
coalesced-allowed-in-this-range happen you still need a spinlock/mutex
in this unorthodox way.