Re: [PATCH] kvm: Flush coalesced MMIO buffer periodly

[Sheng Yang <sheng@linux.intel.com>]

On Sunday 24 January 2010 15:35:58 Avi Kivity wrote:
> On 01/22/2010 04:22 AM, Sheng Yang wrote:
> > The default action of coalesced MMIO is, cache the writing in buffer,
> > until: 1. The buffer is full.
> > 2. Or the exit to QEmu due to other reasons.
> >
> > But this would result in a very late writing in some condition.
> > 1. The each time write to MMIO content is small.
> > 2. The writing interval is big.
> > 3. No need for input or accessing other devices frequently.
> >
> > This issue was observed in a experimental embbed system. The test image
> > simply print "test" every 1 seconds. The output in QEmu meets
> > expectation, but the output in KVM is delayed for seconds.
> >
> > Per Avi's suggestion, I hooked a flushing for coalesced MMIO buffer in
> > VGA update handler. By this way, We don't need vcpu explicit exit to QEmu
> > to handle this issue.
> >
> > Signed-off-by: Sheng Yang<sheng@linux.intel.com>
> > ---
> >
> > Like this?
> >
> >   qemu-kvm.c |   26 ++++++++++++++++++++++++--
> >   qemu-kvm.h |    6 ++++++
> >   vl.c       |    2 ++
> >   3 files changed, 32 insertions(+), 2 deletions(-)
> >
> >
> >
> > +#ifdef KVM_CAP_COALESCED_MMIO
> > +void kvm_flush_coalesced_mmio_buffer(void)
> > +{
> > +    if (kvm_state->coalesced_mmio_ring) {
> > +        struct kvm_coalesced_mmio_ring *ring =
> > +            kvm_state->coalesced_mmio_ring;
> > +        while (ring->first != ring->last) {
> > +           
> > cpu_physical_memory_rw(ring->coalesced_mmio[ring->first].phys_addr,
> > +&ring->coalesced_mmio[ring->first].data[0],
> > +                           ring->coalesced_mmio[ring->first].len, 1);
> > +            smp_wmb();
> > +            ring->first = (ring->first + 1) % KVM_COALESCED_MMIO_MAX;
> > +        }
> > +    }
> > +}
> > +#endif
> > +
> 
> qemu has a coalesced mmio api, (qemu_register_colaesced_mmio), so please
> follow it (stubs in exec.c, and implementation in kvm-specific files).
> Please send the patch against the uq/master branch (which follow upstream).

You means put it in the kvm-all.c?(see the latest patch) Um, seems I have to 
import "libkvm.h" in with a #ifndef KVM_UPSTREAM, due to smp_wmb() and 
PAGE_SIZE macro. I am not sure about which is the proper way to get it work...
(the relationship between qemu-kvm and upstream qemu's kvm often confuse me. I 
supposed kvm-all.c should belong to upstream kvm, and qemu-kvm.* and libkvm 
should belong to qemu-kvm?)

Another issue is, upstream QEmu compile error with "TARGET_PHYS_ADDR_BITS 
redefined". I would like to wait for that to be fixed.
> 
> > diff --git a/vl.c b/vl.c
> > index 9edea10..64902f2 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -3235,6 +3235,7 @@ static void gui_update(void *opaque)
> >               interval = dcl->gui_timer_interval;
> >           dcl = dcl->next;
> >       }
> > +    kvm_flush_coalesced_mmio_buffer();
> >       qemu_mod_timer(ds->gui_timer, interval + qemu_get_clock(rt_clock));
> >   }
> 
> Better to do that before the call to dpy_refresh().

OK.
> 
> > @@ -3242,6 +3243,7 @@ static void nographic_update(void *opaque)
> >   {
> >       uint64_t interval = GUI_REFRESH_INTERVAL;
> >
> > +    kvm_flush_coalesced_mmio_buffer();
> >       qemu_mod_timer(nographic_timer, interval +
> > qemu_get_clock(rt_clock)); }
> 
> Any need to do it here?
> 
> (why does nographic_update use a timer?)
> 

VNC would need nographic_update().

-- 
regards
Yang, Sheng